Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933182AbaLDVQF (ORCPT ); Thu, 4 Dec 2014 16:16:05 -0500 Received: from na6sys009bog030.obsmtp.com ([74.125.150.104]:50168 "HELO na6sys009bog030.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with SMTP id S932854AbaLDVQC (ORCPT ); Thu, 4 Dec 2014 16:16:02 -0500 MIME-Version: 1.0 In-Reply-To: References: <1417718366-14310-1-git-send-email-joestringer@nicira.com> From: Joe Stringer Date: Thu, 4 Dec 2014 13:15:30 -0800 Message-ID: Subject: Re: [PATCHv3 net] i40e: Implement ndo_gso_check() To: Tom Herbert Cc: Linux Netdev List , LKML , Jesse Gross , Shannon Nelson , "Brandeburg, Jesse" , Jeff Kirsher , "linux.nics" Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 4 December 2014 at 12:17, Tom Herbert wrote: > On Thu, Dec 4, 2014 at 10:39 AM, Joe Stringer wrote: >> ndo_gso_check() was recently introduced to allow NICs to report the >> offloading support that they have on a per-skb basis. Add an >> implementation for this driver which checks for IPIP, GRE, UDP tunnels. >> >> Signed-off-by: Joe Stringer >> --- >> v3: Drop IPIP and GRE (no driver support even though hw supports it). >> Check for UDP outer protocol for UDP tunnels. >> v2: Expand to include IP in IP and IPv4/IPv6 inside GRE/UDP tunnels. >> Add MAX_INNER_LENGTH (as 80). >> --- >> drivers/net/ethernet/intel/i40e/i40e_main.c | 26 ++++++++++++++++++++++++++ >> 1 file changed, 26 insertions(+) >> >> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c >> index c3a7f4a..0d6493a 100644 >> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c >> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c >> @@ -7447,6 +7447,31 @@ static int i40e_ndo_fdb_dump(struct sk_buff *skb, >> >> #endif /* USE_DEFAULT_FDB_DEL_DUMP */ >> #endif /* HAVE_FDB_OPS */ >> +static bool i40e_gso_check(struct sk_buff *skb, struct net_device *dev) >> +{ >> + if (skb_shinfo(skb)->gso_type & SKB_GSO_UDP_TUNNEL) { >> + unsigned char *ihdr; >> + >> + if (skb->protocol != IPPROTO_UDP || >> + skb->inner_protocol_type != ENCAP_TYPE_ETHER) >> + return false; >> + >> + if (skb->inner_protocol == htons(ETH_P_TEB)) >> + ihdr = skb_inner_mac_header(skb); >> + else if (skb->inner_protocol == htons(ETH_P_IP) || >> + skb->inner_protocol == htons(ETH_P_IPV6)) >> + ihdr = skb_inner_network_header(skb); >> + else >> + return false; >> + > > Wow, this is getting complicated! :-( It's not clear that the protocol > specific checks are needed here since it looks like the header length > is being passed to the device later on. Also, I think we need > skb_inner_mac_header(skb) - skb_transport_header(skb) to always work > to give the length of the encapsulation headers (in case there is no > real inner mac header, then that offset should be for the network > header). > > So would a simple check like this work: > > if (skb->encapsulation && > (skb_inner_mac_header(skb) - skb_transport_header(skb)) > > MAX_TUNNEL_HDR_LEN) > return false; Ah, I didn't realise that the inner_mac_header() would be equivalent to inner network when inner mac doesn't exist. This is much tidier, thanks Tom. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/