Re: [PATCH net-next v2 1/5] net: Introduce NETIF_F_GRO_HW.

On Thu, Dec 7, 2017 at 3:35 PM, Alexander Duyck
[off-list ref] wrote:

On Thu, Dec 7, 2017 at 3:17 PM, Michael Chan [off-list ref] wrote:

quoted

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.

quoted

quoted

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.

quoted

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.