Re: [PATCH] pktgen: Clone skb to avoid corruption of skbs in ndo_start_xmit methods

Michał Mirosław
2011-07-20T15:36:09Z

2011/7/20 Neil Horman <nhorman@tuxdriver.com>:
> On Wed, Jul 20, 2011 at 06:24:15AM +0200, Eric Dumazet wrote:
>> Le mardi 19 juillet 2011 à 22:07 -0400, Neil Horman a écrit :
>> > >
>> > I think this is a good idea.  It lets pktgen dynamically make the clone/share
>> > decision dynamically and only impacts performance for those systems.
>> >
>>
>> Just let pktgen refuse to use clone_skb command for these devices.
>>
> copy that, This is by no means final, but what do you think of this?  If its
> agreeable to you, Ben, et al. I can add this to my local tree and start auditing
> all the drivers that may need to have the flag set.
>
> Regards
> Neil
>
>
> diff --git a/include/linux/if.h b/include/linux/if.h
> index 3bc63e6..ae904fe 100644
> --- a/include/linux/if.h
> +++ b/include/linux/if.h
> @@ -76,6 +76,7 @@
>  #define IFF_BRIDGE_PORT        0x4000          /* device used as bridge port */
>  #define IFF_OVS_DATAPATH       0x8000  /* device used as Open vSwitch
>                                         * datapath port */
> +#define IFF_CANT_SHARE_SKB     0x10000 /* Device can't share skbs in tx path */
>
>  #define IF_GET_IFACE   0x0001          /* for querying only */
>  #define IF_GET_PROTO   0x0002
> diff --git a/net/core/pktgen.c b/net/core/pktgen.c
> index f76079c..bf6d88d 100644
> --- a/net/core/pktgen.c
> +++ b/net/core/pktgen.c
> @@ -1071,6 +1071,9 @@ static ssize_t pktgen_if_write(struct file *file,
>                if (len < 0)
>                        return len;
>
> +               if (pkt_dev->priv_flags & IFF_CANT_SHARE_SKB)
> +                       return -EOPNOTSUPP;
> +
>                i += len;
>                pkt_dev->clone_skb = value;
>

I would prefer that the flag be inclusive, i.e. it should mark drivers
which can use shared skbs. And it might be better to clone the skb in
case the flag is disabled to keep functionality working.

Best Regards,
Michał Mirosław