Re: [PATCH net-next 2/5] bnx2x: save cycles in setting gso_size

[David Miller <davem@davemloft.net>]

From: "Dmitry Kravkov" <dmitry@broadcom.com>
Date: Mon, 11 Oct 2010 00:27:55 +0200

> diff --git a/drivers/net/bnx2x/bnx2x_cmn.c b/drivers/net/bnx2x/bnx2x_cmn.c
> index cb2a3d6..ddf90e1 100644
> --- a/drivers/net/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/bnx2x/bnx2x_cmn.c
> @@ -277,8 +277,7 @@ static int bnx2x_fill_frag_skb(struct bnx2x *bp, struct bnx2x_fastpath *fp,
>  
>  	/* This is needed in order to enable forwarding support */
>  	if (frag_size)
> -		skb_shinfo(skb)->gso_size = min((u32)SGE_PAGE_SIZE,
> -					       max(frag_size, (u32)len_on_bd));
> +		skb_shinfo(skb)->gso_size = 1;
>  

I wonder why you need to set this here.

When you pass this packet into the stack, dev_gro_receive() is going
to set ->gro_size() unconditionally if any GRO processing is going to
occur at all.

Why do you need to set it at all?

And if we do need it, doesn't every driver that builds fragmented SKBs?
And if it's correct, why is setting a don't care value like "1" ok and
will not cause problems to whoever cares about this value?

I really want all of these questions answered, and at least lightly
explained in the commit message before I apply this patch.

Thanks.