mlx5 support tc accept action

mlx5 support tc accept action

[Tonghao Zhang]

Hi Roi, Saeed
In one cause, we want the "accept" action: the IP of VF will be
"accept", and others
packets will be done with other actions(e.g. hairpin rule to other VF).

For example:

PF0=enp130s0f0
VF0_REP=enp130s0f0_0
VF0=p4p1_0
VF1=p4p2_0 # belong to PF1
VF0_IP=3.3.3.200

ethtool -K $PF0 hw-tc-offload on
ethtool -K $VF0 hw-tc-offload on
tc qdisc add dev $PF0 ingress
tc qdisc add dev $VF0 ingress
tc filter add dev $PF0 protocol all parent ffff: prio 10 handle 1
flower skip_sw action mirred egress redirect dev $VF0_REP
tc filter add dev $VF0 protocol ip parent ffff: prio 1 handle 3 flower
skip_sw dst_ip $VF0_IP action pass
tc filter add dev $VF0 protocol all parent ffff: prio 10 handle 2
flower skip_sw action mirred egress redirect dev $VF1

When I change the driver, the rule which action "action pass", can be
offloaded, but it didn't work.
+               case FLOW_ACTION_ACCEPT:
+                   action |= MLX5_FLOW_CONTEXT_ACTION_ALLOW;
+                   break;


How can we support it, this function is import for us.

Re: mlx5 support tc accept action

[Saeed Mahameed]

On Wed, Dec 4, 2019 at 10:41 PM Tonghao Zhang <xiangxia.m.yue@gmail.com> wrote:
>
> Hi Roi, Saeed
> In one cause, we want the "accept" action: the IP of VF will be
> "accept", and others
> packets will be done with other actions(e.g. hairpin rule to other VF).
>
> For example:
>
> PF0=enp130s0f0
> VF0_REP=enp130s0f0_0
> VF0=p4p1_0
> VF1=p4p2_0 # belong to PF1
> VF0_IP=3.3.3.200
>
> ethtool -K $PF0 hw-tc-offload on
> ethtool -K $VF0 hw-tc-offload on
> tc qdisc add dev $PF0 ingress
> tc qdisc add dev $VF0 ingress
> tc filter add dev $PF0 protocol all parent ffff: prio 10 handle 1
> flower skip_sw action mirred egress redirect dev $VF0_REP
> tc filter add dev $VF0 protocol ip parent ffff: prio 1 handle 3 flower
> skip_sw dst_ip $VF0_IP action pass
> tc filter add dev $VF0 protocol all parent ffff: prio 10 handle 2
> flower skip_sw action mirred egress redirect dev $VF1
>
> When I change the driver, the rule which action "action pass", can be
> offloaded, but it didn't work.
> +               case FLOW_ACTION_ACCEPT:
> +                   action |= MLX5_FLOW_CONTEXT_ACTION_ALLOW;
> +                   break;
>
>
> How can we support it, this function is import for us.

Hi Tonghao,
where did you add the above code to ?
parse_tc_fdb_actions() ? or parse_tc_nic_actions() ?
in your use case you need to add it to parse_tc_nic_actions(),

currently in mlx5 we don't support ALLOW/pass actions.
it might be a little more complicated than what you did in order to
support this,
as a work around you can use action: FLOW_ACTION_MARK in the tc
command line rule without any change in the driver.
or change your code to do MLX5_FLOW_CONTEXT_ACTION_FWD_DEST instead of
MLX5_FLOW_CONTEXT_ACTION_ALLOW

Adding Mark and Ariel, they might have better feedback than mine

Thanks,
Saeed/

Re: mlx5 support tc accept action

[Tonghao Zhang]

On Fri, Dec 6, 2019 at 5:30 AM Saeed Mahameed <saeedm@dev.mellanox.co.il> wrote:
>
> On Wed, Dec 4, 2019 at 10:41 PM Tonghao Zhang <xiangxia.m.yue@gmail.com> wrote:
> >
> > Hi Roi, Saeed
> > In one cause, we want the "accept" action: the IP of VF will be
> > "accept", and others
> > packets will be done with other actions(e.g. hairpin rule to other VF).
> >
> > For example:
> >
> > PF0=enp130s0f0
> > VF0_REP=enp130s0f0_0
> > VF0=p4p1_0
> > VF1=p4p2_0 # belong to PF1
> > VF0_IP=3.3.3.200
> >
> > ethtool -K $PF0 hw-tc-offload on
> > ethtool -K $VF0 hw-tc-offload on
> > tc qdisc add dev $PF0 ingress
> > tc qdisc add dev $VF0 ingress
> > tc filter add dev $PF0 protocol all parent ffff: prio 10 handle 1
> > flower skip_sw action mirred egress redirect dev $VF0_REP
> > tc filter add dev $VF0 protocol ip parent ffff: prio 1 handle 3 flower
> > skip_sw dst_ip $VF0_IP action pass
> > tc filter add dev $VF0 protocol all parent ffff: prio 10 handle 2
> > flower skip_sw action mirred egress redirect dev $VF1
> >
> > When I change the driver, the rule which action "action pass", can be
> > offloaded, but it didn't work.
> > +               case FLOW_ACTION_ACCEPT:
> > +                   action |= MLX5_FLOW_CONTEXT_ACTION_ALLOW;
> > +                   break;
> >
> >
> > How can we support it, this function is import for us.
>
> Hi Tonghao,
> where did you add the above code to ?
> parse_tc_fdb_actions() ? or parse_tc_nic_actions() ?
> in your use case you need to add it to parse_tc_nic_actions(),
>
> currently in mlx5 we don't support ALLOW/pass actions.
> it might be a little more complicated than what you did in order to
> support this,
> as a work around you can use action: FLOW_ACTION_MARK in the tc
> command line rule without any change in the driver.
> or change your code to do MLX5_FLOW_CONTEXT_ACTION_FWD_DEST instead of
> MLX5_FLOW_CONTEXT_ACTION_ALLOW
Hi Saeed, FLOW_ACTION_MARK works fine for us. Thanks.

> Adding Mark and Ariel, they might have better feedback than mine
>
> Thanks,
> Saeed/

Re: mlx5 support tc accept action

[Mark Bloch]

On 12/5/2019 23:51, Tonghao Zhang wrote:
> On Fri, Dec 6, 2019 at 5:30 AM Saeed Mahameed <saeedm@dev.mellanox.co.il> wrote:
>>
>> On Wed, Dec 4, 2019 at 10:41 PM Tonghao Zhang <xiangxia.m.yue@gmail.com> wrote:
>>>
>>> Hi Roi, Saeed
>>> In one cause, we want the "accept" action: the IP of VF will be
>>> "accept", and others
>>> packets will be done with other actions(e.g. hairpin rule to other VF).
>>>
>>> For example:
>>>
>>> PF0=enp130s0f0
>>> VF0_REP=enp130s0f0_0
>>> VF0=p4p1_0
>>> VF1=p4p2_0 # belong to PF1
>>> VF0_IP=3.3.3.200
>>>
>>> ethtool -K $PF0 hw-tc-offload on
>>> ethtool -K $VF0 hw-tc-offload on
>>> tc qdisc add dev $PF0 ingress
>>> tc qdisc add dev $VF0 ingress
>>> tc filter add dev $PF0 protocol all parent ffff: prio 10 handle 1
>>> flower skip_sw action mirred egress redirect dev $VF0_REP
>>> tc filter add dev $VF0 protocol ip parent ffff: prio 1 handle 3 flower
>>> skip_sw dst_ip $VF0_IP action pass
>>> tc filter add dev $VF0 protocol all parent ffff: prio 10 handle 2
>>> flower skip_sw action mirred egress redirect dev $VF1
>>>
>>> When I change the driver, the rule which action "action pass", can be
>>> offloaded, but it didn't work.
>>> +               case FLOW_ACTION_ACCEPT:
>>> +                   action |= MLX5_FLOW_CONTEXT_ACTION_ALLOW;
>>> +                   break;
>>>
>>>
>>> How can we support it, this function is import for us.
>>
>> Hi Tonghao,
>> where did you add the above code to ?
>> parse_tc_fdb_actions() ? or parse_tc_nic_actions() ?
>> in your use case you need to add it to parse_tc_nic_actions(),
>>
>> currently in mlx5 we don't support ALLOW/pass actions.
>> it might be a little more complicated than what you did in order to
>> support this,

Yep, from a quick look at the code we don't support ALLOW + counter so a change
will be have to made in counter_is_valid() and then account for that on deletion in
del_sw_hw_rule().

Doesn't look too complicated to add if needed (It was just never requested/needed).

>> as a work around you can use action: FLOW_ACTION_MARK in the tc
>> command line rule without any change in the driver.
>> or change your code to do MLX5_FLOW_CONTEXT_ACTION_FWD_DEST instead of
>> MLX5_FLOW_CONTEXT_ACTION_ALLOW
> Hi Saeed, FLOW_ACTION_MARK works fine for us. Thanks.

Great.

Mark

> 
>> Adding Mark and Ariel, they might have better feedback than mine
>>
>> Thanks,
>> Saeed/