From: Paolo Abeni <pabeni@redhat.com> Date: 2017-09-27 08:29:38
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:
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
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:
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
From: Paolo Abeni <pabeni@redhat.com> Date: 2017-09-27 09:47:01
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:
quoted
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
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:
quoted
Wed, Sep 27, 2017 at 10:29:35AM CEST, pabeni@redhat.com wrote:
quoted
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?
quoted
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
From: Paolo Abeni <pabeni@redhat.com> Date: 2017-09-27 12:31:39
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:
quoted
On Wed, 2017-09-27 at 11:17 +0200, Jiri Pirko wrote:
quoted
Wed, Sep 27, 2017 at 10:29:35AM CEST, pabeni@redhat.com wrote:
quoted
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
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:
quoted
Wed, Sep 27, 2017 at 11:46:58AM CEST, pabeni@redhat.com wrote:
quoted
On Wed, 2017-09-27 at 11:17 +0200, Jiri Pirko wrote:
quoted
Wed, Sep 27, 2017 at 10:29:35AM CEST, pabeni@redhat.com wrote:
quoted
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.
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
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:
quoted
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