netdev - Re: [PATCHv2 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CANr6G5zrA6NUtcyhX1cwEO1Hiq9jnarB36P6m5eoHUFqWWc26Q@mail.gmail.com>
Date:	Fri, 21 Nov 2014 09:59:41 -0800
From:	Joe Stringer <joestringer@...ira.com>
To:	Jesse Gross <jesse@...ira.com>
Cc:	netdev <netdev@...r.kernel.org>,
	Shannon Nelson <shannon.nelson@...el.com>,
	"Brandeburg, Jesse" <jesse.brandeburg@...el.com>,
	Jeff Kirsher <jeffrey.t.kirsher@...el.com>,
	Tom Herbert <therbert@...gle.com>,
	"linux.nics" <linux.nics@...el.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCHv2 net] i40e: Implement ndo_gso_check()

On 20 November 2014 16:19, Jesse Gross <jesse@...ira.com> wrote:
> On Thu, Nov 20, 2014 at 3:11 PM, Joe Stringer <joestringer@...ira.com> wrote:
>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> index c3a7f4a..2b01c8d 100644
>> --- a/drivers/net/ethernet/intel/i40e/i40e_main.c
>> +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c
>> @@ -7447,6 +7447,36 @@ 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_IPIP) &&
>> +           (skb->inner_protocol_type != ENCAP_TYPE_IPPROTO ||
>> +            skb->inner_protocol != htons(IPPROTO_IPIP))) {
>
> I think this check on inner_protocol isn't really semantically correct
> - that field is a union with inner_ipproto, so it would be more
> correct to check against that and then you wouldn't need to use htons
> either.

Yes, translation error on my part, but if IPIP isn't exposed I'll just
drop this section.

> I don't know if we need to have the check at all for IPIP though -
> after all the driver doesn't expose support for it all (actually it
> doesn't expose GRE either). This raises kind of an interesting
> question about the checks though - it's pretty easy to add support to
> the driver for a new GSO type (and I imagine that people will be
> adding GRE soon) and forget to update the check.

If the check is more conservative, then testing would show that it's
not working and lead people to figure out why (and update the check).

>> +       if (skb_shinfo(skb)->gso_type & (SKB_GSO_GRE | SKB_GSO_UDP_TUNNEL)) {
>> +               unsigned char *ihdr;
>> +
>> +               if (skb->inner_protocol_type != ENCAP_TYPE_ETHER)
>> +                       return false;
>
> I guess if you want to be nice, it might be good to check the
> equivalent IP protocol types as well.

Can do (just UDP as I'll drop GRE as per the above).
--
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