| Message ID | 20220921155341.547711-1-rjarry@redhat.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [ovs-dev,RFC] netdev-dpdk: add control plane protection support | expand |
| Context | Check | Description |
|---|---|---|
| ovsrobot/apply-robot | warning | apply and check: warning |
| ovsrobot/github-robot-_Build_and_Test | fail | github build: failed |
| ovsrobot/intel-ovs-compilation | success | test: success |
Bleep bloop. Greetings Robin Jarry, I am a robot and I have tried out your patch.
Thanks for your contribution.
I encountered some error that I wasn't expecting. See the details below.
checkpatch:
WARNING: Line is 80 characters long (recommended limit is 79)
#133 FILE: Documentation/topics/dpdk/phy.rst:138:
engines. In SDN environments, these packets share the same physical network than
ERROR: Use ovs_strzcpy() in place of strncpy()
#228 FILE: lib/netdev-dpdk.c:1952:
strncpy(buf, arg, sizeof(buf));
Lines checked: 594, Warnings: 1, Errors: 1
Please check this out. If you feel there has been an error, please email aconole@redhat.com
Thanks,
0-day Robot
Hi Robin, Nice! I have been also working on a similar line but in a different context of steering the flows toward the right RXQ based on the guest MAC-address. Will send out a patch in a day or two. Please find some comments inline for my understanding. Thanks Thilak Raj S -----Original Message----- From: dev <ovs-dev-bounces@openvswitch.org> On Behalf Of Robin Jarry Sent: 21 September 2022 08:54 To: dev@openvswitch.org Subject: [ovs-dev] [RFC PATCH] netdev-dpdk: add control plane protection support Some control protocols are used to maintain link status between forwarding engines (e.g. LACP). When the system is not sized properly, the PMD threads may not be able to process all incoming traffic from the configured Rx queues. When a signaling packet of such protocols is dropped, it can cause link flapping, worsening the situation. Use the RTE flow API to redirect these protocols into a dedicated Rx queue. The assumption is made that the ratio between control protocol traffic and user data traffic is very low and thus this dedicated Rx queue will never get full. Regular RSS is done on all other Rx queues. The additional Rx queue will be assigned a PMD core like any other Rx queue. Polling that extra queue may introduce increased latency and a slight performance penalty at the benefit of preventing link flapping. This feature must be enabled per port on specific protocols via the cp-protection option. This option takes a coma-separated list of protocol names. Example: ovs-vsctl add-bond br-phy bond0 phy0 phy1 -- \ set interface phy0 type=dpdk options:dpdk-devargs=0000:ca:00.0 -- \ set interface phy0 options:cp-protection=lacp -- \ set interface phy1 type=dpdk options:dpdk-devargs=0000:ca:00.1 -- \ set interface phy1 options:cp-protection=lacp As a starting point, only one protocol is supported: LACP. Other protocols can be added in the future. NIC compatibility should be checked. To validate that this works as intended, I used a traffic generator to generate random traffic slightly above the machine capacity at line rate on a two ports bond interface. OVS is configured to receive traffic on two VLANs and pop/push them in a br-int bridge based on tags set on patch ports. +----------------------+ | DUT | |+--------------------+| || br-int || default flow, action=NORMAL || || || patch10 patch11 || |+---|-----------|----+| | | | | |+---|-----------|----+| || patch00 patch01 || || tag:10 tag:20 || || || || br-phy || default flow, action=NORMAL || || || bond0 || balance-slb, lacp=passive, lacp-time=fast || phy0 phy1 || |+------|-----|-------+| +-------|-----|--------+ | | +-------|-----|--------+ | port0 port1 | balance L3/L4, lacp=active, lacp-time=fast | lag | mode trunk VLANs 10, 20 | | | switch | | | | vlan 10 vlan 20 | mode access | port2 port3 | +-----|----------|-----+ | | +-----|----------|-----+ | port0 port1 | Random traffic that is properly balanced | | across the bond ports in both directions. | traffic generator | +----------------------+ Without cp-protection, the bond0 links are randomly switching to "defaulted" when one of the LACP packets sent by the switch is dropped because the RX queues are full and the PMD threads did not process them fast enough. When that happens, all traffic must go through a single link which causes above line rate traffic to be dropped. When cp-protection is enabled, no LACP packet is dropped and the bond links remain enabled at all times, maximizing the throughput. Notes: * This feature may be considered as "QoS". However, it does not work by limiting the rate of traffic explicitly. It only guarantees that some protocols have a lower chance of being dropped because the PMD cores cannot keep up with regular traffic. * The choice of protocols is limited on purpose. This is not meant to be configurable by users. Some limited configurability could be considered in the future but it would expose to more potential issues if users are accidentally redirecting all traffic in the control plane queue. * Encapsulated traffic (into VXLAN, GRE, Geneve) is not redirected into that extra queue. This may be addressed in the future as well. * For now, I validated that this works on Mellanox Connect-X 5 and Intel E810 NICs. Intel X710 does not support the match all flows. Validation on other NICs should be done. * Unit tests may not be possible here. The netdev-dummy driver does not share code with netdev-dpdk. Cc: Christophe Fontaine <cfontain@redhat.com> Cc: Kevin Traynor <ktraynor@redhat.com> Signed-off-by: Robin Jarry <rjarry@redhat.com> --- Documentation/topics/dpdk/phy.rst | 41 ++++++ lib/netdev-dpdk.c | 235 ++++++++++++++++++++++++++++++ lib/netdev-dpdk.h | 5 + lib/netdev-offload-dpdk.c | 18 ++- lib/netdev-provider.h | 7 + 5 files changed, 302 insertions(+), 4 deletions(-) diff --git a/Documentation/topics/dpdk/phy.rst b/Documentation/topics/dpdk/phy.rst index 937f4c40e5a8..00b3887c70bd 100644 --- a/Documentation/topics/dpdk/phy.rst +++ b/Documentation/topics/dpdk/phy.rst @@ -131,6 +131,47 @@ possible with DPDK acceleration. It is possible to configure multiple Rx queues for ``dpdk`` ports, thus ensuring this is not a bottleneck for performance. For information on configuring PMD threads, refer to :doc:`pmd`. +Control Plane Protection +------------------------ + +Some control protocols are used to maintain link status between +forwarding engines. In SDN environments, these packets share the same +physical network than the user data traffic. + +When the system is not sized properly, the PMD threads may not be able +to process all incoming traffic from the configured Rx queues. When a +signaling packet of such protocols is dropped, it can cause link +flapping, worsening the situation. + +Some physical NICs can be programmed to put these protocols in a +dedicated hardware Rx queue using the rte_flow__ API. + +__ +https://urldefense.proofpoint.com/v2/url?u=https-3A__doc.dpdk.org_guide +s_prog-5Fguide_rte-5Fflow.html-23device-2Dcompatibility&d=DwICAg&c=s883 +GpUCOChKOHiocYtGcg&r=1sd3waKor_ps6hs2j0tfqmW6ts2tlVvmmMySlXCPN6w&m=5cLM +3svbMnJH20rf7TpgzWWb6vD5fzG5FzBrcLzoi5tFOQYH3E6Qp-28YTGdgmuH&s=2NmhHktC +wsKIP1r_TcVVYGgoHOIqh69tdH-dDyWjGiw&e= + +The currently supported control plane protocols are: + +``lacp`` + `Link Aggregation Control Protocol`__. Ether type ``0x8809``. + + __ + https://urldefense.proofpoint.com/v2/url?u=https-3A__www.ieee802.org_3 + _ad_public_mar99_seaman-5F1-5F0399.pdf&d=DwICAg&c=s883GpUCOChKOHiocYtG + cg&r=1sd3waKor_ps6hs2j0tfqmW6ts2tlVvmmMySlXCPN6w&m=5cLM3svbMnJH20rf7Tp + gzWWb6vD5fzG5FzBrcLzoi5tFOQYH3E6Qp-28YTGdgmuH&s=ZIT9Uudb7uyDa4E02yYI9g + ABm9fFkupBmy2M_7sLXu4&e= + +.. warning:: + + This feature is not compatible with all NICs. Refer to vendor documentation + for more information. + +Control plane protection must be enabled on specific protocols per +port. The ``cp-protection`` option requires a coma separated list of protocol names:: + + $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \ + options:dpdk-devargs=0000:01:00.0 options:cp-protection=lacp + +.. note:: + + If multiple Rx queues are already configured, regular RSS (Receive Side + Scaling) queue balancing is done on all but the extra control plane + protection queue. + .. _dpdk-phy-flow-control: Flow Control diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 0dd655507b50..033ba3e5ff6f 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -529,6 +529,12 @@ struct netdev_dpdk { /* VF configuration. */ struct eth_addr requested_hwaddr; + + /* Requested control plane protection flags, + * from the enum set 'netdev_cp_prot_flags' */ + uint64_t requested_cp_prot_flags; + size_t cp_prot_flows_num; + struct rte_flow **cp_prot_flows; ); PADDED_MEMBERS(CACHE_LINE_SIZE, @@ -1305,10 +1311,14 @@ common_construct(struct netdev *netdev, dpdk_port_t port_no, netdev->n_rxq = 0; netdev->n_txq = 0; + netdev->cp_prot_flags = 0; dev->requested_n_rxq = NR_QUEUE; dev->requested_n_txq = NR_QUEUE; dev->requested_rxq_size = NIC_PORT_DEFAULT_RXQ_SIZE; dev->requested_txq_size = NIC_PORT_DEFAULT_TXQ_SIZE; + dev->requested_cp_prot_flags = 0; + dev->cp_prot_flows_num = 0; + dev->cp_prot_flows = NULL; /* Initialize the flow control to NULL */ memset(&dev->fc_conf, 0, sizeof dev->fc_conf); @@ -1904,6 +1914,9 @@ dpdk_set_rxq_config(struct netdev_dpdk *dev, const struct smap *args) int new_n_rxq; new_n_rxq = MAX(smap_get_int(args, "n_rxq", NR_QUEUE), 1); + if (dev->requested_cp_prot_flags) { + new_n_rxq += 1; + } if (new_n_rxq != dev->requested_n_rxq) { dev->requested_n_rxq = new_n_rxq; netdev_request_reconfigure(&dev->up); @@ -1927,6 +1940,39 @@ dpdk_process_queue_size(struct netdev *netdev, const struct smap *args, } } +static int +dpdk_cp_prot_set_config(struct netdev *netdev, struct netdev_dpdk *dev, + const struct smap *args, char **errp) { + const char *arg = smap_get_def(args, "cp-protection", ""); + uint64_t flags = 0; + char buf[256]; + char *token, *saveptr; + + strncpy(buf, arg, sizeof(buf)); + buf[sizeof(buf) - 1] = '\0'; + + token = strtok_r(buf, ",", &saveptr); + while (token) { + if (strcmp(token, "lacp") == 0) { + flags |= NETDEV_CP_PROT_LACP; + } else { + VLOG_WARN_BUF( + errp, "%s options:cp-protection=%s unknown protocol '%s'", + netdev_get_name(netdev), arg, token); + return -1; + } + token = strtok_r(NULL, ",", &saveptr); + } + + if (flags != dev->requested_cp_prot_flags) { + dev->requested_cp_prot_flags = flags; + netdev_request_reconfigure(netdev); + } + + return 0; +} + static int netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, char **errp) @@ -1946,6 +1992,11 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, ovs_mutex_lock(&dpdk_mutex); ovs_mutex_lock(&dev->mutex); + if (dpdk_cp_prot_set_config(netdev, dev, args, errp) < 0) { + err = EINVAL; + goto out; + } + dpdk_set_rxq_config(dev, args); dpdk_process_queue_size(netdev, args, "n_rxq_desc", @@ -4933,6 +4984,179 @@ static const struct dpdk_qos_ops trtcm_policer_ops = { .qos_queue_dump_state_init = trtcm_policer_qos_queue_dump_state_init }; +static int +dpdk_cp_prot_add_flow(struct netdev_dpdk *dev, + const struct rte_flow_attr *attr, + const struct rte_flow_item items[], + const struct rte_flow_action actions[], + const char *desc) { + struct rte_flow_error error; + struct rte_flow *flow; + size_t num; + Could we call rte_flow_validate() before calling flow_create ? + flow = rte_flow_create(dev->port_id, attr, items, actions, &error); + if (flow == NULL) { + VLOG_WARN("%s: cp-protection: failed to add %s flow: %s", + netdev_get_name(&dev->up), desc, error.message); + return rte_errno; + } + + num = dev->cp_prot_flows_num + 1; + dev->cp_prot_flows = xrealloc(dev->cp_prot_flows, sizeof(flow) * num); + dev->cp_prot_flows[dev->cp_prot_flows_num] = flow; + dev->cp_prot_flows_num = num; + + return 0; +} + +static int +dpdk_cp_prot_add_traffic_flow(struct netdev_dpdk *dev, + const struct rte_flow_item items[], + const char *desc) { + const struct rte_flow_attr attr = { + .group = 0, + .priority = NETDEV_DPDK_FLOW_PRIORITY_MAX, + .ingress = 1, + .egress = 0, + .transfer = 0, + }; + const struct rte_flow_action actions[] = { + { + .type = RTE_FLOW_ACTION_TYPE_QUEUE, + .conf = &(const struct rte_flow_action_queue) { + .index = dev->up.n_rxq - 1, + }, + }, + { .type = RTE_FLOW_ACTION_TYPE_END }, + }; + + VLOG_INFO("%s: cp-protection: redirecting %s traffic to queue %d", + netdev_get_name(&dev->up), desc, dev->up.n_rxq - 1); + return dpdk_cp_prot_add_flow(dev, &attr, items, actions, desc); } + +static int +dpdk_cp_prot_add_rss_flow(struct netdev_dpdk *dev) { + int rss_n_rxq = dev->up.n_rxq - 1; + const struct rte_flow_attr attr = { + .group = 0, + .priority = NETDEV_DPDK_FLOW_PRIORITY_FALLBACK, + .ingress = 1, + .egress = 0, + .transfer = 0, + }; + const struct rte_flow_item items[] = { + { .type = RTE_FLOW_ITEM_TYPE_ANY }, + { .type = RTE_FLOW_ITEM_TYPE_END }, + }; + + if (rss_n_rxq == 1) { + const struct rte_flow_action actions[] = { + { + .type = RTE_FLOW_ACTION_TYPE_QUEUE, + .conf = &(const struct rte_flow_action_queue) { + .index = 0, + }, + }, + { .type = RTE_FLOW_ACTION_TYPE_END }, + }; + + VLOG_INFO("%s: cp-protection: redirecting other traffic to queue 0", + netdev_get_name(&dev->up)); + return dpdk_cp_prot_add_flow(dev, &attr, items, actions, "other"); + } + + uint16_t rss_queues[rss_n_rxq]; + char buf[128]; + buf[0] = '\0'; + for (int i = 0; i < rss_n_rxq; i++) { + rss_queues[i] = i; + snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf) - 1, " %d", i); + } + buf[sizeof(buf) - 1] = '\0'; + const struct rte_flow_action actions[] = { + { + .type = RTE_FLOW_ACTION_TYPE_RSS, + .conf = &(const struct rte_flow_action_rss){ + .func = RTE_ETH_HASH_FUNCTION_DEFAULT, + .level = 0, + .types = RTE_ETH_RSS_IP | RTE_ETH_RSS_UDP | RTE_ETH_RSS_TCP, + .queue_num = rss_n_rxq, + .queue = rss_queues, + .key_len = 0, + .key = NULL, + }, + }, + { .type = RTE_FLOW_ACTION_TYPE_END }, + }; + + VLOG_INFO("%s: cp-protection: applying rss on queues:%s", + netdev_get_name(&dev->up), buf); + return dpdk_cp_prot_add_flow(dev, &attr, items, actions, "rss"); } + +static int +dpdk_cp_prot_configure(struct netdev_dpdk *dev) { + int err = 0; + + if (dev->requested_cp_prot_flags & NETDEV_CP_PROT_LACP) { + err = dpdk_cp_prot_add_traffic_flow( + dev, + (const struct rte_flow_item []) { + { + .type = RTE_FLOW_ITEM_TYPE_ETH, + .spec = &(const struct rte_flow_item_eth){ + .type = htons(ETH_TYPE_LACP), + }, + .mask = &(const struct rte_flow_item_eth){ + .type = htons(0xffff), + }, + }, + { .type = RTE_FLOW_ITEM_TYPE_END }, + }, + "lacp" + ); + if (err) { + goto out; + } + } + + /* default flow is RSS in all but the cp protection queue */ + if (dev->requested_cp_prot_flags) { Can we not use isolate the Queue using rte_flow_isolate()? + err = dpdk_cp_prot_add_rss_flow(dev); + } + +out: + dev->up.cp_prot_flags = dev->requested_cp_prot_flags; + return err; +} + +static void +dpdk_cp_prot_unconfigure(struct netdev_dpdk *dev) { + struct rte_flow_error error; + + if (dev->cp_prot_flows_num == 0) { + return; + } + + VLOG_INFO("%s: cp-protection: reset flows", + netdev_get_name(&dev->up)); + + for (int i = 0; i < dev->cp_prot_flows_num; i++) { + if (rte_flow_destroy(dev->port_id, dev->cp_prot_flows[i], &error)) { + VLOG_DBG("%s: cp-protection: failed to destroy flow: %s", + netdev_get_name(&dev->up), error.message); + } + } + free(dev->cp_prot_flows); + dev->cp_prot_flows_num = 0; + dev->cp_prot_flows = NULL; +} + static int netdev_dpdk_reconfigure(struct netdev *netdev) { @@ -4943,6 +5167,7 @@ netdev_dpdk_reconfigure(struct netdev *netdev) if (netdev->n_txq == dev->requested_n_txq && netdev->n_rxq == dev->requested_n_rxq + && netdev->cp_prot_flags == dev->requested_cp_prot_flags && dev->mtu == dev->requested_mtu && dev->lsc_interrupt_mode == dev->requested_lsc_interrupt_mode && dev->rxq_size == dev->requested_rxq_size @@ -4987,6 +5212,8 @@ netdev_dpdk_reconfigure(struct netdev *netdev) } } + dpdk_cp_prot_unconfigure(dev); + err = dpdk_eth_dev_init(dev); if (dev->hw_ol_features & NETDEV_TX_TSO_OFFLOAD) { netdev->ol_flags |= NETDEV_TX_OFFLOAD_TCP_TSO; @@ -5014,6 +5241,14 @@ netdev_dpdk_reconfigure(struct netdev *netdev) if (!dev->tx_q) { err = ENOMEM; } + if (!err && netdev->n_rxq > 1) { + err = dpdk_cp_prot_configure(dev); + if (err) { + /* no hardware support, recover gracefully */ + dpdk_cp_prot_unconfigure(dev); + err = 0; + } + } netdev_change_seq_changed(netdev); diff --git a/lib/netdev-dpdk.h b/lib/netdev-dpdk.h index 7d2f64af23e1..24acdc7b193c 100644 --- a/lib/netdev-dpdk.h +++ b/lib/netdev-dpdk.h @@ -17,6 +17,7 @@ #ifndef NETDEV_DPDK_H #define NETDEV_DPDK_H +#include <stdint.h> #include <config.h> #include "openvswitch/compiler.h" @@ -32,6 +33,10 @@ struct netdev; void netdev_dpdk_register(const struct smap *); void free_dpdk_buf(struct dp_packet *); +#define NETDEV_DPDK_FLOW_PRIORITY_MAX 0 #define +NETDEV_DPDK_FLOW_PRIORITY_OFFLOAD 10 #define +NETDEV_DPDK_FLOW_PRIORITY_FALLBACK UINT32_MAX + bool netdev_dpdk_flow_api_supported(struct netdev *); int diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c index cceefbc50751..f19db670d56f 100644 --- a/lib/netdev-offload-dpdk.c +++ b/lib/netdev-offload-dpdk.c @@ -1702,6 +1702,7 @@ add_flow_mark_rss_actions(struct flow_actions *actions, uint16_t queue[0]; } *rss_data; BUILD_ASSERT_DECL(offsetof(struct action_rss_data, conf) == 0); + int n_rxq = netdev_n_rxq(netdev); int i; mark = xzalloc(sizeof *mark); @@ -1709,6 +1710,11 @@ add_flow_mark_rss_actions(struct flow_actions *actions, mark->id = flow_mark; add_flow_action(actions, RTE_FLOW_ACTION_TYPE_MARK, mark); + if (netdev->cp_prot_flags) { + /* last rxq is reserved for control plane packets */ + n_rxq -= 1; + } + rss_data = xmalloc(sizeof *rss_data + netdev_n_rxq(netdev) * sizeof rss_data->queue[0]); *rss_data = (struct action_rss_data) { @@ -1716,7 +1722,7 @@ add_flow_mark_rss_actions(struct flow_actions *actions, .func = RTE_ETH_HASH_FUNCTION_DEFAULT, .level = 0, .types = RTE_ETH_RSS_IP | RTE_ETH_RSS_UDP | RTE_ETH_RSS_TCP, - .queue_num = netdev_n_rxq(netdev), + .queue_num = n_rxq, .queue = rss_data->queue, .key_len = 0, .key = NULL @@ -1724,7 +1730,7 @@ add_flow_mark_rss_actions(struct flow_actions *actions, }; /* Override queue array with default. */ - for (i = 0; i < netdev_n_rxq(netdev); i++) { + for (i = 0; i < n_rxq; i++) { rss_data->queue[i] = i; } @@ -1744,7 +1750,7 @@ netdev_offload_dpdk_mark_rss(struct flow_patterns *patterns, }; const struct rte_flow_attr flow_attr = { .group = 0, - .priority = 0, + .priority = NETDEV_DPDK_FLOW_PRIORITY_OFFLOAD, .ingress = 1, .egress = 0 }; @@ -2235,7 +2241,11 @@ netdev_offload_dpdk_actions(struct netdev *netdev, struct nlattr *nl_actions, size_t actions_len) { - const struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1 }; + const struct rte_flow_attr flow_attr = { + .priority = NETDEV_DPDK_FLOW_PRIORITY_OFFLOAD, + .ingress = 1, + .transfer = 1, + }; struct flow_actions actions = { .actions = NULL, .cnt = 0, diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h index b5420947d0cd..e7ed6df37a4b 100644 --- a/lib/netdev-provider.h +++ b/lib/netdev-provider.h @@ -45,6 +45,10 @@ enum netdev_ol_flags { NETDEV_TX_OFFLOAD_TCP_TSO = 1 << 4, }; +enum netdev_cp_prot_flags { + NETDEV_CP_PROT_LACP = 1 << 0, +}; + /* A network device (e.g. an Ethernet device). * * Network device implementations may read these members but should not modify @@ -98,6 +102,9 @@ struct netdev { OVSRCU_TYPE(const struct netdev_flow_api *) flow_api; const char *dpif_type; /* Type of dpif this netdev belongs to. */ struct netdev_hw_info hw_info; /* Offload-capable netdev info. */ + /* Control plane protection flags, + * from the enum set 'netdev_cp_prot_flags' */ + uint64_t cp_prot_flags; }; static inline void -- 2.37.3
Hi Thilak, Thilak Raj Surendra Babu, Sep 22, 2022 at 07:16: > Could we call rte_flow_validate() before calling flow_create ? At first, I thought it would not be necessary hence skipped the validation and I am only relying on the driver to reject the rule if not supported. However, it may be a bit more robust in case the last rss flow is not supported. We would not have to flush other flows. > Can we not use isolate the Queue using rte_flow_isolate()? From what I understand, rte_flow_isolate is very intrusive. https://doc.dpdk.org/guides-21.11/prog_guide/rte_flow.html#flow-isolated-mode It may not play well with the dpdk rte flow offload subsystem. Thanks for reviewing.
diff --git a/Documentation/topics/dpdk/phy.rst b/Documentation/topics/dpdk/phy.rst index 937f4c40e5a8..00b3887c70bd 100644 --- a/Documentation/topics/dpdk/phy.rst +++ b/Documentation/topics/dpdk/phy.rst @@ -131,6 +131,47 @@ possible with DPDK acceleration. It is possible to configure multiple Rx queues for ``dpdk`` ports, thus ensuring this is not a bottleneck for performance. For information on configuring PMD threads, refer to :doc:`pmd`. +Control Plane Protection +------------------------ + +Some control protocols are used to maintain link status between forwarding +engines. In SDN environments, these packets share the same physical network than +the user data traffic. + +When the system is not sized properly, the PMD threads may not be able to +process all incoming traffic from the configured Rx queues. When a signaling +packet of such protocols is dropped, it can cause link flapping, worsening the +situation. + +Some physical NICs can be programmed to put these protocols in a dedicated +hardware Rx queue using the rte_flow__ API. + +__ https://doc.dpdk.org/guides/prog_guide/rte_flow.html#device-compatibility + +The currently supported control plane protocols are: + +``lacp`` + `Link Aggregation Control Protocol`__. Ether type ``0x8809``. + + __ https://www.ieee802.org/3/ad/public/mar99/seaman_1_0399.pdf + +.. warning:: + + This feature is not compatible with all NICs. Refer to vendor documentation + for more information. + +Control plane protection must be enabled on specific protocols per port. The +``cp-protection`` option requires a coma separated list of protocol names:: + + $ ovs-vsctl add-port br0 dpdk-p0 -- set Interface dpdk-p0 type=dpdk \ + options:dpdk-devargs=0000:01:00.0 options:cp-protection=lacp + +.. note:: + + If multiple Rx queues are already configured, regular RSS (Receive Side + Scaling) queue balancing is done on all but the extra control plane + protection queue. + .. _dpdk-phy-flow-control: Flow Control diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 0dd655507b50..033ba3e5ff6f 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -529,6 +529,12 @@ struct netdev_dpdk { /* VF configuration. */ struct eth_addr requested_hwaddr; + + /* Requested control plane protection flags, + * from the enum set 'netdev_cp_prot_flags' */ + uint64_t requested_cp_prot_flags; + size_t cp_prot_flows_num; + struct rte_flow **cp_prot_flows; ); PADDED_MEMBERS(CACHE_LINE_SIZE, @@ -1305,10 +1311,14 @@ common_construct(struct netdev *netdev, dpdk_port_t port_no, netdev->n_rxq = 0; netdev->n_txq = 0; + netdev->cp_prot_flags = 0; dev->requested_n_rxq = NR_QUEUE; dev->requested_n_txq = NR_QUEUE; dev->requested_rxq_size = NIC_PORT_DEFAULT_RXQ_SIZE; dev->requested_txq_size = NIC_PORT_DEFAULT_TXQ_SIZE; + dev->requested_cp_prot_flags = 0; + dev->cp_prot_flows_num = 0; + dev->cp_prot_flows = NULL; /* Initialize the flow control to NULL */ memset(&dev->fc_conf, 0, sizeof dev->fc_conf); @@ -1904,6 +1914,9 @@ dpdk_set_rxq_config(struct netdev_dpdk *dev, const struct smap *args) int new_n_rxq; new_n_rxq = MAX(smap_get_int(args, "n_rxq", NR_QUEUE), 1); + if (dev->requested_cp_prot_flags) { + new_n_rxq += 1; + } if (new_n_rxq != dev->requested_n_rxq) { dev->requested_n_rxq = new_n_rxq; netdev_request_reconfigure(&dev->up); @@ -1927,6 +1940,39 @@ dpdk_process_queue_size(struct netdev *netdev, const struct smap *args, } } +static int +dpdk_cp_prot_set_config(struct netdev *netdev, struct netdev_dpdk *dev, + const struct smap *args, char **errp) +{ + const char *arg = smap_get_def(args, "cp-protection", ""); + uint64_t flags = 0; + char buf[256]; + char *token, *saveptr; + + strncpy(buf, arg, sizeof(buf)); + buf[sizeof(buf) - 1] = '\0'; + + token = strtok_r(buf, ",", &saveptr); + while (token) { + if (strcmp(token, "lacp") == 0) { + flags |= NETDEV_CP_PROT_LACP; + } else { + VLOG_WARN_BUF( + errp, "%s options:cp-protection=%s unknown protocol '%s'", + netdev_get_name(netdev), arg, token); + return -1; + } + token = strtok_r(NULL, ",", &saveptr); + } + + if (flags != dev->requested_cp_prot_flags) { + dev->requested_cp_prot_flags = flags; + netdev_request_reconfigure(netdev); + } + + return 0; +} + static int netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, char **errp) @@ -1946,6 +1992,11 @@ netdev_dpdk_set_config(struct netdev *netdev, const struct smap *args, ovs_mutex_lock(&dpdk_mutex); ovs_mutex_lock(&dev->mutex); + if (dpdk_cp_prot_set_config(netdev, dev, args, errp) < 0) { + err = EINVAL; + goto out; + } + dpdk_set_rxq_config(dev, args); dpdk_process_queue_size(netdev, args, "n_rxq_desc", @@ -4933,6 +4984,179 @@ static const struct dpdk_qos_ops trtcm_policer_ops = { .qos_queue_dump_state_init = trtcm_policer_qos_queue_dump_state_init }; +static int +dpdk_cp_prot_add_flow(struct netdev_dpdk *dev, + const struct rte_flow_attr *attr, + const struct rte_flow_item items[], + const struct rte_flow_action actions[], + const char *desc) +{ + struct rte_flow_error error; + struct rte_flow *flow; + size_t num; + + flow = rte_flow_create(dev->port_id, attr, items, actions, &error); + if (flow == NULL) { + VLOG_WARN("%s: cp-protection: failed to add %s flow: %s", + netdev_get_name(&dev->up), desc, error.message); + return rte_errno; + } + + num = dev->cp_prot_flows_num + 1; + dev->cp_prot_flows = xrealloc(dev->cp_prot_flows, sizeof(flow) * num); + dev->cp_prot_flows[dev->cp_prot_flows_num] = flow; + dev->cp_prot_flows_num = num; + + return 0; +} + +static int +dpdk_cp_prot_add_traffic_flow(struct netdev_dpdk *dev, + const struct rte_flow_item items[], + const char *desc) +{ + const struct rte_flow_attr attr = { + .group = 0, + .priority = NETDEV_DPDK_FLOW_PRIORITY_MAX, + .ingress = 1, + .egress = 0, + .transfer = 0, + }; + const struct rte_flow_action actions[] = { + { + .type = RTE_FLOW_ACTION_TYPE_QUEUE, + .conf = &(const struct rte_flow_action_queue) { + .index = dev->up.n_rxq - 1, + }, + }, + { .type = RTE_FLOW_ACTION_TYPE_END }, + }; + + VLOG_INFO("%s: cp-protection: redirecting %s traffic to queue %d", + netdev_get_name(&dev->up), desc, dev->up.n_rxq - 1); + return dpdk_cp_prot_add_flow(dev, &attr, items, actions, desc); +} + +static int +dpdk_cp_prot_add_rss_flow(struct netdev_dpdk *dev) +{ + int rss_n_rxq = dev->up.n_rxq - 1; + const struct rte_flow_attr attr = { + .group = 0, + .priority = NETDEV_DPDK_FLOW_PRIORITY_FALLBACK, + .ingress = 1, + .egress = 0, + .transfer = 0, + }; + const struct rte_flow_item items[] = { + { .type = RTE_FLOW_ITEM_TYPE_ANY }, + { .type = RTE_FLOW_ITEM_TYPE_END }, + }; + + if (rss_n_rxq == 1) { + const struct rte_flow_action actions[] = { + { + .type = RTE_FLOW_ACTION_TYPE_QUEUE, + .conf = &(const struct rte_flow_action_queue) { + .index = 0, + }, + }, + { .type = RTE_FLOW_ACTION_TYPE_END }, + }; + + VLOG_INFO("%s: cp-protection: redirecting other traffic to queue 0", + netdev_get_name(&dev->up)); + return dpdk_cp_prot_add_flow(dev, &attr, items, actions, "other"); + } + + uint16_t rss_queues[rss_n_rxq]; + char buf[128]; + buf[0] = '\0'; + for (int i = 0; i < rss_n_rxq; i++) { + rss_queues[i] = i; + snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf) - 1, " %d", i); + } + buf[sizeof(buf) - 1] = '\0'; + const struct rte_flow_action actions[] = { + { + .type = RTE_FLOW_ACTION_TYPE_RSS, + .conf = &(const struct rte_flow_action_rss){ + .func = RTE_ETH_HASH_FUNCTION_DEFAULT, + .level = 0, + .types = RTE_ETH_RSS_IP | RTE_ETH_RSS_UDP | RTE_ETH_RSS_TCP, + .queue_num = rss_n_rxq, + .queue = rss_queues, + .key_len = 0, + .key = NULL, + }, + }, + { .type = RTE_FLOW_ACTION_TYPE_END }, + }; + + VLOG_INFO("%s: cp-protection: applying rss on queues:%s", + netdev_get_name(&dev->up), buf); + return dpdk_cp_prot_add_flow(dev, &attr, items, actions, "rss"); +} + +static int +dpdk_cp_prot_configure(struct netdev_dpdk *dev) +{ + int err = 0; + + if (dev->requested_cp_prot_flags & NETDEV_CP_PROT_LACP) { + err = dpdk_cp_prot_add_traffic_flow( + dev, + (const struct rte_flow_item []) { + { + .type = RTE_FLOW_ITEM_TYPE_ETH, + .spec = &(const struct rte_flow_item_eth){ + .type = htons(ETH_TYPE_LACP), + }, + .mask = &(const struct rte_flow_item_eth){ + .type = htons(0xffff), + }, + }, + { .type = RTE_FLOW_ITEM_TYPE_END }, + }, + "lacp" + ); + if (err) { + goto out; + } + } + + /* default flow is RSS in all but the cp protection queue */ + if (dev->requested_cp_prot_flags) { + err = dpdk_cp_prot_add_rss_flow(dev); + } + +out: + dev->up.cp_prot_flags = dev->requested_cp_prot_flags; + return err; +} + +static void +dpdk_cp_prot_unconfigure(struct netdev_dpdk *dev) +{ + struct rte_flow_error error; + + if (dev->cp_prot_flows_num == 0) { + return; + } + + VLOG_INFO("%s: cp-protection: reset flows", netdev_get_name(&dev->up)); + + for (int i = 0; i < dev->cp_prot_flows_num; i++) { + if (rte_flow_destroy(dev->port_id, dev->cp_prot_flows[i], &error)) { + VLOG_DBG("%s: cp-protection: failed to destroy flow: %s", + netdev_get_name(&dev->up), error.message); + } + } + free(dev->cp_prot_flows); + dev->cp_prot_flows_num = 0; + dev->cp_prot_flows = NULL; +} + static int netdev_dpdk_reconfigure(struct netdev *netdev) { @@ -4943,6 +5167,7 @@ netdev_dpdk_reconfigure(struct netdev *netdev) if (netdev->n_txq == dev->requested_n_txq && netdev->n_rxq == dev->requested_n_rxq + && netdev->cp_prot_flags == dev->requested_cp_prot_flags && dev->mtu == dev->requested_mtu && dev->lsc_interrupt_mode == dev->requested_lsc_interrupt_mode && dev->rxq_size == dev->requested_rxq_size @@ -4987,6 +5212,8 @@ netdev_dpdk_reconfigure(struct netdev *netdev) } } + dpdk_cp_prot_unconfigure(dev); + err = dpdk_eth_dev_init(dev); if (dev->hw_ol_features & NETDEV_TX_TSO_OFFLOAD) { netdev->ol_flags |= NETDEV_TX_OFFLOAD_TCP_TSO; @@ -5014,6 +5241,14 @@ netdev_dpdk_reconfigure(struct netdev *netdev) if (!dev->tx_q) { err = ENOMEM; } + if (!err && netdev->n_rxq > 1) { + err = dpdk_cp_prot_configure(dev); + if (err) { + /* no hardware support, recover gracefully */ + dpdk_cp_prot_unconfigure(dev); + err = 0; + } + } netdev_change_seq_changed(netdev); diff --git a/lib/netdev-dpdk.h b/lib/netdev-dpdk.h index 7d2f64af23e1..24acdc7b193c 100644 --- a/lib/netdev-dpdk.h +++ b/lib/netdev-dpdk.h @@ -17,6 +17,7 @@ #ifndef NETDEV_DPDK_H #define NETDEV_DPDK_H +#include <stdint.h> #include <config.h> #include "openvswitch/compiler.h" @@ -32,6 +33,10 @@ struct netdev; void netdev_dpdk_register(const struct smap *); void free_dpdk_buf(struct dp_packet *); +#define NETDEV_DPDK_FLOW_PRIORITY_MAX 0 +#define NETDEV_DPDK_FLOW_PRIORITY_OFFLOAD 10 +#define NETDEV_DPDK_FLOW_PRIORITY_FALLBACK UINT32_MAX + bool netdev_dpdk_flow_api_supported(struct netdev *); int diff --git a/lib/netdev-offload-dpdk.c b/lib/netdev-offload-dpdk.c index cceefbc50751..f19db670d56f 100644 --- a/lib/netdev-offload-dpdk.c +++ b/lib/netdev-offload-dpdk.c @@ -1702,6 +1702,7 @@ add_flow_mark_rss_actions(struct flow_actions *actions, uint16_t queue[0]; } *rss_data; BUILD_ASSERT_DECL(offsetof(struct action_rss_data, conf) == 0); + int n_rxq = netdev_n_rxq(netdev); int i; mark = xzalloc(sizeof *mark); @@ -1709,6 +1710,11 @@ add_flow_mark_rss_actions(struct flow_actions *actions, mark->id = flow_mark; add_flow_action(actions, RTE_FLOW_ACTION_TYPE_MARK, mark); + if (netdev->cp_prot_flags) { + /* last rxq is reserved for control plane packets */ + n_rxq -= 1; + } + rss_data = xmalloc(sizeof *rss_data + netdev_n_rxq(netdev) * sizeof rss_data->queue[0]); *rss_data = (struct action_rss_data) { @@ -1716,7 +1722,7 @@ add_flow_mark_rss_actions(struct flow_actions *actions, .func = RTE_ETH_HASH_FUNCTION_DEFAULT, .level = 0, .types = RTE_ETH_RSS_IP | RTE_ETH_RSS_UDP | RTE_ETH_RSS_TCP, - .queue_num = netdev_n_rxq(netdev), + .queue_num = n_rxq, .queue = rss_data->queue, .key_len = 0, .key = NULL @@ -1724,7 +1730,7 @@ add_flow_mark_rss_actions(struct flow_actions *actions, }; /* Override queue array with default. */ - for (i = 0; i < netdev_n_rxq(netdev); i++) { + for (i = 0; i < n_rxq; i++) { rss_data->queue[i] = i; } @@ -1744,7 +1750,7 @@ netdev_offload_dpdk_mark_rss(struct flow_patterns *patterns, }; const struct rte_flow_attr flow_attr = { .group = 0, - .priority = 0, + .priority = NETDEV_DPDK_FLOW_PRIORITY_OFFLOAD, .ingress = 1, .egress = 0 }; @@ -2235,7 +2241,11 @@ netdev_offload_dpdk_actions(struct netdev *netdev, struct nlattr *nl_actions, size_t actions_len) { - const struct rte_flow_attr flow_attr = { .ingress = 1, .transfer = 1 }; + const struct rte_flow_attr flow_attr = { + .priority = NETDEV_DPDK_FLOW_PRIORITY_OFFLOAD, + .ingress = 1, + .transfer = 1, + }; struct flow_actions actions = { .actions = NULL, .cnt = 0, diff --git a/lib/netdev-provider.h b/lib/netdev-provider.h index b5420947d0cd..e7ed6df37a4b 100644 --- a/lib/netdev-provider.h +++ b/lib/netdev-provider.h @@ -45,6 +45,10 @@ enum netdev_ol_flags { NETDEV_TX_OFFLOAD_TCP_TSO = 1 << 4, }; +enum netdev_cp_prot_flags { + NETDEV_CP_PROT_LACP = 1 << 0, +}; + /* A network device (e.g. an Ethernet device). * * Network device implementations may read these members but should not modify @@ -98,6 +102,9 @@ struct netdev { OVSRCU_TYPE(const struct netdev_flow_api *) flow_api; const char *dpif_type; /* Type of dpif this netdev belongs to. */ struct netdev_hw_info hw_info; /* Offload-capable netdev info. */ + /* Control plane protection flags, + * from the enum set 'netdev_cp_prot_flags' */ + uint64_t cp_prot_flags; }; static inline void
Some control protocols are used to maintain link status between forwarding engines (e.g. LACP). When the system is not sized properly, the PMD threads may not be able to process all incoming traffic from the configured Rx queues. When a signaling packet of such protocols is dropped, it can cause link flapping, worsening the situation. Use the RTE flow API to redirect these protocols into a dedicated Rx queue. The assumption is made that the ratio between control protocol traffic and user data traffic is very low and thus this dedicated Rx queue will never get full. Regular RSS is done on all other Rx queues. The additional Rx queue will be assigned a PMD core like any other Rx queue. Polling that extra queue may introduce increased latency and a slight performance penalty at the benefit of preventing link flapping. This feature must be enabled per port on specific protocols via the cp-protection option. This option takes a coma-separated list of protocol names. Example: ovs-vsctl add-bond br-phy bond0 phy0 phy1 -- \ set interface phy0 type=dpdk options:dpdk-devargs=0000:ca:00.0 -- \ set interface phy0 options:cp-protection=lacp -- \ set interface phy1 type=dpdk options:dpdk-devargs=0000:ca:00.1 -- \ set interface phy1 options:cp-protection=lacp As a starting point, only one protocol is supported: LACP. Other protocols can be added in the future. NIC compatibility should be checked. To validate that this works as intended, I used a traffic generator to generate random traffic slightly above the machine capacity at line rate on a two ports bond interface. OVS is configured to receive traffic on two VLANs and pop/push them in a br-int bridge based on tags set on patch ports. +----------------------+ | DUT | |+--------------------+| || br-int || default flow, action=NORMAL || || || patch10 patch11 || |+---|-----------|----+| | | | | |+---|-----------|----+| || patch00 patch01 || || tag:10 tag:20 || || || || br-phy || default flow, action=NORMAL || || || bond0 || balance-slb, lacp=passive, lacp-time=fast || phy0 phy1 || |+------|-----|-------+| +-------|-----|--------+ | | +-------|-----|--------+ | port0 port1 | balance L3/L4, lacp=active, lacp-time=fast | lag | mode trunk VLANs 10, 20 | | | switch | | | | vlan 10 vlan 20 | mode access | port2 port3 | +-----|----------|-----+ | | +-----|----------|-----+ | port0 port1 | Random traffic that is properly balanced | | across the bond ports in both directions. | traffic generator | +----------------------+ Without cp-protection, the bond0 links are randomly switching to "defaulted" when one of the LACP packets sent by the switch is dropped because the RX queues are full and the PMD threads did not process them fast enough. When that happens, all traffic must go through a single link which causes above line rate traffic to be dropped. When cp-protection is enabled, no LACP packet is dropped and the bond links remain enabled at all times, maximizing the throughput. Notes: * This feature may be considered as "QoS". However, it does not work by limiting the rate of traffic explicitly. It only guarantees that some protocols have a lower chance of being dropped because the PMD cores cannot keep up with regular traffic. * The choice of protocols is limited on purpose. This is not meant to be configurable by users. Some limited configurability could be considered in the future but it would expose to more potential issues if users are accidentally redirecting all traffic in the control plane queue. * Encapsulated traffic (into VXLAN, GRE, Geneve) is not redirected into that extra queue. This may be addressed in the future as well. * For now, I validated that this works on Mellanox Connect-X 5 and Intel E810 NICs. Intel X710 does not support the match all flows. Validation on other NICs should be done. * Unit tests may not be possible here. The netdev-dummy driver does not share code with netdev-dpdk. Cc: Christophe Fontaine <cfontain@redhat.com> Cc: Kevin Traynor <ktraynor@redhat.com> Signed-off-by: Robin Jarry <rjarry@redhat.com> --- Documentation/topics/dpdk/phy.rst | 41 ++++++ lib/netdev-dpdk.c | 235 ++++++++++++++++++++++++++++++ lib/netdev-dpdk.h | 5 + lib/netdev-offload-dpdk.c | 18 ++- lib/netdev-provider.h | 7 + 5 files changed, 302 insertions(+), 4 deletions(-)