netdev - Re: [PATCHv3 net] i40e: 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 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] [day] [month] [year] [list]

Date:	Thu, 4 Dec 2014 13:15:30 -0800
From:	Joe Stringer <joestringer@...ira.com>
To:	Tom Herbert <therbert@...gle.com>
Cc:	Linux Netdev List <netdev@...r.kernel.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Jesse Gross <jesse@...ira.com>,
	Shannon Nelson <shannon.nelson@...el.com>,
	"Brandeburg, Jesse" <jesse.brandeburg@...el.com>,
	Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
	"linux.nics" <linux.nics@...el.com>
Subject: Re: [PATCHv3 net] i40e: Implement ndo_gso_check()

On 4 December 2014 at 12:17, Tom Herbert <therbert@...gle.com> wrote:
> On Thu, Dec 4, 2014 at 10:39 AM, Joe Stringer <joestringer@...ira.com> 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 <joestringer@...ira.com>
>> ---
>> 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 netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html