> 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). That looks like a larger change. Should we put this patch on hold waiting for the code refactor or we merge it as is (as it tells no lies about current code).