netdev - Re: [PATCH net] net: Generalize ndo_gso_check to ndo_features

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: <CA+mtBx_F-ZbtfW5Mf-wXPF5ML3GMXTVNyxhjQzUjNOXaBW9+jA@mail.gmail.com>
Date:	Tue, 23 Dec 2014 13:54:37 -0800
From:	Tom Herbert <therbert@...gle.com>
To:	Sathya Perla <Sathya.Perla@...lex.com>
Cc:	Jesse Gross <jesse@...ira.com>, David Miller <davem@...emloft.net>,
	Linux Netdev List <netdev@...r.kernel.org>,
	Joe Stringer <joestringer@...ira.com>,
	Eric Dumazet <edumazet@...gle.com>
Subject: Re: [PATCH net] net: Generalize ndo_gso_check to ndo_features_check

On Mon, Dec 22, 2014 at 10:24 PM, Sathya Perla <Sathya.Perla@...lex.com> wrote:
>> -----Original Message-----
>> From: netdev-owner@...r.kernel.org [mailto:netdev-
>> owner@...r.kernel.org] On Behalf Of Tom Herbert
>>
>> On Mon, Dec 22, 2014 at 8:03 AM, Jesse Gross <jesse@...ira.com> wrote:
>> > GSO isn't the only offload feature with restrictions that
>> > potentially can't be expressed with the current features mechanism.
>> > Checksum is another although it's a general issue that could in
>> > theory apply to anything. Even if it may be possible to
>> > implement these restrictions in other ways, it can result in
>> > duplicate code or inefficient per-packet behavior.
>> >
>> > This generalizes ndo_gso_check so that drivers can remove any
>> > features that don't make sense for a given packet, similar to
>> > netif_skb_features(). It also converts existing driver
>> > restrictions to the new format, completing the work that was
>> > done to support tunnel protocols since the issues apply to
>> > checksums as well.
>> >
>> It's a nice feature, but I really hope that this is not used for
>> checksums. We already have a sufficiently general interface for that
>> and checksum is already computed in drivers to work around HW bugs.
>>
> The ndo_featureas_check() interface that includes a means to report
> inability to compute inner checksums on some tunnel types, seems like
> a useful feature. The Skyhawk-R NIC can support inner csum offload for
> either vxlan or nv-gre, but not both simultaneously. So, this ndo_
> is useful in reporting this situation.
> This would also obviate the need to have extra code in the drivers to
> compute csums in the above scenario.
>
You could use it for that, but many drivers already call
skb_checksum_help and I really doubt it makes sense to change all of
those. Besides that, I don't think we should be "encouraging" vendors
to continue developing NICs that do protocol specific checksums. The
general interface (NETIF_HW_CSUM) is incredibly simple and obviates a
whole bunch of complexity in drivers to figure out whether it can
checksum a packet.

Tom

> thanks,
> -Sathya
--
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