netdev - Re: [PATCH net-next v2] docs: net: dsa: describe issues with checksum offload

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]

Date:   Thu, 14 Apr 2022 12:58:50 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Luiz Angelo Daros de Luca <luizluca@...il.com>
CC:     Andrew Lunn <andrew@...n.ch>, Kurt Kanzenbach <kurt@...utronix.de>,
        George McCollister <george.mccollister@...il.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "bhelgaas@...gle.com" <bhelgaas@...gle.com>,
        "corbet@....net" <corbet@....net>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        Florian Fainelli <f.fainelli@...il.com>
Subject: Re: [PATCH net-next v2] docs: net: dsa: describe issues with checksum
 offload

On Wed, Apr 13, 2022 at 11:48:46PM -0300, Luiz Angelo Daros de Luca wrote:
> > Ok, I'll go with "no checksum offload for its trailer tag, and bugs
> > never fixed because no one uses it", in any case no Sasquatch. Thanks.
> 
> Vladimir, so the DSA switch will not copy the offload flags when a tag
> requests tail room? At least it will work.
> 
> Now, if the offload HW does support that tag, what would be the
> options? Set the slave port checksum flag from userland?
> It would be nice to have some type of "magic trick" to have it enabled
> by default. I'm already expecting a no, but wouldn't it be a nice case
> for a DSA property in the device tree?
> 
> Regards,
> 
> Luiz

DSA calls netdev_upper_dev_link(master, slave_dev, NULL) to establish a
relationship with its master and the master driver can detect this by
monitoring NETDEV_CHANGEUPPER.

If we look a bit closer at the implementation of netdev_upper_dev_link
we see it calls __netdev_upper_dev_link() which contains an interesting
pair of arguments "void *upper_priv, void *upper_info". These are
accessible to netdev_master_upper_dev_link(), and the bonding driver
(for example) makes use of them, see bond_master_upper_dev_link().

I'm thinking DSA could create a struct netdev_dsa_upper_info too, and
certain DSA masters could populate things in it. Then DSA could look at
what the DSA master has said (or not said) and fix up features
accordingly.

One information that could get populated by the master is a bit field of
whether checksumming is supported for a certain tagging protocol.
I'd rather pass a full bit field of all tagging protocols, rather than
just the protocol in current use by the slave, because:
(a) it's less gory compared to having the master look at
    dsa_port_from_netdev(info->upper_dev)->cpu_dp->tag_ops->proto
(b) the DSA tagging protocol can change at runtime, and when it does, no
    NETDEV_CHANGEUPPER will be emitted, so the master won't have a
    chance to inform us whether it can offload checksumming for the new
    protocol. So it's better to have this information from the get go.

We'd also need the DSA master to populate a "bool acked=true" from this
new struct netdev_dsa_upper_info. The reason is to be able to
distinguish between an empty bit mask that means "yup, I really can't
offload checksumming for anything", and a bit mask that means "DSA who?"
(where checksum offloading is expected to work under the normal
circumstances described by you, no special code required).