On Thu, Dec 7, 2017 at 3:35 PM, Alexander Duyck <alexander.du...@gmail.com> wrote: > On Thu, Dec 7, 2017 at 3:17 PM, Michael Chan <michael.c...@broadcom.com> > wrote: >> I don't get this. I don't see how TSO is related. > > It isn't. That is the point. If I change ANY feature it will trigger > fix_features to get called. It will then see we have GRO_HW and LRO > since we have one interface that supports one and one that supports > another. It will trigger this code and cause LRO to be disabled which > then will be propagated down.
I see. But this won't happen. Because the bonding driver is not advertising NETIF_F_GRO_HW in its hw_features. It is not advertising NETIF_F_GRO either but I think it gets added automatically since it is a software feature. So LRO won't get disabled on the bond when a unrelated feature is changed. But I think I see your point. I can make it so that it is up to individual driver's .ndo_fix_features() to drop LRO/GRO_HW as it sees fit, instead of doing it in the common netdev_fix_features(). That way, it is more flexible at least. > >>> That >>> is a bad behavior and shouldn't happen. In addition I would want to be >>> able to disable GRO_HW from the bond should I encounter a buggy >>> implementation of the GRO hardware offload at some point in the >>> future. I don't like it being tied so closely with GRO since all it >>> takes is one hardware failure and then we are dealing with people >>> disabling GRO due to reports of it being buggy instead of it being >>> tied specifically to GRO_HW. >>> >> >> Today, GRO, RXCSUM, NTUPLE, etc on a bond do not get propagated. You >> can propose and make them propagate if you want. > > Or you can fix your patches so you take it into account now instead of > putting things in a broken state and asking others to fix it. I don't think that things are necessarily broken today. LRO truly needs to be propagated. It's debatable whether other features like GRO/RXCSUM/NTUPLE should be centrally set by the upper device or not. > >> So GRO_HW just naturally follows GRO since it is a true subset of it. > > Right, and I am saying you have exposed a bug. If I don't want GRO on > the bond it should be propagated down. It doesn't make sense to allow > lower devices to assemble frames when the bond doesn't want them and > GSO won't kick in before it gets to the bond. GRO kicks in at the lower device before it gets to the bond if the lower device calls napi_gro_receive() and GRO is enabled.
