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

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

quoted

On the bond, you can have LRO enabled and it is propagated to lower
devices so that lower devices will enable LRO if it is supported.  If
LRO is disabled on the bond (e.g. due to bridging), It will be
propagated so all lower devices will disable LRO if it is enabled.

GRO/GRO_HW is different and it is not propagated from upper devices to
lower devices like LRO.  Lower devices can have GRO/GRO_HW
enabled/disabled regardless of upper device.  This is no different
from say RXCSUM.  Lower devices can have RXCSUM on/off regardless.

So on the bond device you should be able to have LRO, GRO, and GRO_HW
enabled all at the same time and that means devices below it can have
any combination of those enabled. If I recall enabling it on the lower
device shouldn't be allowed if an upper device has it disabled. If it
is enabled on the upper device it can be either enabled or disabled on
the lower device depending on the state the device prefers.

Only LRO behaves the way you describe.  I believe LRO is treated like
this differently because bridging or routing needs to be able to
disable it completely.

GRO is not like that.  RXCSUM is not like that.

The GRO and GRO_HW should be propagated just like RXCSUM and
everything else. Basically if I turn it off on the bond all of the
lower devices should have it disabled. Only if it is enabled should
lower devices be allowed to have it either enabled or disabled. The
easiest way to think of it is on the bond the features are like a mask
of what can be enabled/disabled for the lower devices. If a feature is
not present in the bond it should not be present on any of the devices
below it. That is why I am saying you cannot say LRO and GRO_HW are
mutually exclusive for all devices. For physical devices I would say
yes it can be mutually exclusive, but for things like bond it cannot
be since you can have one device doing LRO and one device doing GRO_HW
getting added to a bond and both features should be advertised there.

I think you keep thinking that whatever we do with LRO, we must do the
same thing with GRO_HW.

But that's not true.  GRO/GRO_HW don't have to be disabled when
bridging/routing are enabled.  That's why upper devices don't have to
treat GRO/GRO_HW like LRO.  It can be treated just like RXCSUM which
is don't care.

My concern is if I have a bond with both GRO_HW and LRO enabled on
lower devices, and then disable something like TSO we are going to see
LRO disabled on the bond and propagated to all the lower devices.

I don't get this.  I don't see how TSO is related.

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.

So GRO_HW just naturally follows GRO since it is a true subset of it.