Re: [ovs-dev] [PATCH v2 0/6] Add support for ovs metering with tc offload
On Fri, 2021-09-24 at 15:51 +0200, Ilya Maximets wrote: > On 9/22/21 09:25, Roi Dayan wrote: > > > > > > On 2021-09-02 3:59 PM, Roi Dayan wrote: > > > Hi, > > > > > > This series is adding support for tc offloading of ovs metering. > > > The first 3 patches add lib support. > > > 4th patch adding tc police actions to reflect ovs metering. > > > 5th patch cleaning existing tc police actions on start to avoid > > > conflicts. > > > last patch is offloacing tc rules with the police actions. > > > > > > Thanks, > > > Roi > > > > > > > > > Notes: > > > v2 > > > - Move tc police parse call from last patch to 2nd patch. > > > 2nd patch is adding the parse lib func and add the call to > > > it in that patch. > > > Last patch is the put of tc police act. > > > In 2nd patch also add empty switch case for tc police act > > > put as the impl. > > > is done in the last patch when support for put is being > > > added. > > > > > > > > > > > > Jianbo Liu (6): > > > netdev-linux: Refactor put police action netlink message > > > tc: Add support parsing tc police action > > > netdev-linux: Add functions to manipulate tc police action > > > dpif-netlink: Offloading meter to tc police action > > > dpif-netlink: Cleanup police actions with reserved indexes on > > > startup > > > netdev-offload-tc: Offloading rules with police actions > > > > > > lib/dpif-netlink.c | 259 > > > ++- > > > lib/netdev-linux.c | 350 > > > ++-- > > > lib/netdev-linux.h | 9 ++ > > > lib/netdev-offload-tc.c | 11 ++ > > > lib/netdev-offload.h | 4 + > > > lib/tc.c | 103 ++ > > > lib/tc.h | 11 ++ > > > 7 files changed, 702 insertions(+), 45 deletions(-) > > > > > > > > > Hi, > > > > Just pinging about this series. adding support for metering for tc. > > Would like to get some initial comments/thoughts. > > Hi, Roi. Thanks for working on this! > I didn't look closely at the code, but I have one design comment. Thanks for your time to take a look at the patches. > > In theory it should be possible to use netdev-oddload-tc from the > userspace datapath (dpif-netdev). E.g. we may want to use it for > afxdp ports with skip_sw policy. > > I could be wrong, but at the quick glance it seems that some of > the important parts of meter offloading for tc are implemented > inside the lib/dpif-netlink and can not be re-used by the userspace > datapath and will need to be duplicated. If that's true, I think, > we need to move them to a lower netdev-offload-tc layer, if possible. > > You may also want to look at old patch set for meter offloading > in userspace datapath: > > https://patchwork.ozlabs.org/project/openvswitch/list/?series=150532 > I'm not saying that has a perfect API or that it can smoothly use > netdev-offload-tc, it definitely needs some changes. But it may > give some idea on what userspace datapath needs and what we need > to do in order to came up with a common dpif API that can be used > by both datapath implementations, and a common netdev-offload API > that also can be used from both of them. It's a good idea to have a common API for both datapaths. This patchset has been there for long time. I wonder why it was not merged, what did it miss? Could you please give me more guides before I continue to work on it? Thanks! Jianbo > > Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 0/6] Add support for ovs metering with tc offload
On 9/22/21 09:25, Roi Dayan wrote: > > > On 2021-09-02 3:59 PM, Roi Dayan wrote: >> Hi, >> >> This series is adding support for tc offloading of ovs metering. >> The first 3 patches add lib support. >> 4th patch adding tc police actions to reflect ovs metering. >> 5th patch cleaning existing tc police actions on start to avoid conflicts. >> last patch is offloacing tc rules with the police actions. >> >> Thanks, >> Roi >> >> >> Notes: >> v2 >> - Move tc police parse call from last patch to 2nd patch. >> 2nd patch is adding the parse lib func and add the call to it in that >> patch. >> Last patch is the put of tc police act. >> In 2nd patch also add empty switch case for tc police act put as the >> impl. >> is done in the last patch when support for put is being added. >> >> >> >> Jianbo Liu (6): >> netdev-linux: Refactor put police action netlink message >> tc: Add support parsing tc police action >> netdev-linux: Add functions to manipulate tc police action >> dpif-netlink: Offloading meter to tc police action >> dpif-netlink: Cleanup police actions with reserved indexes on startup >> netdev-offload-tc: Offloading rules with police actions >> >> lib/dpif-netlink.c | 259 ++- >> lib/netdev-linux.c | 350 >> ++-- >> lib/netdev-linux.h | 9 ++ >> lib/netdev-offload-tc.c | 11 ++ >> lib/netdev-offload.h | 4 + >> lib/tc.c | 103 ++ >> lib/tc.h | 11 ++ >> 7 files changed, 702 insertions(+), 45 deletions(-) >> > > > Hi, > > Just pinging about this series. adding support for metering for tc. > Would like to get some initial comments/thoughts. Hi, Roi. Thanks for working on this! I didn't look closely at the code, but I have one design comment. In theory it should be possible to use netdev-oddload-tc from the userspace datapath (dpif-netdev). E.g. we may want to use it for afxdp ports with skip_sw policy. I could be wrong, but at the quick glance it seems that some of the important parts of meter offloading for tc are implemented inside the lib/dpif-netlink and can not be re-used by the userspace datapath and will need to be duplicated. If that's true, I think, we need to move them to a lower netdev-offload-tc layer, if possible. You may also want to look at old patch set for meter offloading in userspace datapath: https://patchwork.ozlabs.org/project/openvswitch/list/?series=150532 I'm not saying that has a perfect API or that it can smoothly use netdev-offload-tc, it definitely needs some changes. But it may give some idea on what userspace datapath needs and what we need to do in order to came up with a common dpif API that can be used by both datapath implementations, and a common netdev-offload API that also can be used from both of them. Best regards, Ilya Maximets. ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Re: [ovs-dev] [PATCH v2 0/6] Add support for ovs metering with tc offload
On 2021-09-02 3:59 PM, Roi Dayan wrote: Hi, This series is adding support for tc offloading of ovs metering. The first 3 patches add lib support. 4th patch adding tc police actions to reflect ovs metering. 5th patch cleaning existing tc police actions on start to avoid conflicts. last patch is offloacing tc rules with the police actions. Thanks, Roi Notes: v2 - Move tc police parse call from last patch to 2nd patch. 2nd patch is adding the parse lib func and add the call to it in that patch. Last patch is the put of tc police act. In 2nd patch also add empty switch case for tc police act put as the impl. is done in the last patch when support for put is being added. Jianbo Liu (6): netdev-linux: Refactor put police action netlink message tc: Add support parsing tc police action netdev-linux: Add functions to manipulate tc police action dpif-netlink: Offloading meter to tc police action dpif-netlink: Cleanup police actions with reserved indexes on startup netdev-offload-tc: Offloading rules with police actions lib/dpif-netlink.c | 259 ++- lib/netdev-linux.c | 350 ++-- lib/netdev-linux.h | 9 ++ lib/netdev-offload-tc.c | 11 ++ lib/netdev-offload.h| 4 + lib/tc.c| 103 ++ lib/tc.h| 11 ++ 7 files changed, 702 insertions(+), 45 deletions(-) Hi, Just pinging about this series. adding support for metering for tc. Would like to get some initial comments/thoughts. Thanks, Roi ___ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
