Re: Question about flow table offload in mlx5e

[wenxu <wenxu@ucloud.cn>]

The normal FWD rules that fwd to pf-rep or vf-rep can real offloaded

在 2019/11/21 21:05, Paul Blakey 写道:
> I see, I will test that, and how about normal FWD rules?
>
> Paul.
>
>
>> -----Original Message-----
>> From: wenxu <wenxu@ucloud.cn>
>> Sent: Thursday, November 21, 2019 2:35 PM
>> To: Paul Blakey <paulb@mellanox.com>
>> Cc: pablo@netfilter.org; netdev@vger.kernel.org; Mark Bloch
>> <markb@mellanox.com>
>> Subject: Re: Question about flow table offload in mlx5e
>>
>>
>> 在 2019/11/21 19:39, Paul Blakey 写道:
>>> They are good fixes, exactly what we had when we tested this, thanks.
>>>
>>> Regarding encap, I don't know what changes you did, how does the encap
>> rule look? Is it a FWD to vxlan device? If not it should be, as our driver
>> expects that.
>> It is fwd to a gretap devices
>>> I tried it on my setup via tc, by changing the callback of tc
>> (mlx5e_rep_setup_tc_cb) to that of ft (mlx5e_rep_setup_ft_cb),
>>> and testing a vxlan encap rule:
>>> sudo tc qdisc add dev ens1f0_0 ingress
>>> sudo ifconfig ens1f0 7.7.7.7/24 up
>>> sudo ip link add name vxlan0 type vxlan dev ens1f0 remote 7.7.7.8 dstport
>> 4789 external
>>> sudo ifconfig vxlan0 up
>>> sudo tc filter add dev ens1f0_0 ingress prio 1 chain 0 protocol ip flower
>> dst_mac aa:bb:cc:dd:ee:ff ip_proto udp skip_sw  action tunnel_key set
>> src_ip 0.0.0.0 dst_ip 7.7.7.8 id 1234 dst_port 4789 pipe action mirred egress
>> redirect dev vxlan
>>> then tc show:
>>> filter protocol ip pref 1 flower chain 0 handle 0x1 dst_mac aa:bb:cc:dd:ee:ff
>> ip_proto udp skip_sw in_hw in_hw_count 1
>>>         tunnel_key set src_ip 0.0.0.0 dst_ip 7.7.7.8 key_id 1234 dst_port 4789
>> csum pipe
>>>         Stats: used 119 sec      0 pkt
>>>         mirred (Egress Redirect to device vxlan0)
>>>         Stats: used 119 sec      0 pkt
>> Can you send packet that match this offloaded flow to check it is real
>> offloaded?
>>
>> In the flowtable offload with my patches both TC_SETUP_BLOCK and
>> TC_SETUP_FT can offload the rule success
>>
>> But in the TC_SETUP_FT case the packet is not real offloaded.
>>
>>
>> I  will test like u did.
>>
>>>
>>>
>>>> -----Original Message-----
>>>> From: wenxu <wenxu@ucloud.cn>
>>>> Sent: Thursday, November 21, 2019 10:29 AM
>>>> To: Paul Blakey <paulb@mellanox.com>
>>>> Cc: pablo@netfilter.org; netdev@vger.kernel.org; Mark Bloch
>>>> <markb@mellanox.com>
>>>> Subject: Re: Question about flow table offload in mlx5e
>>>>
>>>>
>>>> On 11/21/2019 3:42 PM, Paul Blakey wrote:
>>>>> Hi,
>>>>>
>>>>> The original design was the block setup to use TC_SETUP_FT type, and
>> the
>>>> tc event type to be case TC_SETUP_CLSFLOWER.
>>>>> We will post a patch to change that. I would advise to wait till we fix that
>>>> 😊
>>>>> I'm not sure how you get to this function mlx5e_rep_setup_ft_cb() if it
>> the
>>>> nf_flow_table_offload ndo_setup_tc event was TC_SETUP_BLOCK, and
>> not
>>>> TC_SETUP_FT.
>>>>
>>>>
>>>> Yes I change the TC_SETUP_BLOCK to TC_SETUP_FT in the
>>>> nf_flow_table_offload_setup.
>>>>
>>>> Two fixes patch provide:
>>>>
>>>> http://patchwork.ozlabs.org/patch/1197818/
>>>>
>>>> http://patchwork.ozlabs.org/patch/1197876/
>>>>
>>>> So this change made by me is not correct currently?
>>>>
>>>>> In our driver en_rep.c we have:
>>>>>> -------switch (type) {
>>>>>> -------case TC_SETUP_BLOCK:
>>>>>> ------->-------return flow_block_cb_setup_simple(type_data,
>>>>>> ------->------->------->------->------->-------
>> &mlx5e_rep_block_tc_cb_list,
>>>>>> ------->------->------->------->------->-------  mlx5e_rep_setup_tc_cb,
>>>>>> ------->------->------->------->------->-------  priv, priv, true);
>>>>>> -------case TC_SETUP_FT:
>>>>>> ------->-------return flow_block_cb_setup_simple(type_data,
>>>>>> ------->------->------->------->------->-------
>> &mlx5e_rep_block_ft_cb_list,
>>>>>> ------->------->------->------->------->-------  mlx5e_rep_setup_ft_cb,
>>>>>> ------->------->------->------->------->-------  priv, priv, true);
>>>>>> -------default:
>>>>>> ------->-------return -EOPNOTSUPP;
>>>>>> -------}
>>>>> In nf_flow_table_offload.c:
>>>>>> -------bo.binder_type>-=
>> FLOW_BLOCK_BINDER_TYPE_CLSACT_INGRESS;
>>>>>> -------bo.extack>------= &extack;
>>>>>> -------INIT_LIST_HEAD(&bo.cb_list);
>>>>>> -------err = dev->netdev_ops->ndo_setup_tc(dev, TC_SETUP_BLOCK,
>>>> &bo);
>>>>>> -------if (err < 0)
>>>>>> ------->-------return err;
>>>>>> -------return nf_flow_table_block_setup(flowtable, &bo, cmd);
>>>>> }
>>>>> EXPORT_SYMBOL_GPL(nf_flow_table_offload_setup);
>>>>>
>>>>>
>>>>> So unless you changed that as well, you should have gotten to
>>>> mlx5e_rep_setup_tc_cb and not mlx5e_rep_setup_tc_ft.
>>>>> Regarding the encap action, there should be no difference on which
>> chain
>>>> the rule is on.
>>>>
>>>>
>>>> But for the same encap rule can be real offloaded when setup through
>>>> through TC_SETUP_BLOCK. But TC_SETUP_FT can't.
>>>>
>>>> So it is the problem of TC_SETUP_FT in mlx5e_rep_setup_ft_cb ?
>>>>
>>>>>> -----Original Message-----
>>>>>> From: wenxu <wenxu@ucloud.cn>
>>>>>> Sent: Thursday, November 21, 2019 9:30 AM
>>>>>> To: Paul Blakey <paulb@mellanox.com>
>>>>>> Cc: pablo@netfilter.org; netdev@vger.kernel.org; Mark Bloch
>>>>>> <markb@mellanox.com>
>>>>>> Subject: Question about flow table offload in mlx5e
>>>>>>
>>>>>> Hi  paul,
>>>>>>
>>>>>> The flow table offload in the mlx5e is based on TC_SETUP_FT.
>>>>>>
>>>>>>
>>>>>> It is almost the same as TC_SETUP_BLOCK.
>>>>>>
>>>>>> It just set MLX5_TC_FLAG(FT_OFFLOAD) flags and change
>>>>>> cls_flower.common.chain_index = FDB_FT_CHAIN;
>>>>>>
>>>>>> In following codes line 1380 and 1392
>>>>>>
>>>>>> 1368 static int mlx5e_rep_setup_ft_cb(enum tc_setup_type type, void
>>>>>> *type_data,
>>>>>> 1369                                  void *cb_priv)
>>>>>> 1370 {
>>>>>> 1371         struct flow_cls_offload *f = type_data;
>>>>>> 1372         struct flow_cls_offload cls_flower;
>>>>>> 1373         struct mlx5e_priv *priv = cb_priv;
>>>>>> 1374         struct mlx5_eswitch *esw;
>>>>>> 1375         unsigned long flags;
>>>>>> 1376         int err;
>>>>>> 1377
>>>>>> 1378         flags = MLX5_TC_FLAG(INGRESS) |
>>>>>> 1379                 MLX5_TC_FLAG(ESW_OFFLOAD) |
>>>>>> 1380                 MLX5_TC_FLAG(FT_OFFLOAD);
>>>>>> 1381         esw = priv->mdev->priv.eswitch;
>>>>>> 1382
>>>>>> 1383         switch (type) {
>>>>>> 1384         case TC_SETUP_CLSFLOWER:
>>>>>> 1385                 if (!mlx5_eswitch_prios_supported(esw) || f-
>>>>>>> common.chain_index)
>>>>>> 1386                         return -EOPNOTSUPP;
>>>>>> 1387
>>>>>> 1388                 /* Re-use tc offload path by moving the ft flow to the
>>>>>> 1389                  * reserved ft chain.
>>>>>> 1390                  */
>>>>>> 1391                 memcpy(&cls_flower, f, sizeof(*f));
>>>>>> 1392                cls_flower.common.chain_index = FDB_FT_CHAIN;
>>>>>> 1393                 err = mlx5e_rep_setup_tc_cls_flower(priv, &cls_flower,
>>>> flags);
>>>>>> 1394                 memcpy(&f->stats, &cls_flower.stats, sizeof(f->stats));
>>>>>>
>>>>>>
>>>>>> I want to add tunnel offload support in the flow table, I  add some
>> patches
>>>> in
>>>>>> nf_flow_table_offload.
>>>>>>
>>>>>> Also add the indr setup support in the mlx driver. And Now I can  flow
>>>> table
>>>>>> offload with decap.
>>>>>>
>>>>>>
>>>>>> But I meet a problem with the encap.  The encap rule can be added in
>>>>>> hardware  successfully But it can't be offloaded.
>>>>>>
>>>>>> But I think the rule I added is correct.  If I mask the line 1392. The rule
>> also
>>>> can
>>>>>> be add success and can be offloaded.
>>>>>>
>>>>>> So there are some limit for encap operation for FT_OFFLOAD in
>>>>>> FDB_FT_CHAIN?
>>>>>>
>>>>>>
>>>>>> BR
>>>>>>
>>>>>> wenxu
>>>>>>