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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 10 Oct 2010 20:52:54 -0700 (PDT)
From:	David Miller <davem@...emloft.net>
To:	dmitry@...adcom.com
Cc:	netdev@...r.kernel.org, vladz@...adcom.com, eilong@...adcom.com
Subject: Re: [PATCH net-next 2/5] bnx2x: save cycles in setting gso_size

From: "Dmitry Kravkov" <dmitry@...adcom.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.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html