Re: [PATCH v3] net: af_packet: don't call tpacket_destruct_skb() until the skb is sent out

[Eric Dumazet <eric.dumazet@gmail.com>]

Le jeudi 23 septembre 2010 à 18:15 +0800, Changli Gao a écrit :
> Since skb->destructor() is used to account socket memory, and maybe called
> before the skb is sent out, a corrupt skb maybe sent out finally.
> 
> A new destructor is added into structure skb_shared_info(), and it won't
> be called until the last reference to the data of an skb is put. af_packet
> uses this destructor instead.
> 
> Signed-off-by: Changli Gao <xiaosuo@gmail.com>
> ---
> v3: rename destructor to data_destructor, destructor_arg to data_destructor_arg,
>     fix splice the skbs generated by AF_PACKET socket to the pipe.

I dont understand this.

Could you describe how splice(from af_packet to pipe) is possible with
af_packet send path ?

Also, on such risky patch, could you please avoid inserting cleanups ?
I am referring to these bits :



@@ -884,9 +883,8 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
        to_write = tp_len;
 
        if (sock->type == SOCK_DGRAM) {
-               err = dev_hard_header(skb, dev, ntohs(proto), addr,
-                               NULL, tp_len);
-               if (unlikely(err < 0))
+               if (unlikely(dev_hard_header(skb, dev, ntohs(proto), addr,
+                                            NULL, tp_len) < 0))
                        return -EINVAL;
        } else if (dev->hard_header_len) {
                /* net device doesn't like empty head */
@@ -897,8 +895,7 @@ static int tpacket_fill_skb(struct packet_sock *po,
struct sk_buff *skb,
                }
 
                skb_push(skb, dev->hard_header_len);
-               err = skb_store_bits(skb, 0, data,
-                               dev->hard_header_len);
+               err = skb_store_bits(skb, 0, data, dev->hard_header_len);
                if (unlikely(err))
                        return err;
 
@@ -906,7 +903,6 @@ static int tpacket_fill_skb(struct packet_sock *po, struct sk_buff *skb,
                to_write -= dev->hard_header_len;
        }
 
-       err = -EFAULT;
        page = virt_to_page(data);
        offset = offset_in_page(data);
        len_max = PAGE_SIZE - offset;