Menu
▾
▴
Re: [Linux-ATM-General] Can "pop" really be NULL?
|
From: chas w. <ch...@cm...> - 2003-08-30 13:59:16
|
In message <200...@wa...>,Duncan Sands writes:
>Why? Can vcc->pop really ever be NULL? And if it can,
>wouldn't it be better to initialize vcc->pop to
>dev_kfree_skb_any and eliminate all these checks from
>the drivers?
i dont believe vcc->pop() will be necessary in the future. you should
just call dev_kfree_skb_any(skb) and if a protocol (lec, clip, aal5
socket) needs to do something special it should use the destructor.
this isnt practical right now since the atm layer doesnt properly clone
skb's like it should. for instance if lec is passed a skb from the
network layer it should clone the skb if there is already a 'user'.
this is typicaly true for skb's associated with an active tcp session.
the destructor function is being used to manipulate the sk's wmem_alloc.
see skb_set_owner_[rw]. the atm layer doesnt take advantage of this
(probably because this feature post dates the initial atm code).
i had a patch that did part of this work. it was part of a rewrite to
fix sendmsg. i didnt do a very good job on the pppoatm bit.
===== net/atm/br2684.c 1.3 vs edited =====
--- 1.3/net/atm/br2684.c Wed Jun 4 20:57:07 2003
+++ edited/net/atm/br2684.c Thu Jun 19 11:53:56 2003
@@ -152,6 +152,7 @@
struct br2684_vcc *brvcc)
{
struct atm_vcc *atmvcc;
+ struct sk_buff *skb2;
#ifdef FASTER_VERSION
if (brvcc->encaps == e_llc)
memcpy(skb_push(skb, 8), llc_oui_pid_pad, 8);
@@ -161,15 +162,16 @@
#else
int minheadroom = (brvcc->encaps == e_llc) ? 10 : 2;
if (skb_headroom(skb) < minheadroom) {
- struct sk_buff *skb2 = skb_realloc_headroom(skb, minheadroom);
+ skb2 = skb_realloc_headroom(skb, minheadroom);
brvcc->copies_needed++;
- dev_kfree_skb(skb);
- if (skb2 == NULL) {
+ if (!skb2)
brvcc->copies_failed++;
- return 0;
- }
- skb = skb2;
- }
+ } else
+ skb2 = skb_clone(skb, GFP_ATOMIC);
+ dev_kfree_skb(skb);
+ if (!skb2)
+ return 0;
+ skb = skb2;
skb_push(skb, minheadroom);
if (brvcc->encaps == e_llc)
memcpy(skb->data, llc_oui_pid_pad, 10);
@@ -187,8 +189,8 @@
*/
dev_kfree_skb(skb);
return 0;
- }
- atomic_add(skb->truesize, &atmvcc->sk->sk_wmem_alloc);
+ }
+ skb_set_owner_w(skb, atmvcc->sk);
ATM_SKB(skb)->atm_options = atmvcc->atm_options;
brdev->stats.tx_packets++;
brdev->stats.tx_bytes += skb->len;
===== net/atm/clip.c 1.17 vs edited =====
--- 1.17/net/atm/clip.c Wed Jun 18 11:10:23 2003
+++ edited/net/atm/clip.c Thu Jun 19 11:48:06 2003
@@ -388,6 +388,7 @@
struct atm_vcc *vcc;
int old;
unsigned long flags;
+ struct sk_buff *skb2;
DPRINTK("clip_start_xmit (skb %p)\n",skb);
if (!skb->dst) {
@@ -426,6 +427,12 @@
return 0;
}
DPRINTK("neigh %p, vccs %p\n",entry,entry->vccs);
+ if (!(skb2 = skb_clone(skb, GFP_ATOMIC))) {
+ clip_priv->stats.tx_dropped++;
+ return 0;
+ }
+ dev_kfree_skb(skb);
+ skb = skb2;
ATM_SKB(skb)->vcc = vcc = entry->vccs->vcc;
DPRINTK("using neighbour %p, vcc %p\n",skb->dst->neighbour,vcc);
if (entry->vccs->encap) {
@@ -435,7 +442,7 @@
memcpy(here,llc_oui,sizeof(llc_oui));
((u16 *) here)[3] = skb->protocol;
}
- atomic_add(skb->truesize, &vcc->sk->sk_wmem_alloc);
+ skb_set_owner_w(skb, vcc->sk);
ATM_SKB(skb)->atm_options = vcc->atm_options;
entry->vccs->last_use = jiffies;
DPRINTK("atm_skb(%p)->vcc(%p)->dev(%p)\n",skb,vcc,vcc->dev);
===== net/atm/common.c 1.35 vs edited =====
--- 1.35/net/atm/common.c Wed Jun 18 11:10:23 2003
+++ edited/net/atm/common.c Thu Jun 19 11:29:42 2003
@@ -180,24 +180,6 @@
}
-static struct sk_buff *alloc_tx(struct atm_vcc *vcc,unsigned int size)
-{
- struct sk_buff *skb;
-
- if (atomic_read(&vcc->sk->sk_wmem_alloc) && !atm_may_send(vcc, size)) {
- DPRINTK("Sorry: wmem_alloc = %d, size = %d, sndbuf = %d\n",
- atomic_read(&vcc->sk->sk_wmem_alloc), size,
- vcc->sk->sk_sndbuf);
- return NULL;
- }
- while (!(skb = alloc_skb(size,GFP_KERNEL))) schedule();
- DPRINTK("AlTx %d += %d\n", atomic_read(&vcc->sk->sk_wmem_alloc),
- skb->truesize);
- atomic_add(skb->truesize, &vcc->sk->sk_wmem_alloc);
- return skb;
-}
-
-
EXPORT_SYMBOL(vcc_sklist);
EXPORT_SYMBOL(vcc_sklist_lock);
EXPORT_SYMBOL(vcc_insert_socket);
@@ -504,32 +486,24 @@
}
-int vcc_sendmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *m,
- int total_len)
+int vcc_sendmsg(struct kiocb *iocb, struct socket *sock, struct msghdr *msg,
+ int size)
{
- struct sock *sk = sock->sk;
DEFINE_WAIT(wait);
+ struct sock *sk = sock->sk;
struct atm_vcc *vcc;
struct sk_buff *skb;
- int eff,error;
- const void *buff;
- int size;
+ int error;
lock_sock(sk);
if (sock->state != SS_CONNECTED) {
error = -ENOTCONN;
goto out;
}
- if (m->msg_name) {
+ if (msg->msg_name) {
error = -EISCONN;
goto out;
}
- if (m->msg_iovlen != 1) {
- error = -ENOSYS; /* fix this later @@@ */
- goto out;
- }
- buff = m->msg_iov->iov_base;
- size = m->msg_iov->iov_len;
vcc = ATM_SD(sock);
if (test_bit(ATM_VF_RELEASED, &vcc->flags) ||
test_bit(ATM_VF_CLOSE, &vcc->flags)) {
@@ -540,51 +514,25 @@
error = -EPIPE;
goto out;
}
- if (!size) {
- error = 0;
- goto out;
- }
if (size < 0 || size > vcc->qos.txtp.max_sdu) {
error = -EMSGSIZE;
goto out;
}
- /* verify_area is done by net/socket.c */
- eff = (size+3) & ~3; /* align to word boundary */
- prepare_to_wait(&vcc->sleep, &wait, TASK_INTERRUPTIBLE);
- error = 0;
- while (!(skb = alloc_tx(vcc,eff))) {
- if (m->msg_flags & MSG_DONTWAIT) {
- error = -EAGAIN;
- break;
- }
- schedule();
- if (signal_pending(current)) {
- error = -ERESTARTSYS;
- break;
- }
- if (test_bit(ATM_VF_RELEASED,&vcc->flags) ||
- test_bit(ATM_VF_CLOSE,&vcc->flags)) {
- error = vcc->reply;
- break;
- }
- if (!test_bit(ATM_VF_READY,&vcc->flags)) {
- error = -EPIPE;
- break;
- }
- prepare_to_wait(&vcc->sleep, &wait, TASK_INTERRUPTIBLE);
- }
- finish_wait(&vcc->sleep, &wait);
- if (error)
+ skb = sock_alloc_send_skb(sk, size + 16, msg->msg_flags & MSG_DONTWAIT, &error);
+ if (!skb)
goto out;
+
+ skb_reserve(skb, 16);
+
skb->dev = NULL; /* for paths shared with net_device interfaces */
ATM_SKB(skb)->atm_options = vcc->atm_options;
- if (copy_from_user(skb_put(skb,size),buff,size)) {
+
+ error = memcpy_fromiovec(skb_put(skb, size), msg->msg_iov, size);
+ if (error) {
kfree_skb(skb);
- error = -EFAULT;
goto out;
}
- if (eff != size) memset(skb->data+size,0,eff-size);
- error = vcc->dev->ops->send(vcc,skb);
+ error = vcc->dev->ops->send(vcc, skb);
error = error ? error : size;
out:
release_sock(sk);
===== net/atm/lec.c 1.29 vs edited =====
--- 1.29/net/atm/lec.c Wed Jun 18 11:10:23 2003
+++ edited/net/atm/lec.c Thu Jun 19 13:32:03 2003
@@ -65,7 +65,6 @@
single destination while waiting for SVC */
static int lec_open(struct net_device *dev);
-static int lec_send_packet(struct sk_buff *skb, struct net_device *dev);
static int lec_close(struct net_device *dev);
static struct net_device_stats *lec_get_stats(struct net_device *dev);
static void lec_init(struct net_device *dev);
@@ -210,7 +209,7 @@
lec_send(struct atm_vcc *vcc, struct sk_buff *skb, struct lec_priv *priv)
{
if (atm_may_send(vcc, skb->len)) {
- atomic_add(skb->truesize, &vcc->sk->sk_wmem_alloc);
+ skb_set_owner_w(skb, vcc->sk);
ATM_SKB(skb)->vcc = vcc;
ATM_SKB(skb)->atm_options = vcc->atm_options;
priv->stats.tx_packets++;
@@ -223,7 +222,7 @@
}
static int
-lec_send_packet(struct sk_buff *skb, struct net_device *dev)
+lec_start_xmit(struct sk_buff *skb, struct net_device *dev)
{
struct sk_buff *skb2;
struct lec_priv *priv = (struct lec_priv *)dev->priv;
@@ -257,32 +256,35 @@
lec_handle_bridge(skb, dev);
#endif
- /* Make sure we have room for lec_id */
- if (skb_headroom(skb) < 2) {
+ min_frame_size = LEC_MINIMUM_8023_SIZE;
+#ifdef CONFIG_TR
+ if (priv->is_trdev)
+ min_frame_size = LEC_MINIMUM_8025_SIZE;
+#endif
- DPRINTK("lec_send_packet: reallocating skb\n");
- skb2 = skb_realloc_headroom(skb, LEC_HEADER_LEN);
- kfree_skb(skb);
- if (skb2 == NULL) return 0;
- skb = skb2;
- }
- skb_push(skb, 2);
+ /* Ensure we have headroom/tailroom or just clone */
+ if (skb_headroom(skb) < 2 ||
+ (skb->len + skb_tailroom(skb)) < min_frame_size) {
+ skb2 = skb_copy_expand(skb, 2 /* atleast */,
+ (skb->truesize < min_frame_size) ?
+ (min_frame_size - skb->truesize) : 0, GFP_ATOMIC);
+ } else
+ skb2 = skb_clone(skb, GFP_ATOMIC);
+ dev_kfree_skb(skb);
+ if (!skb2) {
+ priv->stats.tx_dropped++;
+ return 0;
+ }
+ skb = skb2;
+
+ /* Pad to minimum ethernet-frame size */
+ skb_padto(skb, min_frame_size);
/* Put le header to place, works for TokenRing too */
- lec_h = (struct lecdatahdr_8023*)skb->data;
+ skb_push(skb, 2);
+ lec_h = (struct lecdatahdr_8023 *) skb->data;
lec_h->le_header = htons(priv->lecid);
-#ifdef CONFIG_TR
- /* Ugly. Use this to realign Token Ring packets for
- * e.g. PCA-200E driver. */
- if (priv->is_trdev) {
- skb2 = skb_realloc_headroom(skb, LEC_HEADER_LEN);
- kfree_skb(skb);
- if (skb2 == NULL) return 0;
- skb = skb2;
- }
-#endif
-
#if DUMP_PACKETS > 0
printk("%s: send datalen:%ld lecid:%4.4x\n", dev->name,
skb->len, priv->lecid);
@@ -300,27 +302,6 @@
else
printk("%s...\n",buf);
#endif /* DUMP_PACKETS > 0 */
-
- /* Minimum ethernet-frame size */
-#ifdef CONFIG_TR
- if (priv->is_trdev)
- min_frame_size = LEC_MINIMUM_8025_SIZE;
- else
-#endif
- min_frame_size = LEC_MINIMUM_8023_SIZE;
- if (skb->len < min_frame_size) {
- if ((skb->len + skb_tailroom(skb)) < min_frame_size) {
- skb2 = skb_copy_expand(skb, 0,
- min_frame_size - skb->truesize, GFP_ATOMIC);
- dev_kfree_skb(skb);
- if (skb2 == NULL) {
- priv->stats.tx_dropped++;
- return 0;
- }
- skb = skb2;
- }
- skb_put(skb, min_frame_size - skb->len);
- }
/* Send to right vcc */
is_rdesc = 0;
@@ -340,13 +321,13 @@
send_vcc, send_vcc?send_vcc->flags:0, entry);
if (!send_vcc || !test_bit(ATM_VF_READY,&send_vcc->flags)) {
if (entry && (entry->tx_wait.qlen < LEC_UNRES_QUE_LEN)) {
- DPRINTK("%s:lec_send_packet: queuing packet, ", dev->name);
+ DPRINTK("%s:lec_start_xmit: queuing packet, ", dev->name);
DPRINTK("MAC address 0x%02x:%02x:%02x:%02x:%02x:%02x\n",
lec_h->h_dest[0], lec_h->h_dest[1], lec_h->h_dest[2],
lec_h->h_dest[3], lec_h->h_dest[4], lec_h->h_dest[5]);
skb_queue_tail(&entry->tx_wait, skb);
} else {
- DPRINTK("%s:lec_send_packet: tx queue full or no arp entry, dropping, ", dev->name);
+ DPRINTK("%s:lec_start_xmit: tx queue full or no arp entry, dropping, ", dev->name);
DPRINTK("MAC address 0x%02x:%02x:%02x:%02x:%02x:%02x\n",
lec_h->h_dest[0], lec_h->h_dest[1], lec_h->h_dest[2],
lec_h->h_dest[3], lec_h->h_dest[4], lec_h->h_dest[5]);
@@ -406,7 +387,6 @@
int i;
char *tmp; /* FIXME */
- atomic_sub(skb->truesize, &vcc->sk->sk_wmem_alloc);
mesg = (struct atmlec_msg *)skb->data;
tmp = skb->data;
tmp += sizeof(struct atmlec_msg);
@@ -633,7 +613,7 @@
dev->change_mtu = lec_change_mtu;
dev->open = lec_open;
dev->stop = lec_close;
- dev->hard_start_xmit = lec_send_packet;
+ dev->hard_start_xmit = lec_start_xmit;
dev->get_stats = lec_get_stats;
dev->set_multicast_list = lec_set_multicast_list;
===== net/atm/mpc.c 1.20 vs edited =====
--- 1.20/net/atm/mpc.c Wed Jun 18 11:10:23 2003
+++ edited/net/atm/mpc.c Thu Jun 19 12:06:19 2003
@@ -473,6 +473,7 @@
struct iphdr *iph;
char *buff;
uint32_t ipaddr = 0;
+ struct sk_buff *skb2;
static struct {
struct llc_snap_hdr hdr;
@@ -523,7 +524,12 @@
memcpy(skb->data, &llc_snap_mpoa_data, sizeof(struct llc_snap_hdr));
}
- atomic_add(skb->truesize, &entry->shortcut->sk->sk_wmem_alloc);
+ skb2 = skb_clone(skb, GFP_ATOMIC);
+ dev_kfree_skb_any(skb);
+ if (!skb2)
+ return 0;
+ skb = skb2;
+ skb_set_owner_w(skb, entry->shortcut->sk);
ATM_SKB(skb)->atm_options = entry->shortcut->atm_options;
entry->shortcut->send(entry->shortcut, skb);
entry->packets_fwded++;
@@ -869,7 +875,6 @@
struct mpoa_client *mpc = find_mpc_by_vcc(vcc);
struct k_message *mesg = (struct k_message*)skb->data;
- atomic_sub(skb->truesize, &vcc->sk->sk_wmem_alloc);
if (mpc == NULL) {
printk("mpoa: msg_from_mpoad: no mpc found\n");
===== net/atm/pppoatm.c 1.7 vs edited =====
--- 1.7/net/atm/pppoatm.c Wed Jun 4 20:57:07 2003
+++ edited/net/atm/pppoatm.c Thu Jun 19 12:05:42 2003
@@ -199,6 +199,8 @@
static int pppoatm_send(struct ppp_channel *chan, struct sk_buff *skb)
{
struct pppoatm_vcc *pvcc = chan_to_pvcc(chan);
+ struct sk_buff *skb2;
+
ATM_SKB(skb)->vcc = pvcc->atmvcc;
DPRINTK("(unit %d): pppoatm_send (skb=0x%p, vcc=0x%p)\n",
pvcc->chan.unit, skb, pvcc->atmvcc);
@@ -231,7 +233,12 @@
kfree_skb(skb);
return 1;
}
- atomic_add(skb->truesize, &ATM_SKB(skb)->vcc->sk->sk_wmem_alloc);
+ skb2 = skb_clone(skb, GFP_ATOMIC);
+ kfree_skb(skb);
+ if (!skb2)
+ return DROP_PACKET;
+ skb = skb2;
+ skb_set_owner_w(skb, ATM_SKB(skb)->vcc->sk);
ATM_SKB(skb)->atm_options = ATM_SKB(skb)->vcc->atm_options;
DPRINTK("(unit %d): atm_skb(%p)->vcc(%p)->dev(%p)\n",
pvcc->chan.unit, skb, ATM_SKB(skb)->vcc,
===== net/atm/raw.c 1.4 vs edited =====
--- 1.4/net/atm/raw.c Wed Jun 4 20:57:07 2003
+++ edited/net/atm/raw.c Thu Jun 19 12:08:50 2003
@@ -38,7 +38,6 @@
{
DPRINTK("APopR (%d) %d -= %d\n", vcc->vci, vcc->sk->sk_wmem_alloc,
skb->truesize);
- atomic_sub(skb->truesize, &vcc->sk->sk_wmem_alloc);
dev_kfree_skb_any(skb);
wake_up(&vcc->sleep);
}
===== net/atm/signaling.c 1.14 vs edited =====
--- 1.14/net/atm/signaling.c Wed Jun 18 11:10:23 2003
+++ edited/net/atm/signaling.c Thu Jun 19 12:08:54 2003
@@ -97,7 +97,6 @@
struct atm_vcc *session_vcc;
msg = (struct atmsvc_msg *) skb->data;
- atomic_sub(skb->truesize, &vcc->sk->sk_wmem_alloc);
DPRINTK("sigd_send %d (0x%lx)\n",(int) msg->type,
(unsigned long) msg->vcc);
vcc = *(struct atm_vcc **) &msg->vcc;
|