Message ID | 20170316154038.GA11529@penelope.horms.nl |
---|---|
State | Not Applicable |
Headers | show |
On 16/03/2017 17:40, Simon Horman wrote: > On Mon, Mar 13, 2017 at 03:36:50PM +0200, Roi Dayan wrote: >> This patch series introduces rule offload functionality to dpif-netlink >> via netdev ports new flow offloading API. The user can specify whether to >> enable rule offloading or not via OVS configuration. Netdev providers >> are able to implement netdev flow offload API in order to offload rules. >> >> This patch series also implements one offload scheme for netdev-linux, >> using TC flower classifier, which was chosen because its sort of natural >> to state OVS DP rules for this classifier. However, the code can be >> extended to support other classifiers such as U32, eBPF, etc which support >> offload as well. >> >> The use-case we are currently addressing is the newly sriov switchdev mode >> in the Linux kernel which was introduced in version 4.8 [1][2]. >> This series was tested against sriov vfs vports representors of the >> Mellanox 100G ConnectX-4 series exposed by the mlx5 kernel driver. >> >> >> V3->V4: >> - Move declarations to the right commit with implementation >> - Fix tc_get_flower flow return false success >> - Fix memory leaks - not releasing tc_transact replies >> - Fix travis failure for OSX compilation >> - Fix loop in dpif_netlink_flow_dump_next >> - Fix declared default value for tc-policy in vswitch.xml >> - Refactor loop in netdev_tc_flow_dump_next >> - Add missing error checks in parse_flow_put >> - Fix handling modify request where old rule is in hw and new >> rule is not supported and needs to be in sw. >> - Use 2 hashmaps instead of 1 for faster reverse lookup of ufid from tc >> - Init ports when enabling hw-offload after OVS is running >> >> TODO: Fix breaking of datapath compilation >> Fix testsuite failures >> >> Travis >> https://travis-ci.org/roidayan/ovs/builds/210549325 >> AppVeyor >> https://ci.appveyor.com/project/roidayan/ovs/build/1.0.15 > > Hi Roi, > > we have found that in our testing it seems to resolve problems flagged by > Travis. I think that if this is the way forwards then the code could > be collapsed back into netdev_vport_get_ifindex() and the surrounding > #ifdef __linux__ logic could be removed. > thanks Simon! looks good. We'll send V5 with your patches and some other fixes we did. > diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c > index 4cc086e..d055d53 100644 > --- a/lib/netdev-vport.c > +++ b/lib/netdev-vport.c > @@ -783,7 +783,7 @@ get_stats(const struct netdev *netdev, struct netdev_stats *stats) > > #ifdef __linux__ > static int > -netdev_vport_get_ifindex(const struct netdev *netdev_) > +netdev_vport_get_ifindex__(const struct netdev *netdev_) > { > char buf[NETDEV_VPORT_NAME_BUFSIZE]; > const char *name = netdev_vport_get_dpif_port(netdev_, buf, sizeof(buf)); > @@ -791,6 +791,15 @@ netdev_vport_get_ifindex(const struct netdev *netdev_) > return linux_get_ifindex(name); > } > > +static int > +netdev_vport_get_ifindex(const struct netdev *netdev_) > +{ > + if (netdev_flow_api_enabled) > + return netdev_vport_get_ifindex__(netdev_); > + else > + return -EOPNOTSUPP; > +} > + > #define NETDEV_VPORT_GET_IFINDEX netdev_vport_get_ifindex > #define NETDEV_FLOW_OFFLOAD_API LINUX_FLOW_OFFLOAD_API > #else /* !__linux__ */ >
On 20/03/2017 10:33, Roi Dayan wrote: > > > On 16/03/2017 17:40, Simon Horman wrote: >> On Mon, Mar 13, 2017 at 03:36:50PM +0200, Roi Dayan wrote: >>> This patch series introduces rule offload functionality to dpif-netlink >>> via netdev ports new flow offloading API. The user can specify >>> whether to >>> enable rule offloading or not via OVS configuration. Netdev providers >>> are able to implement netdev flow offload API in order to offload rules. >>> >>> This patch series also implements one offload scheme for netdev-linux, >>> using TC flower classifier, which was chosen because its sort of natural >>> to state OVS DP rules for this classifier. However, the code can be >>> extended to support other classifiers such as U32, eBPF, etc which >>> support >>> offload as well. >>> >>> The use-case we are currently addressing is the newly sriov switchdev >>> mode >>> in the Linux kernel which was introduced in version 4.8 [1][2]. >>> This series was tested against sriov vfs vports representors of the >>> Mellanox 100G ConnectX-4 series exposed by the mlx5 kernel driver. >>> >>> >>> V3->V4: >>> - Move declarations to the right commit with implementation >>> - Fix tc_get_flower flow return false success >>> - Fix memory leaks - not releasing tc_transact replies >>> - Fix travis failure for OSX compilation >>> - Fix loop in dpif_netlink_flow_dump_next >>> - Fix declared default value for tc-policy in vswitch.xml >>> - Refactor loop in netdev_tc_flow_dump_next >>> - Add missing error checks in parse_flow_put >>> - Fix handling modify request where old rule is in hw and new >>> rule is not supported and needs to be in sw. >>> - Use 2 hashmaps instead of 1 for faster reverse lookup of ufid >>> from tc >>> - Init ports when enabling hw-offload after OVS is running >>> >>> TODO: Fix breaking of datapath compilation >>> Fix testsuite failures >>> >>> Travis >>> https://travis-ci.org/roidayan/ovs/builds/210549325 >>> AppVeyor >>> https://ci.appveyor.com/project/roidayan/ovs/build/1.0.15 >> >> Hi Roi, >> >> we have found that in our testing it seems to resolve problems flagged by >> Travis. I think that if this is the way forwards then the code could >> be collapsed back into netdev_vport_get_ifindex() and the surrounding >> #ifdef __linux__ logic could be removed. >> > > thanks Simon! looks good. > We'll send V5 with your patches and some other fixes we did. > > Hi Simon. I'm not sure we can get rid of the ifdef __linux__ because of linux_get_ifindex(). Also we are actually missing another ifdef __linux__ around the include of netdev-linux.h. Thanks, Roi >> diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c >> index 4cc086e..d055d53 100644 >> --- a/lib/netdev-vport.c >> +++ b/lib/netdev-vport.c >> @@ -783,7 +783,7 @@ get_stats(const struct netdev *netdev, struct >> netdev_stats *stats) >> >> #ifdef __linux__ >> static int >> -netdev_vport_get_ifindex(const struct netdev *netdev_) >> +netdev_vport_get_ifindex__(const struct netdev *netdev_) >> { >> char buf[NETDEV_VPORT_NAME_BUFSIZE]; >> const char *name = netdev_vport_get_dpif_port(netdev_, buf, >> sizeof(buf)); >> @@ -791,6 +791,15 @@ netdev_vport_get_ifindex(const struct netdev >> *netdev_) >> return linux_get_ifindex(name); >> } >> >> +static int >> +netdev_vport_get_ifindex(const struct netdev *netdev_) >> +{ >> + if (netdev_flow_api_enabled) >> + return netdev_vport_get_ifindex__(netdev_); >> + else >> + return -EOPNOTSUPP; >> +} >> + >> #define NETDEV_VPORT_GET_IFINDEX netdev_vport_get_ifindex >> #define NETDEV_FLOW_OFFLOAD_API LINUX_FLOW_OFFLOAD_API >> #else /* !__linux__ */ >>
On Mon, Mar 20, 2017 at 10:56:34AM +0200, Roi Dayan wrote: > > > On 20/03/2017 10:33, Roi Dayan wrote: > > > > > >On 16/03/2017 17:40, Simon Horman wrote: > >>On Mon, Mar 13, 2017 at 03:36:50PM +0200, Roi Dayan wrote: > >>>This patch series introduces rule offload functionality to dpif-netlink > >>>via netdev ports new flow offloading API. The user can specify > >>>whether to > >>>enable rule offloading or not via OVS configuration. Netdev providers > >>>are able to implement netdev flow offload API in order to offload rules. > >>> > >>>This patch series also implements one offload scheme for netdev-linux, > >>>using TC flower classifier, which was chosen because its sort of natural > >>>to state OVS DP rules for this classifier. However, the code can be > >>>extended to support other classifiers such as U32, eBPF, etc which > >>>support > >>>offload as well. > >>> > >>>The use-case we are currently addressing is the newly sriov switchdev > >>>mode > >>>in the Linux kernel which was introduced in version 4.8 [1][2]. > >>>This series was tested against sriov vfs vports representors of the > >>>Mellanox 100G ConnectX-4 series exposed by the mlx5 kernel driver. > >>> > >>> > >>>V3->V4: > >>> - Move declarations to the right commit with implementation > >>> - Fix tc_get_flower flow return false success > >>> - Fix memory leaks - not releasing tc_transact replies > >>> - Fix travis failure for OSX compilation > >>> - Fix loop in dpif_netlink_flow_dump_next > >>> - Fix declared default value for tc-policy in vswitch.xml > >>> - Refactor loop in netdev_tc_flow_dump_next > >>> - Add missing error checks in parse_flow_put > >>> - Fix handling modify request where old rule is in hw and new > >>> rule is not supported and needs to be in sw. > >>> - Use 2 hashmaps instead of 1 for faster reverse lookup of ufid > >>>from tc > >>> - Init ports when enabling hw-offload after OVS is running > >>> > >>> TODO: Fix breaking of datapath compilation > >>> Fix testsuite failures > >>> > >>> Travis > >>> https://travis-ci.org/roidayan/ovs/builds/210549325 > >>> AppVeyor > >>> https://ci.appveyor.com/project/roidayan/ovs/build/1.0.15 > >> > >>Hi Roi, > >> > >>we have found that in our testing it seems to resolve problems flagged by > >>Travis. I think that if this is the way forwards then the code could > >>be collapsed back into netdev_vport_get_ifindex() and the surrounding > >>#ifdef __linux__ logic could be removed. > >> > > > >thanks Simon! looks good. > >We'll send V5 with your patches and some other fixes we did. > > > > > > Hi Simon. > > I'm not sure we can get rid of the ifdef __linux__ because of > linux_get_ifindex(). Also we are actually missing another ifdef __linux__ > around the include of netdev-linux.h. I'm with the __linux__ bits if they are still needed.
diff --git a/lib/netdev-vport.c b/lib/netdev-vport.c index 4cc086e..d055d53 100644 --- a/lib/netdev-vport.c +++ b/lib/netdev-vport.c @@ -783,7 +783,7 @@ get_stats(const struct netdev *netdev, struct netdev_stats *stats) #ifdef __linux__ static int -netdev_vport_get_ifindex(const struct netdev *netdev_) +netdev_vport_get_ifindex__(const struct netdev *netdev_) { char buf[NETDEV_VPORT_NAME_BUFSIZE]; const char *name = netdev_vport_get_dpif_port(netdev_, buf, sizeof(buf)); @@ -791,6 +791,15 @@ netdev_vport_get_ifindex(const struct netdev *netdev_) return linux_get_ifindex(name); } +static int +netdev_vport_get_ifindex(const struct netdev *netdev_) +{ + if (netdev_flow_api_enabled) + return netdev_vport_get_ifindex__(netdev_); + else + return -EOPNOTSUPP; +} + #define NETDEV_VPORT_GET_IFINDEX netdev_vport_get_ifindex #define NETDEV_FLOW_OFFLOAD_API LINUX_FLOW_OFFLOAD_API #else /* !__linux__ */