tc H/W offload issue with vxlan tunnels [was: nfp: flower vxlan tunnel offload]

tc H/W offload issue with vxlan tunnels [was: nfp: flower vxlan tunnel offload]

[Paolo Abeni]

Hi,

Moving to a separate theread, since I think this is more related to the
flower core infrastructure than to the netrome patches.

On Wed, 2017-09-27 at 09:40 +0200, Jiri Pirko wrote:
> This kind of hooks are giving me nightmares. The code is screwed up as
> it is already. I'm currently working on conversion to callbacks. This
> part is handled in:
> https://github.com/jpirko/linux_mlxsw/commits/jiri_devel_egdevcb

Thanks for the pointer.

I skimmed quickly on the code and indeed it cleans this area a lot.
If I read it correctly the ('good') command:

tc filter add dev vxlan0 protocol ip parent ffff: flower enc_key_id 102 
   enc_dst_port 4789 src_ip 3.4.5.6 skip_sw action [...]

will generate a call to:

mlx5e_setup_tc(eth0, TC_SETUP_CLSFLOWER, &cls_flower) via:

fl_hw_replace_filter() ->
  tc_setup_cb_call() -> 
    tc_exts_setup_cb_egdev_call() ->
      tc_setup_cb_egdev_call() ->
        tcf_action_egdev_cb_call() ->
          mlx5e_rep_setup_tc_cb()

and the 'bad' command:

tc filter add dev eth0 protocol ip parent ffff: flower enc_key_id 102 \
   enc_dst_port 4789 src_ip 3.4.5.6 skip_sw action [...]

will also call:

mlx5e_setup_tc(eth0, TC_SETUP_CLSFLOWER, &cls_flower) via:

fl_hw_replace_filter() ->
  ndo_setup_tc()

So it looks like the H/W offload hook will still be called with the
same arguments in both case, and 'bad' rule will still be pushed to the
H/W as the driver itself has no way to distinct between the two
scenarios.

[ Note: I referred to the mlx hook just for convenience, should be the
same with any driver implementing the same APIs ]

Am I missing something?

Thanks,

Paolo

Re: tc H/W offload issue with vxlan tunnels [was: nfp: flower vxlan tunnel offload]

[Jiri Pirko]

Wed, Sep 27, 2017 at 10:29:35AM CEST, pabeni@redhat.com wrote:
>Hi,
>
>Moving to a separate theread, since I think this is more related to the
>flower core infrastructure than to the netrome patches.
>
>On Wed, 2017-09-27 at 09:40 +0200, Jiri Pirko wrote:
>> This kind of hooks are giving me nightmares. The code is screwed up as
>> it is already. I'm currently working on conversion to callbacks. This
>> part is handled in:
>> https://github.com/jpirko/linux_mlxsw/commits/jiri_devel_egdevcb
>
>Thanks for the pointer.
>
>I skimmed quickly on the code and indeed it cleans this area a lot.
>If I read it correctly the ('good') command:
>
>tc filter add dev vxlan0 protocol ip parent ffff: flower enc_key_id 102 
>   enc_dst_port 4789 src_ip 3.4.5.6 skip_sw action [...]

I suppose "action mirred redirect eth0". Then yes, it will generate the
callpath you described below.

>
>will generate a call to:
>
>mlx5e_setup_tc(eth0, TC_SETUP_CLSFLOWER, &cls_flower) via:
>
>fl_hw_replace_filter() ->
>  tc_setup_cb_call() -> 
>    tc_exts_setup_cb_egdev_call() ->
>      tc_setup_cb_egdev_call() ->
>        tcf_action_egdev_cb_call() ->
>          mlx5e_rep_setup_tc_cb()
>
>and the 'bad' command:
>
>tc filter add dev eth0 protocol ip parent ffff: flower enc_key_id 102 \
>   enc_dst_port 4789 src_ip 3.4.5.6 skip_sw action [...]
>
>will also call:
>
>mlx5e_setup_tc(eth0, TC_SETUP_CLSFLOWER, &cls_flower) via:
>
>fl_hw_replace_filter() ->
>  ndo_setup_tc()

Sure. You are adding a rule to eth0, the call goes down to eth0 driver.
I'm missing why is it a problem? Why the call should not go down to the
eth0 driver?


>
>So it looks like the H/W offload hook will still be called with the
>same arguments in both case, and 'bad' rule will still be pushed to the
>H/W as the driver itself has no way to distinct between the two
>scenarios.

Why "bad"?

Regarding the distinction, driver knows if user add a rule directly to
the eth0, or if the eth0 is egress device in the action. Those are 2
separete driver entrypoints - of course, talking about code with my
changes.


>
>[ Note: I referred to the mlx hook just for convenience, should be the
>same with any driver implementing the same APIs ]
>
>Am I missing something?
>
>Thanks,
>
>Paolo

Re: tc H/W offload issue with vxlan tunnels [was: nfp: flower vxlan tunnel offload]

[Paolo Abeni]

On Wed, 2017-09-27 at 11:17 +0200, Jiri Pirko wrote:
> Wed, Sep 27, 2017 at 10:29:35AM CEST, pabeni@redhat.com wrote:
> > So it looks like the H/W offload hook will still be called with the
> > same arguments in both case, and 'bad' rule will still be pushed to the
> > H/W as the driver itself has no way to distinct between the two
> > scenarios.
> 
> Why "bad"?

Such rule is coped differently by the SW and the HW data path.

a rule like:

tc filter add dev eth0 protocol ip parent ffff: flower \
   enc_key_id 102 enc_dst_port 4789 src_ip 3.4.5.6 skip_hw \
   action action mirred redirect eth0_vf_1

will match 0 packets, while:

tc filter add dev eth0 protocol ip parent ffff: flower \
   enc_key_id 102 enc_dst_port 4789 src_ip 3.4.5.6 skip_sw \
   action action mirred redirect eth0_vf_1

[just flipped 'skip_sw' and 'skip_hw' ]
will match the vxlan-tunneled packets. I understand that one of the
design goal for the h/w offload path is being consistent with the sw
one, but that does not hold in the above scenario.

> Regarding the distinction, driver knows if user add a rule directly to
> the eth0, or if the eth0 is egress device in the action. Those are 2
> separete driver entrypoints - of course, talking about code with my
> changes.

ok, but than each driver should catch the scenario "rule with tunnel
match over non tunnel device" and cope with them properly - never match
it - why don't simply avoiding pushing such rules to the H/W ? 

Cheers,

Paolo

Re: tc H/W offload issue with vxlan tunnels [was: nfp: flower vxlan tunnel offload]

[Jiri Pirko]

Wed, Sep 27, 2017 at 11:46:58AM CEST, pabeni@redhat.com wrote:
>On Wed, 2017-09-27 at 11:17 +0200, Jiri Pirko wrote:
>> Wed, Sep 27, 2017 at 10:29:35AM CEST, pabeni@redhat.com wrote:
>> > So it looks like the H/W offload hook will still be called with the
>> > same arguments in both case, and 'bad' rule will still be pushed to the
>> > H/W as the driver itself has no way to distinct between the two
>> > scenarios.
>> 
>> Why "bad"?
>
>Such rule is coped differently by the SW and the HW data path.
>
>a rule like:
>
>tc filter add dev eth0 protocol ip parent ffff: flower \
>   enc_key_id 102 enc_dst_port 4789 src_ip 3.4.5.6 skip_hw \
>   action action mirred redirect eth0_vf_1
>
>will match 0 packets, while:
>
>tc filter add dev eth0 protocol ip parent ffff: flower \
>   enc_key_id 102 enc_dst_port 4789 src_ip 3.4.5.6 skip_sw \
>   action action mirred redirect eth0_vf_1
>
>[just flipped 'skip_sw' and 'skip_hw' ]
>will match the vxlan-tunneled packets. I understand that one of the
>design goal for the h/w offload path is being consistent with the sw
>one, but that does not hold in the above scenario.

Sure, the consistency is important. Howcome "skip_hw" won't match and
"skip_sw" will match? What's different?


>
>> Regarding the distinction, driver knows if user add a rule directly to
>> the eth0, or if the eth0 is egress device in the action. Those are 2
>> separete driver entrypoints - of course, talking about code with my
>> changes.
>
>ok, but than each driver should catch the scenario "rule with tunnel
>match over non tunnel device" and cope with them properly - never match
>it - why don't simply avoiding pushing such rules to the H/W ? 
>
>Cheers,
>
>Paolo

Re: tc H/W offload issue with vxlan tunnels [was: nfp: flower vxlan tunnel offload]

[Paolo Abeni]

On Wed, 2017-09-27 at 13:11 +0200, Jiri Pirko wrote:
> Wed, Sep 27, 2017 at 11:46:58AM CEST, pabeni@redhat.com wrote:
> > On Wed, 2017-09-27 at 11:17 +0200, Jiri Pirko wrote:
> > > Wed, Sep 27, 2017 at 10:29:35AM CEST, pabeni@redhat.com wrote:
> > > > So it looks like the H/W offload hook will still be called with the
> > > > same arguments in both case, and 'bad' rule will still be pushed to the
> > > > H/W as the driver itself has no way to distinct between the two
> > > > scenarios.
> > > 
> > > Why "bad"?
> > 
> > Such rule is coped differently by the SW and the HW data path.
> > 
> > a rule like:
> > 
> > tc filter add dev eth0 protocol ip parent ffff: flower \
> >   enc_key_id 102 enc_dst_port 4789 src_ip 3.4.5.6 skip_hw \
> >   action action mirred redirect eth0_vf_1
> > 
> > will match 0 packets, while:
> > 
> > tc filter add dev eth0 protocol ip parent ffff: flower \
> >   enc_key_id 102 enc_dst_port 4789 src_ip 3.4.5.6 skip_sw \
> >   action action mirred redirect eth0_vf_1
> > 
> > [just flipped 'skip_sw' and 'skip_hw' ]
> > will match the vxlan-tunneled packets. I understand that one of the
> > design goal for the h/w offload path is being consistent with the sw
> > one, but that does not hold in the above scenario.
> 
> Sure, the consistency is important. Howcome "skip_hw" won't match and
> "skip_sw" will match? What's different?

For the SW datapath, we need a metadata based/lwt tunnel to collect the
tunnel information. eth0 is not a such device and does not provide the
metadata. Any match on tunnel based field will fail - correctly.

When the HW datapath is used, the underlaying NIC is programmed exactly
as done when we replace eth0 with vxlan0. The programmed flow matches
vxlan encapsulated traffic.

Cheers,

Paolo

Re: tc H/W offload issue with vxlan tunnels [was: nfp: flower vxlan tunnel offload]

[Jiri Pirko]

Wed, Sep 27, 2017 at 02:31:36PM CEST, pabeni@redhat.com wrote:
>On Wed, 2017-09-27 at 13:11 +0200, Jiri Pirko wrote:
>> Wed, Sep 27, 2017 at 11:46:58AM CEST, pabeni@redhat.com wrote:
>> > On Wed, 2017-09-27 at 11:17 +0200, Jiri Pirko wrote:
>> > > Wed, Sep 27, 2017 at 10:29:35AM CEST, pabeni@redhat.com wrote:
>> > > > So it looks like the H/W offload hook will still be called with the
>> > > > same arguments in both case, and 'bad' rule will still be pushed to the
>> > > > H/W as the driver itself has no way to distinct between the two
>> > > > scenarios.
>> > > 
>> > > Why "bad"?
>> > 
>> > Such rule is coped differently by the SW and the HW data path.
>> > 
>> > a rule like:
>> > 
>> > tc filter add dev eth0 protocol ip parent ffff: flower \
>> >   enc_key_id 102 enc_dst_port 4789 src_ip 3.4.5.6 skip_hw \
>> >   action action mirred redirect eth0_vf_1
>> > 
>> > will match 0 packets, while:
>> > 
>> > tc filter add dev eth0 protocol ip parent ffff: flower \
>> >   enc_key_id 102 enc_dst_port 4789 src_ip 3.4.5.6 skip_sw \
>> >   action action mirred redirect eth0_vf_1
>> > 
>> > [just flipped 'skip_sw' and 'skip_hw' ]
>> > will match the vxlan-tunneled packets. I understand that one of the
>> > design goal for the h/w offload path is being consistent with the sw
>> > one, but that does not hold in the above scenario.
>> 
>> Sure, the consistency is important. Howcome "skip_hw" won't match and
>> "skip_sw" will match? What's different?
>
>For the SW datapath, we need a metadata based/lwt tunnel to collect the
>tunnel information. eth0 is not a such device and does not provide the
>metadata. Any match on tunnel based field will fail - correctly.

So where do you attach the tc filter instead of eth0? vxlan0?


>
>When the HW datapath is used, the underlaying NIC is programmed exactly
>as done when we replace eth0 with vxlan0. The programmed flow matches
>vxlan encapsulated traffic.

Ok, so we should unify the behaviour with sw path and don't allow such
mathing in hw.


>
>Cheers,
>
>Paolo

Re: tc H/W offload issue with vxlan tunnels [was: nfp: flower vxlan tunnel offload]

[Jiri Benc]

On Wed, 27 Sep 2017 14:55:09 +0200, Jiri Pirko wrote:
> So where do you attach the tc filter instead of eth0? vxlan0?

Yes, vxlan0. I'm pasting the example from earlier in this thread again:

This will match:

ip link add vxlan0 type vxlan dstport 4789 dev eth0 external
ip link set dev vxlan0 up
tc qdisc add dev vxlan0 ingress
ethtool -K eth0 hw-tc-offload on
tc filter add dev vxlan0 protocol ip parent ffff: flower enc_key_id 102 \
   enc_dst_port 4789 src_ip 3.4.5.6 skip_sw action [...]

while this must NOT match:

ip link add vxlan0 type vxlan dstport 4789 dev eth0 external
ip link set dev vxlan0 up
tc qdisc add dev eth0 ingress
ethtool -K eth0 hw-tc-offload on
tc filter add dev eth0 protocol ip parent ffff: flower enc_key_id 102 \
   enc_dst_port 4789 src_ip 3.4.5.6 skip_sw action [...]

 Jiri

Re: tc H/W offload issue with vxlan tunnels [was: nfp: flower vxlan tunnel offload]

[Jiri Pirko]

Wed, Sep 27, 2017 at 05:27:29PM CEST, jbenc@redhat.com wrote:
>On Wed, 27 Sep 2017 14:55:09 +0200, Jiri Pirko wrote:
>> So where do you attach the tc filter instead of eth0? vxlan0?
>
>Yes, vxlan0. I'm pasting the example from earlier in this thread again:
>
>This will match:
>
>ip link add vxlan0 type vxlan dstport 4789 dev eth0 external
>ip link set dev vxlan0 up
>tc qdisc add dev vxlan0 ingress
>ethtool -K eth0 hw-tc-offload on
>tc filter add dev vxlan0 protocol ip parent ffff: flower enc_key_id 102 \
>   enc_dst_port 4789 src_ip 3.4.5.6 skip_sw action [...]
>
>while this must NOT match:
>
>ip link add vxlan0 type vxlan dstport 4789 dev eth0 external
>ip link set dev vxlan0 up
>tc qdisc add dev eth0 ingress
>ethtool -K eth0 hw-tc-offload on
>tc filter add dev eth0 protocol ip parent ffff: flower enc_key_id 102 \
>   enc_dst_port 4789 src_ip 3.4.5.6 skip_sw action [...]

Right. Driver of eth0 should get ndo_setup_tc calls for vxlan0. Similar
thing will be needed for bonding/team. I'm taking this into
considaration for my sharedblock patchset. Work in progress:
https://github.com/jpirko/linux_mlxsw/commits/jiri_devel_shblock
Basically the eth0 driver will register a callback function that would
be called whenever filter is added/deleted on vxlan0