Hi Ilya, 
was curious myself. Mark & RSS is working  (didn't test with representors yet). 
I've tested offload is supported on vport using !has_system_port, do you think 
failing in this test is enough to say that this is netdev bridge port?
When I returned supported, "dpdk_put" was called as expected (after removing 
the disallow).

One thing I've encounters is while addin a tap to the dpdk bridge. I got this:

2019-06-06T10:18:21.865Z|00055|netdev_offload_tc|INFO|probe tc: block offload 
is supported.
2019-06-06T10:18:21.866Z|00056|netdev_offload_tc|INFO|added ingress qdisc to 
veth0
2019-06-06T10:18:21.866Z|00057|netdev_offload|INFO|veth0: Assigned flow API 
'linux_tc'.
2019-06-06T10:18:21.866Z|00058|bridge|INFO|bridge br-int: added interface veth0 
on port 1
2019-06-06T10:18:21.867Z|00059|bridge|INFO|bridge br-phy: added interface 
br-phy on port 65534

Seems that we should block this as well. 

BR,
Roni

>-----Original Message-----
>From: Ilya Maximets <[email protected]>
>Sent: Monday, June 3, 2019 6:30 PM
>To: Ophir Munk <[email protected]>; Roi Dayan <[email protected]>;
>[email protected]
>Cc: Ian Stokes <[email protected]>; Flavio Leitner <[email protected]>; 
>Kevin
>Traynor <[email protected]>; Roni Bar Yanai <[email protected]>; Finn
>Christensen <[email protected]>; Ben Pfaff <[email protected]>; Simon Horman
><[email protected]>; Olga Shern <[email protected]>; Asaf
>Penso <[email protected]>; Majd Dibbiny <[email protected]>; Oz Shlomo
><[email protected]>
>Subject: Re: [PATCH v4 1/4] netdev: Dynamic per-port Flow API.
>
>Hi Ophir.
>I'm curious, what is the current status of your testing?
>
>I'd like to apply this series to not block further development.
>
>Best regards, Ilya Maximets.
>
>On 23.05.2019 16:54, Ilya Maximets wrote:
>> On 23.05.2019 16:53, Ilya Maximets wrote:
>>> On 23.05.2019 16:18, Ophir Munk wrote:
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: Ilya Maximets <[email protected]>
>>>>> Sent: Wednesday, May 22, 2019 2:50 PM
>>>>> To: Ophir Munk <[email protected]>; Roi Dayan
>>>>> <[email protected]>; [email protected]
>>>>> Cc: Ian Stokes <[email protected]>; Flavio Leitner <[email protected]>;
>>>>> Kevin Traynor <[email protected]>; Roni Bar Yanai
>>>>> <[email protected]>; Finn Christensen <[email protected]>; Ben Pfaff
>>>>> <[email protected]>; Simon Horman <[email protected]>; Olga
>>>>> Shern <[email protected]>; Asaf Penso <[email protected]>; Majd
>>>>> Dibbiny <[email protected]>
>>>>> Subject: Re: [PATCH v4 1/4] netdev: Dynamic per-port Flow API.
>>>>>
>>>>>
>>>>>
>>>>> On 22.05.2019 13:15, Ilya Maximets wrote:
>>>>>> On 22.05.2019 1:12, Ophir Munk wrote:
>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Roi Dayan
>>>>>>>> Sent: Tuesday, May 21, 2019 7:48 PM
>>>>>>>> To: Ilya Maximets <[email protected]>; ovs-
>>>>> [email protected]
>>>>>>>> Cc: Ian Stokes <[email protected]>; Flavio Leitner
>>>>>>>> <[email protected]>; Ophir Munk <[email protected]>; Kevin
>>>>> Traynor
>>>>>>>> <[email protected]>; Roni Bar Yanai <[email protected]>; Finn
>>>>>>>> Christensen <[email protected]>; Ben Pfaff <[email protected]>; Simon
>>>>> Horman
>>>>>>>> <[email protected]>
>>>>>>>> Subject: Re: [PATCH v4 1/4] netdev: Dynamic per-port Flow API.
>>>>>>>>
>>>>>>>>
>>>>>>>> Acked-by: Roi Dayan <[email protected]>
>>>>>>>
>>>>>>> Hi Ilya,
>>>>>>> Can you please send a patch for the detection of netdev vport on top of
>>>>> this series (as you have already started suggesting in ML discussions)?
>>>>>>> I will then test it and will make sure it's applicable with this 
>>>>>>> series. I think it
>>>>> is better to do that before series acceptance.
>>>>>>> What do you think?
>>>>>>
>>>>>> Hi.
>>>>>> Actually patches are already on a list. You only need to add few lines
>>>>>> to make them allow vxlan for netdev-offload-dpdk.
>>>>>>
>>>>>> Apply following patch sets on top of this one:
>>>>>>
>>>>>>
>https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork
>.ozlabs.org%2Fproject%2Fopenvswitch%2Flist%2F%3Fseries%3D107534&amp;dat
>a=02%7C01%7Croniba%40mellanox.com%7C90cd4c9ba95b4629884f08d6e8384c41
>%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636951725824527442&am
>p;sdata=n%2FVMz34ICfk0dvGOP77Ip4L008RRKdraRXMG%2FtFRM64%3D&amp;re
>served=0
>>>>
>>>> FYI - applying this patch succeeded, however applying the next patch failed
>unless I applied
>>>> patches 2/4, 3/4/ 4/4 first and only then I applied the next patch.
>>>
>>> Yes. That is expected.
>>>
>>>>
>>>>>>
>https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork
>.ozlabs.org%2Fproject%2Fopenvswitch%2Flist%2F%3Fseries%3D107545&amp;dat
>a=02%7C01%7Croniba%40mellanox.com%7C90cd4c9ba95b4629884f08d6e8384c41
>%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636951725824527442&am
>p;sdata=pfTmeLEuMW1DyJck3T%2BypfDI7ZlZ7K2%2FO6810tis4PQ%3D&amp;reser
>ved=0
>>>>>>
>>>>>>
>>>>>> Change below should than allow you to use dpdk offloading for vxlan 
>>>>>> ports:
>>>>
>>>> Why do you want to use dpdk offloading for vxlan ports?
>>>
>>> Sorry for misunderstanding, but I thought that you're implementing
>>> vxlan offloading as part of dpdk offloading. If it'll be a separate
>>> module, it's even better.
>>>
>>>> We need to use vport-netdev offloading for vxlan-netdev ports.
>>>> We need to use dpdk offloading for dpdk ports.
>>>> Vxlan-netdev offloading and dpdk offloading have a different implementation
>>>> (unlike the system case where vxlan-system offloading and system offloading
>are identical).
>>>>
>>>> I see four required offloading APIs:
>>>> 1. system
>>>> 2. dpdk
>>>> 3. vport under system (currently it is identical to system API)
>>>> 4. New vport under netdev.
>>>>
>>>> The first three APIs exist. The last (vxlan-netdev) will be sent soon.
>>>>
>>>> I see two options for adding vxlan-netdev API.
>>>> 1. Create a new dedicated vport-netdev offload class.
>>>>
>>>> 2. Having vport-netdev API to be identical to dpdk API but since the
>implementations are different we will have to know the type ("dpdk" versus
>"vxlan").
>>>> In pseudo code:
>>>> If (type=="dpdk")
>>>> {
>>>>    // handle dpdk offloading
>>>> }
>>>> If (type=="vxlan")
>>>> {
>>>>    // handle vxlan offloading
>>>> }
>>>>
>>>> I prefer the first option.
>>>
>>> Yes. Sure. I alse prefer the separate class if it has separate 
>>> implementation
>>> anyway.
>>>
>>>
>>> So, with all above patches applied you just need to make a new file:
>>> netdev-offload-vport-dpdk.c:
>>>
>>> <...>
>>> static int netdev_offload_vport_dpdk_flow_put(...)
>>> {
>>>     ...
>>> }
>>>
>>> static int netdev_offload_vport_dpdk_flow_del(...)
>>> {
>>>     ...
>>> }
>>>
>>> static int
>>> netdev_offload_vport_dpdk_init_flow_api(struct netdev *netdev)
>>> {
>>>     if (netdev_vport_is_vport_class(netdev->netdev_class)
>>>         && netdev_vport_has_system_port(netdev)) {
>>>         VLOG_DBG("%s: vport has backing system interface. Skipping.",
>>>                  netdev_get_name(netdev));
>>>         return EOPNOTSUPP;
>>>     }
>>>     return strcmp(netdev_get_type(netdev), "vxlan");
>>> }
>>>
>>> const struct netdev_flow_api netdev_offload_vport_dpdk = {
>>>     .type = "dpdk_flow_api",
>>
>> s/type = "dpdk_flow_api"/type = "vport_dpdk_flow_api"/
>>
>>>     .flow_put = netdev_offload_vport_dpdk_flow_put,
>>>     .flow_del = netdev_offload_vport_dpdk_flow_del,
>>>     .init_flow_api = netdev_offload_vport_dpdk_init_flow_api,
>>> };
>>>
>>>
>>> And add following line to lib/dpdk.c:
>>>
>>> netdev_register_flow_api_provider(&netdev_offload_vport_dpdk);
>>>
>>>
>>> And following to lib/netdev-offload-provider.h:
>>>
>>> extern const struct netdev_flow_api netdev_offload_vport_dpdk;
>>>
>>>>
>>>>>> diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c
>>>>>> index b7b0616ec..32f23c401 100644
>>>>>> --- a/lib/netdev-offload-dpdk.c
>>>>>> +++ b/lib/netdev-offload-dpdk.c
>>>>>> @@ -760,6 +760,10 @@ netdev_offload_dpdk_init_flow_api(struct netdev
>>>>> *netdev)
>>>>>>          return EOPNOTSUPP;
>>>>>>      }
>>>>>>
>>>>>> +    if (!strcmp(netdev_get_name(netdev), "vxlan")) {
>>>>>
>>>>> Sorry,
>>>>> s/netdev_get_name/netdev_get_type/
>>>>>
>>>>>> +        return 0;
>>>>
>>>> Having said all the above - we still need a way to correctly select between
>vport-netdev API versus vport-system API.
>>>> Reading your suggestion I am still not sure we have a solution here. Say we
>have a system bridge and a netdev bridge both with a vxlan port.
>>>> When the vxlan-netdev is checked first by the system-init API it will pass 
>>>> the
>checking and it will be added as a vxlan-system. Right?
>>>
>>> No. See the 'netdev_vport_has_system_port' function from patch
>>> "[RFC 2/2] netdev-offload: Disallow offloading to unrelated vports.".
>>>
>>>> Can you please advise?
>>>
>>> See the code snippet above.
>>>
>>> Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to