Re: [RFC net-next 0/6] offload linux bonding tc ingress rules

[Ido Schimmel]

On Mon, Mar 05, 2018 at 01:28:28PM +, John Hurley wrote:
> To prevent sync issues between the kernel and offload device, the linux
> bond driver is affectively locked when it has offloaded rules. i.e no new
> ports can be enslaved and no slaves can be released until the offload
> rules are removed. Similarly, if a port on a bond is deleted, the bond is
> destroyed, forcing a flush of all offloaded rules.

Hi John,

I understand where this is coming from, but I don't think these
semantics are acceptable. The part about not adding new slaves might
make sense, but one needs to be able to remove slaves at any time.

Anyway, it would be much better to handle this in a generic way that
team and other stacked devices can later re-use. There's a similar sync
issue with VLAN filtering, which is handled by bond/team by calling
vlan_vids_add_by_dev() and vlan_vids_del_by_dev() in their
ndo_add_slave() and ndo_del_slave(), respectively. You can do something
similar and call into TC to replay the necessary information to the
newly added slave?

Re: [RFC net-next 0/6] offload linux bonding tc ingress rules

[Roopa Prabhu]

On Mon, Mar 5, 2018 at 2:08 PM, Jakub Kicinski
 wrote:
> On Mon,  5 Mar 2018 13:28:28 +, John Hurley wrote:
>> The linux bond itself registers a cb for offloading tc rules. Potential
>> slave netdevs on offload devices can then register with the bond for a
>> further callback - this code is basically the same as registering for an
>> egress dev offload in TC. Then when a rule is offloaded to the bond, it
>> can be relayed to each netdev that has registered with the bond code and
>> which is a slave of the given bond.
>
> As you know I would much rather see this handled in the TC core,
> similarly to how blocks are shared.  We can add a new .ndo_setup_tc
> notification like TC_MASTER_BLOCK_BIND and reuse the existing offload
> tracking.  It would also fix the problem of freezing the bond and allow
> better code reuse with team etc.

+1 to handle this in tc core. We will soon find that many other
devices will need to propagate rules down
 the netdev stack and keeping it in tc core allows re-use like you state above.
The switchdev api's before they moved to notifiers in many cases had
bond and other netdev stack offload
traversal inside the switchdev layer (In the notifier world, I think a
driver can still register and track rules and other offload on
 bonds with its own ports as slaves)

Re: [RFC net-next 0/6] offload linux bonding tc ingress rules

[Jakub Kicinski]

On Mon, 5 Mar 2018 23:57:18 +, John Hurley wrote:
> > what is your approach for rules whose bond is their egress device
> > (ingress = vf port
> > representor)?  
> 
> Egress offload will be handled entirely by the NFP driver.
> Basically, notifiers will track the slaves/masters and update the card
> with any groups that consist entirely of reprs.
> We then offload the TC rule outputting to the given group - because it
> is an ingress match we can access the egress netdev in the block
> callback.

And you handle egdev call too?  Or are we hoping to get rid of that
before? :)

Re: [RFC net-next 0/6] offload linux bonding tc ingress rules

[John Hurley]

On Mon, Mar 5, 2018 at 9:43 PM, Or Gerlitz  wrote:
> On Mon, Mar 5, 2018 at 3:28 PM, John Hurley  wrote:
>> This RFC patchset adds support for offloading tc ingress rules applied to
>> linux bonds. The premise of these patches is that if a rule is applied to
>> a bond port then the rule should be applied to each slave of the bond.
>>
>> The linux bond itself registers a cb for offloading tc rules. Potential
>> slave netdevs on offload devices can then register with the bond for a
>> further callback - this code is basically the same as registering for an
>> egress dev offload in TC. Then when a rule is offloaded to the bond, it
>> can be relayed to each netdev that has registered with the bond code and
>> which is a slave of the given bond.
>>
>> To prevent sync issues between the kernel and offload device, the linux
>> bond driver is affectively locked when it has offloaded rules. i.e no new
>> ports can be enslaved and no slaves can be released until the offload
>> rules are removed. Similarly, if a port on a bond is deleted, the bond is
>> destroyed, forcing a flush of all offloaded rules.
>>
>> Also included in the RFC are changes to the NFP driver to utilise the new
>> code by registering NFP port representors for bond offload rules and
>> modifying cookie handling to allow the relaying of a rule to multiple ports.
>
> what is your approach for rules whose bond is their egress device
> (ingress = vf port
> representor)?

Egress offload will be handled entirely by the NFP driver.
Basically, notifiers will track the slaves/masters and update the card
with any groups that consist entirely of reprs.
We then offload the TC rule outputting to the given group - because it
is an ingress match we can access the egress netdev in the block
callback.

Re: [RFC net-next 0/6] offload linux bonding tc ingress rules

[Jakub Kicinski]

On Mon,  5 Mar 2018 13:28:28 +, John Hurley wrote:
> The linux bond itself registers a cb for offloading tc rules. Potential
> slave netdevs on offload devices can then register with the bond for a
> further callback - this code is basically the same as registering for an
> egress dev offload in TC. Then when a rule is offloaded to the bond, it
> can be relayed to each netdev that has registered with the bond code and
> which is a slave of the given bond.

As you know I would much rather see this handled in the TC core,
similarly to how blocks are shared.  We can add a new .ndo_setup_tc
notification like TC_MASTER_BLOCK_BIND and reuse the existing offload
tracking.  It would also fix the problem of freezing the bond and allow
better code reuse with team etc.

For tunnel offloads we necessarily have to stick to the weak offload
model, where any offload success satisfies skip_sw, but in case of
bonds we should strive for the strong model (as you are doing AFAICT).

The only difficulty seems to be replaying the bind commands when port
joins?  I mean finding all blocks on a bond.  But that should be
surmountable..

Re: [RFC net-next 0/6] offload linux bonding tc ingress rules

[Or Gerlitz]

On Mon, Mar 5, 2018 at 3:28 PM, John Hurley  wrote:
> This RFC patchset adds support for offloading tc ingress rules applied to
> linux bonds. The premise of these patches is that if a rule is applied to
> a bond port then the rule should be applied to each slave of the bond.
>
> The linux bond itself registers a cb for offloading tc rules. Potential
> slave netdevs on offload devices can then register with the bond for a
> further callback - this code is basically the same as registering for an
> egress dev offload in TC. Then when a rule is offloaded to the bond, it
> can be relayed to each netdev that has registered with the bond code and
> which is a slave of the given bond.
>
> To prevent sync issues between the kernel and offload device, the linux
> bond driver is affectively locked when it has offloaded rules. i.e no new
> ports can be enslaved and no slaves can be released until the offload
> rules are removed. Similarly, if a port on a bond is deleted, the bond is
> destroyed, forcing a flush of all offloaded rules.
>
> Also included in the RFC are changes to the NFP driver to utilise the new
> code by registering NFP port representors for bond offload rules and
> modifying cookie handling to allow the relaying of a rule to multiple ports.

what is your approach for rules whose bond is their egress device
(ingress = vf port
representor)?

[RFC net-next 0/6] offload linux bonding tc ingress rules

[John Hurley]

Hi,

This RFC patchset adds support for offloading tc ingress rules applied to
linux bonds. The premise of these patches is that if a rule is applied to
a bond port then the rule should be applied to each slave of the bond.

The linux bond itself registers a cb for offloading tc rules. Potential
slave netdevs on offload devices can then register with the bond for a
further callback - this code is basically the same as registering for an
egress dev offload in TC. Then when a rule is offloaded to the bond, it
can be relayed to each netdev that has registered with the bond code and
which is a slave of the given bond.

To prevent sync issues between the kernel and offload device, the linux
bond driver is affectively locked when it has offloaded rules. i.e no new
ports can be enslaved and no slaves can be released until the offload
rules are removed. Similarly, if a port on a bond is deleted, the bond is
destroyed, forcing a flush of all offloaded rules.

Also included in the RFC are changes to the NFP driver to utilise the new
code by registering NFP port representors for bond offload rules and
modifying cookie handling to allow the relaying of a rule to multiple
ports.

Thanks,
John

John Hurley (6):
  drivers: net: bonding: add tc offload infastructure to bond
  driver: net: bonding: allow registration of tc offload callbacks in
bond
  drivers: net: bonding: restrict bond mods when rules are offloaded
  nfp: add ndo_set_mac_address for representors
  nfp: register repr ports for bond offloads
  nfp: support offloading multiple rules with same cookie

 drivers/net/bonding/bond_main.c| 277 -
 drivers/net/ethernet/netronome/nfp/flower/main.c   |  24 +-
 drivers/net/ethernet/netronome/nfp/flower/main.h   |  10 +-
 .../net/ethernet/netronome/nfp/flower/metadata.c   |  20 +-
 .../net/ethernet/netronome/nfp/flower/offload.c|  33 ++-
 drivers/net/ethernet/netronome/nfp/nfp_net_repr.c  |   1 +
 include/net/bonding.h  |   9 +
 7 files changed, 351 insertions(+), 23 deletions(-)

-- 
2.7.4