| Message ID | 20200731025514.1669061-3-toshiaki.makita1@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | XDP offload using flow API provider | expand |
On Thu, Jul 30, 2020 at 7:55 PM Toshiaki Makita <toshiaki.makita1@gmail.com> wrote: > > The following commit will introduce another offload driver using XDP. > When using afxdp netdev, both of TC and XDP will be supported, so let's > add an other_config to specify which offload driver is preferable. > When not specified and multiple offload drivers can be used, TC will be > used if netdev supports it. > > Signed-off-by: Toshiaki Makita <toshiaki.makita1@gmail.com> > --- The implementation looks good to me. Please also add "offload-driver" to the vswitch.xml. About the interface, currently people are just setting $ ovs-vsctl set Open_vSwitch . other_config:hw-offload=true and assume tc hw offload for linux device and DPDK rte_flow for OVS-DPDK. Instead of boolean, another way is to $ ovs-vsctl set Open_vSwitch . other_config:hw-offload={tc, rte_flow, xdp} or just $ ovs-vsctl set Open_vSwitch . other_config:offload={tc, rte_flow, xdp} Let's wait for others feedback. William
On 2/4/2021 9:18 PM, William Tu wrote: > On Thu, Jul 30, 2020 at 7:55 PM Toshiaki Makita > <toshiaki.makita1@gmail.com> wrote: >> The following commit will introduce another offload driver using XDP. >> When using afxdp netdev, both of TC and XDP will be supported, so let's >> add an other_config to specify which offload driver is preferable. >> When not specified and multiple offload drivers can be used, TC will be >> used if netdev supports it. >> >> Signed-off-by: Toshiaki Makita <toshiaki.makita1@gmail.com> >> --- > The implementation looks good to me. Please also add "offload-driver" > to the vswitch.xml. > > About the interface, currently people are just setting > $ ovs-vsctl set Open_vSwitch . other_config:hw-offload=true > and assume tc hw offload for linux device and DPDK rte_flow for OVS-DPDK. > > Instead of boolean, another way is to > $ ovs-vsctl set Open_vSwitch . other_config:hw-offload={tc, rte_flow, xdp} > or just > $ ovs-vsctl set Open_vSwitch . other_config:offload={tc, rte_flow, xdp} > > Let's wait for others feedback. Those are global configs. Currently the flow_api is determined by the port, which can support mixed configurations (some ports with TC, and some with DPDK). Both this proposal and the one in this commit will remove this support. How about doing such "preferable" offload a port property (or at least bridge)? > William
On 2021/02/05 18:00, Eli Britstein wrote: > On 2/4/2021 9:18 PM, William Tu wrote: >> On Thu, Jul 30, 2020 at 7:55 PM Toshiaki Makita >> <toshiaki.makita1@gmail.com> wrote: >>> The following commit will introduce another offload driver using XDP. >>> When using afxdp netdev, both of TC and XDP will be supported, so let's >>> add an other_config to specify which offload driver is preferable. >>> When not specified and multiple offload drivers can be used, TC will be >>> used if netdev supports it. >>> >>> Signed-off-by: Toshiaki Makita <toshiaki.makita1@gmail.com> >>> --- >> The implementation looks good to me. Please also add "offload-driver" >> to the vswitch.xml. >> >> About the interface, currently people are just setting >> $ ovs-vsctl set Open_vSwitch . other_config:hw-offload=true >> and assume tc hw offload for linux device and DPDK rte_flow for OVS-DPDK. >> >> Instead of boolean, another way is to >> $ ovs-vsctl set Open_vSwitch . other_config:hw-offload={tc, rte_flow, xdp} >> or just >> $ ovs-vsctl set Open_vSwitch . other_config:offload={tc, rte_flow, xdp} >> >> Let's wait for others feedback. > > Those are global configs. Currently the flow_api is determined by the port, which > can support mixed configurations (some ports with TC, and some with DPDK). > > Both this proposal and the one in this commit will remove this support. How about > doing such "preferable" offload a port property (or at least bridge)? Hi, thanks for the feedback. Notice that the one in this commit does not remove mixed configurations support by default, i.e. if "offload-driver" is not specified, - TC is used when TC is supported (e.g. afxdp port) - rte_flow is used when TC is not supported but rte_flow is supported (dpdk port) But yes, I guess it makes more sense to have the config as a port or bridge property. Toshiaki Makita
diff --git a/lib/netdev-offload.c b/lib/netdev-offload.c index 2da3bc701..f9c0b0438 100644 --- a/lib/netdev-offload.c +++ b/lib/netdev-offload.c @@ -60,6 +60,9 @@ VLOG_DEFINE_THIS_MODULE(netdev_offload); static bool netdev_flow_api_enabled = false; +#define FLOW_API_DRIVER_DEFAULT "linux_tc" +static const char *netdev_flow_api_driver = NULL; + /* Protects 'netdev_flow_apis'. */ static struct ovs_mutex netdev_flow_api_provider_mutex = OVS_MUTEX_INITIALIZER; @@ -172,9 +175,38 @@ static int netdev_assign_flow_api(struct netdev *netdev) { struct netdev_registered_flow_api *rfa; + const char *flow_api_driver = netdev_flow_api_driver; + + if (!flow_api_driver) { + flow_api_driver = FLOW_API_DRIVER_DEFAULT; + } CMAP_FOR_EACH (rfa, cmap_node, &netdev_flow_apis) { + if (strcmp(flow_api_driver, rfa->flow_api->type)) { + continue; + } if (!rfa->flow_api->init_flow_api(netdev)) { + goto found; + } + VLOG_DBG("%s: flow API '%s' is not suitable.", + netdev_get_name(netdev), rfa->flow_api->type); + if (netdev_flow_api_driver) { + goto err; + } + break; + } + if (netdev_flow_api_driver) { + VLOG_DBG("%s: flow API '%s' is not found.", + netdev_get_name(netdev), netdev_flow_api_driver); + goto err; + } + + CMAP_FOR_EACH (rfa, cmap_node, &netdev_flow_apis) { + if (!strcmp(flow_api_driver, rfa->flow_api->type)) { + continue; + } + if (!rfa->flow_api->init_flow_api(netdev)) { +found: ovs_refcount_ref(&rfa->refcnt); ovsrcu_set(&netdev->flow_api, rfa->flow_api); VLOG_INFO("%s: Assigned flow API '%s'.", @@ -184,6 +216,7 @@ netdev_assign_flow_api(struct netdev *netdev) VLOG_DBG("%s: flow API '%s' is not suitable.", netdev_get_name(netdev), rfa->flow_api->type); } +err: VLOG_INFO("%s: No suitable flow API found.", netdev_get_name(netdev)); return -1; @@ -647,6 +680,8 @@ netdev_set_flow_api_enabled(const struct smap *ovs_other_config) static struct ovsthread_once once = OVSTHREAD_ONCE_INITIALIZER; if (ovsthread_once_start(&once)) { + const char *offload_driver; + netdev_flow_api_enabled = true; VLOG_INFO("netdev: Flow API Enabled"); @@ -660,6 +695,10 @@ netdev_set_flow_api_enabled(const struct smap *ovs_other_config) netdev_offload_rebalance_policy = true; } + offload_driver = smap_get_def(ovs_other_config, "offload-driver", + NULL); + netdev_flow_api_driver = nullable_xstrdup(offload_driver); + netdev_ports_flow_init(); ovsthread_once_done(&once);
The following commit will introduce another offload driver using XDP. When using afxdp netdev, both of TC and XDP will be supported, so let's add an other_config to specify which offload driver is preferable. When not specified and multiple offload drivers can be used, TC will be used if netdev supports it. Signed-off-by: Toshiaki Makita <toshiaki.makita1@gmail.com> --- lib/netdev-offload.c | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)