netdev - Re: [PATCH net 3/5] fm10k: Implement ndo_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 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:	Fri, 7 Nov 2014 18:05:17 +1300
From:	Joe Stringer <joestringer@...ira.com>
To:	"Vick, Matthew" <matthew.vick@...el.com>
Cc:	"alexander.duyck@...il.com" <alexander.duyck@...il.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"Dept-GELinuxNICDev@...gic.com" <Dept-GELinuxNICDev@...gic.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"sathya.perla@...lex.com" <sathya.perla@...lex.com>,
	"Kirsher, Jeffrey T" <jeffrey.t.kirsher@...el.com>,
	Linux NICS <Linux-nics@...el.com>,
	"amirv@...lanox.com" <amirv@...lanox.com>,
	"shahed.shaikh@...gic.com" <shahed.shaikh@...gic.com>,
	"therbert@...gle.com" <therbert@...gle.com>
Subject: Re: [PATCH net 3/5] fm10k: Implement ndo_gso_check()

On Fri, 07 Nov 2014 14:07:36 Vick, Matthew wrote:
> On 11/6/14, 1:15 PM, "Joe Stringer" <joestringer@...ira.com> wrote:
> >Oh, I suppose we need to check the gso_type too. More like this?
> >
> >+static bool fm10k_gso_check(struct sk_buff *skb, struct net_device *dev)
> >+{
> >+       if ((skb_shinfo(skb)->gso_type & (SKB_GSO_UDP_TUNNEL |
> >SKB_GSO_GRE)) &&
> >+           !fm10k_tx_encap_offload(skb))
> >+               return false;
> >+
> >+       return true;
> >+}
> 
> It seems like the skb_shinfo(skb)->gso_type check should be in some more
> generic ndo_gso_check that drivers can default to/extend. Then, we could
> end up with something like
> 
> static bool fm10k_gso_check(struct sk_buff *skb, struct net_device *dev)
> {
> 	if (skb_gso_check(skb, dev) && !fm10k_tx_encap_offload(skb))
> 		return false;
> 
> 	return true;
> }
> 
> This could even be simplified and still legible as
> 
> static bool fm10k_gso_check(struct sk_buff *skb, struct net_device *dev)
> {
> 	return !(skb_gso_check(skb, dev) && !fm10k_tx_encap_offload(skb));
> }
> 
> What do you think of this approach?

Let's merge both discussions into one thread (pick here or there). We have 
this suggestion or the one which simply checks for tunnels and inner+outer 
header lengths. Do you have a preference between them?

We could introduce an "skb_is_gso_encap()" or similar for this purpose. 
Checking for SKB_GSO_UDP_TUNNEL or SKB_GSO_GRE is pretty closely tied to what 
fm10k_tx_encap_offload() checks for though; it might not even make sense to call 
it if some of the other SKB_GSO_* flags are raised.

Joe
--
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