| Message ID | 1535984320-11574-1-git-send-email-ophirmu@mellanox.com |
|---|---|
| State | Superseded |
| Delegated to: | Ian Stokes |
| Headers | show |
| Series | [ovs-dev,dpdk-hwol,v1] netdev-dpdk: Upgrade to dpdk v18.08.0 | expand |
Hi Ophir, Thank you for working on this. I tried to run some tests and getting some errors while adding physical ports as below. I use Niantic NIC for this test >>>>>>>>>>>>>>> 2018-09-05T06:34:58Z|00069|dpdk|INFO|Device with port_id=0 already stopped 2018-09-05T06:34:58Z|00070|dpdk|ERR|Ethdev port_id=0 invalid rss_hf: 0x3afbc, valid value: 0x38d34 2018-09-05T06:34:58Z|00071|netdev_dpdk|WARN|Interface dpdk0 eth_dev setup error Invalid argument 2018-09-05T06:34:58Z|00072|netdev_dpdk|ERR|Interface dpdk0(rxq:2 txq:2 lsc interrupt mode:false) configure error: Invalid argument 2018-09-05T06:34:58Z|00073|dpif_netdev|ERR|Failed to set interface dpdk0 new configuration 2018-09-05T06:34:58Z|00074|bridge|WARN|could not add network device dpdk0 to ofproto (No such device) ovs-vsctl: Error detected while setting up 'dpdk0': could not add network device dpdk0 to ofproto (No such device). See ovs-vswitchd log for details. ovs-vsctl: The default log directory is "/usr/local/var/log/openvswitch". >>>>>>>>>>>>>>>> Please find below for my comments. Regards _Sugesh > -----Original Message----- > From: Ophir Munk [mailto:ophirmu@mellanox.com] > Sent: Monday, September 3, 2018 3:19 PM > To: ovs-dev@openvswitch.org > Cc: Asaf Penso <asafp@mellanox.com>; Chandran, Sugesh > <sugesh.chandran@intel.com>; Stokes, Ian <ian.stokes@intel.com>; Ben Pfaff > <blp@ovn.org>; Shahaf Shuler <shahafs@mellanox.com>; Thomas Monjalon > <thomas@monjalon.net>; Olga Shern <olgas@mellanox.com>; Ophir Munk > <ophirmu@mellanox.com>; Kevin Traynor <ktraynor@redhat.com> > Subject: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk v18.08.0 > > 1. Enable compilation and linkage with dpdk 18.08.0 The following dpdk > commits which were introduced after dpdk 17.11.x require OVS updates to > accommodate to the dpdk changes. > - ce17eddefc20 ("ethdev: introduce Rx queue offloads API") > - ab3ce1e0c193 ("ethdev: remove old offload API") > - c06ddf9698e0 ("meter: add configuration profile") > - e58638c32411 ("ethdev: fix TPID handling in flow API") > - cd8c7c7ce241 ("ethdev: replace bus specific struct with generic dev") > - ac8d22de2394 ("ethdev: flatten RSS configuration in flow API") > > 2. Update references to DPDK version in Documentation > > 3. Update DPDK version in travis' linux-build script > > Signed-off-by: Ophir Munk <ophirmu@mellanox.com> > --- > .travis/linux-build.sh | 2 +- > Documentation/faq/releases.rst | 2 +- > lib/netdev-dpdk.c | 135 +++++++++++++++++++++++++++++------------ > 3 files changed, 98 insertions(+), 41 deletions(-) > > diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh index 4b9fc4a..c60ac71 > 100755 > --- a/.travis/linux-build.sh > +++ b/.travis/linux-build.sh > @@ -83,7 +83,7 @@ fi > > if [ "$DPDK" ]; then > if [ -z "$DPDK_VER" ]; then > - DPDK_VER="17.11.3" > + DPDK_VER="18.08.0" > fi > install_dpdk $DPDK_VER > if [ "$CC" = "clang" ]; then > diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst > index 41d41e3..646ae09 100644 > --- a/Documentation/faq/releases.rst > +++ b/Documentation/faq/releases.rst > @@ -168,7 +168,7 @@ Q: What DPDK version does each Open vSwitch release > work with? > 2.7.x 16.11.7 > 2.8.x 17.05.2 > 2.9.x 17.11.3 > - 2.10.x 17.11.3 > + 2.10.x 18.08.0 > ============ ======= > > Q: Are all the DPDK releases that OVS versions work with maintained? > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index f91aa27..80d2af9 > 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -168,11 +168,7 @@ static const struct rte_eth_conf port_conf = { > .rxmode = { > .mq_mode = ETH_MQ_RX_RSS, > .split_hdr_size = 0, > - .header_split = 0, /* Header Split disabled */ > - .hw_ip_checksum = 0, /* IP checksum offload disabled */ > - .hw_vlan_filter = 0, /* VLAN filtering disabled */ > - .jumbo_frame = 0, /* Jumbo Frame Support disabled */ > - .hw_strip_crc = 0, > + .offloads = 0, > }, > .rx_adv_conf = { > .rss_conf = { > @@ -364,6 +360,7 @@ struct dpdk_ring { > struct ingress_policer { > struct rte_meter_srtcm_params app_srtcm_params; > struct rte_meter_srtcm in_policer; > + struct rte_meter_srtcm_profile in_prof; > rte_spinlock_t policer_lock; > }; > > @@ -903,16 +900,17 @@ dpdk_eth_dev_port_config(struct netdev_dpdk *dev, > int n_rxq, int n_txq) > if (dev->mtu > ETHER_MTU) { > rte_eth_dev_info_get(dev->port_id, &info); > if (strncmp(info.driver_name, "net_nfp", 7)) { > - conf.rxmode.enable_scatter = 1; > + conf.rxmode.offloads |= DEV_RX_OFFLOAD_SCATTER; > } > } > > conf.intr_conf.lsc = dev->lsc_interrupt_mode; > - conf.rxmode.hw_ip_checksum = (dev->hw_ol_features & > - NETDEV_RX_CHECKSUM_OFFLOAD) != 0; > + conf.rxmode.offloads |= ((dev->hw_ol_features & > + NETDEV_RX_CHECKSUM_OFFLOAD) != 0) ? > + DEV_RX_OFFLOAD_CHECKSUM : 0; > > if (dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP) { > - conf.rxmode.hw_strip_crc = 1; > + conf.rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP; > } > > /* A device may report more queues than it makes available (this has @@ - > 1932,16 +1930,18 @@ netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev, int > qid, > > static inline bool > netdev_dpdk_policer_pkt_handle(struct rte_meter_srtcm *meter, > + struct rte_meter_srtcm_profile *profile, > struct rte_mbuf *pkt, uint64_t time) { > uint32_t pkt_len = rte_pktmbuf_pkt_len(pkt) - sizeof(struct ether_hdr); > > - return rte_meter_srtcm_color_blind_check(meter, time, pkt_len) == > + return rte_meter_srtcm_color_blind_check(meter, profile, time, > + pkt_len) == > e_RTE_METER_GREEN; } > > static int > netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, > + struct rte_meter_srtcm_profile *profile, > struct rte_mbuf **pkts, int pkt_cnt, > bool should_steal) { @@ -1953,7 +1953,8 @@ > netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, > for (i = 0; i < pkt_cnt; i++) { > pkt = pkts[i]; > /* Handle current packet */ > - if (netdev_dpdk_policer_pkt_handle(meter, pkt, current_time)) { > + if (netdev_dpdk_policer_pkt_handle(meter, profile, pkt, > + current_time)) { > if (cnt != i) { > pkts[cnt] = pkt; > } > @@ -1975,8 +1976,8 @@ ingress_policer_run(struct ingress_policer *policer, > struct rte_mbuf **pkts, > int cnt = 0; > > rte_spinlock_lock(&policer->policer_lock); > - cnt = netdev_dpdk_policer_run(&policer->in_policer, pkts, > - pkt_cnt, should_steal); > + cnt = netdev_dpdk_policer_run(&policer->in_policer, &policer->in_prof, > + pkts, pkt_cnt, should_steal); > rte_spinlock_unlock(&policer->policer_lock); > > return cnt; > @@ -2767,8 +2768,15 @@ netdev_dpdk_policer_construct(uint32_t rate, > uint32_t burst) > policer->app_srtcm_params.cir = rate_bytes; > policer->app_srtcm_params.cbs = burst_bytes; > policer->app_srtcm_params.ebs = 0; > + err = rte_meter_srtcm_profile_config(&policer->in_prof, > + &policer->app_srtcm_params); > + if (err) { > + VLOG_ERR("Could not create rte meter profile for ingress policer"); > + free(policer); > + return NULL; > + } > err = rte_meter_srtcm_config(&policer->in_policer, > - &policer->app_srtcm_params); > + &policer->in_prof); > if (err) { > VLOG_ERR("Could not create rte meter for ingress policer"); > free(policer); > @@ -3043,13 +3051,18 @@ netdev_dpdk_get_status(const struct netdev > *netdev, struct smap *args) > smap_add_format(args, "if_descr", "%s %s", rte_version(), > dev_info.driver_name); > > - if (dev_info.pci_dev) { > - smap_add_format(args, "pci-vendor_id", "0x%x", > - dev_info.pci_dev->id.vendor_id); > - smap_add_format(args, "pci-device_id", "0x%x", > - dev_info.pci_dev->id.device_id); > + const struct rte_bus *bus; > + const struct rte_pci_device *pci_dev; > + bus = rte_bus_find_by_device(dev_info.device); > + if (bus && !strcmp(bus->name, "pci")) { > + pci_dev = RTE_DEV_TO_PCI(dev_info.device); > + if (pci_dev) { > + smap_add_format(args, "pci-vendor_id", "0x%u", > + pci_dev->id.vendor_id); > + smap_add_format(args, "pci-device_id", "0x%x", > + pci_dev->id.device_id); > + } > } > - > return 0; > } > > @@ -3727,6 +3740,7 @@ struct egress_policer { > struct qos_conf qos_conf; > struct rte_meter_srtcm_params app_srtcm_params; > struct rte_meter_srtcm egress_meter; > + struct rte_meter_srtcm_profile egress_prof; > }; > > static void > @@ -3749,8 +3763,16 @@ egress_policer_qos_construct(const struct smap > *details, > policer = xmalloc(sizeof *policer); > qos_conf_init(&policer->qos_conf, &egress_policer_ops); > egress_policer_details_to_param(details, &policer->app_srtcm_params); > + err = rte_meter_srtcm_profile_config(&policer->egress_prof, > + &policer->app_srtcm_params); > + if (err) { [Sugesh] Should we error out the message on failing the profile config?? > + free(policer); > + *conf = NULL; > + return -err; > + } > err = rte_meter_srtcm_config(&policer->egress_meter, > - &policer->app_srtcm_params); > + &policer->egress_prof); > + > if (!err) { > *conf = &policer->qos_conf; > } else { > @@ -3803,7 +3825,8 @@ egress_policer_run(struct qos_conf *conf, struct > rte_mbuf **pkts, int pkt_cnt, > struct egress_policer *policer = > CONTAINER_OF(conf, struct egress_policer, qos_conf); > > - cnt = netdev_dpdk_policer_run(&policer->egress_meter, pkts, > + cnt = netdev_dpdk_policer_run(&policer->egress_meter, > + &policer->egress_prof, pkts, > pkt_cnt, should_steal); > > return cnt; > @@ -4103,15 +4126,15 @@ dump_flow_pattern(struct rte_flow_item *item) > > VLOG_DBG("rte flow vlan pattern:\n"); > if (vlan_spec) { > - VLOG_DBG(" Spec: tpid=0x%"PRIx16", tci=0x%"PRIx16"\n", > - ntohs(vlan_spec->tpid), ntohs(vlan_spec->tci)); > + VLOG_DBG(" Spec: inner_type=0x%"PRIx16", tci=0x%"PRIx16"\n", > + ntohs(vlan_spec->inner_type), > + ntohs(vlan_spec->tci)); > } else { > VLOG_DBG(" Spec = null\n"); > } > > if (vlan_mask) { > - VLOG_DBG(" Mask: tpid=0x%"PRIx16", tci=0x%"PRIx16"\n", > - vlan_mask->tpid, vlan_mask->tci); > + VLOG_DBG(" Mask: inner_type=0x%"PRIx16", tci=0x%"PRIx16"\n", > + vlan_mask->inner_type, vlan_mask->tci); > } else { > VLOG_DBG(" Mask = null\n"); > } > @@ -4281,27 +4304,56 @@ add_flow_action(struct flow_actions *actions, > enum rte_flow_action_type type, > actions->cnt++; > } > > +/* > + * Storage for struct rte_flow_action_rss > + * including storage for key and queue array */ #define > +MAX_RSS_HASH_KEY_LENGTH 128 #define MAX_ACTION_RSS_QUEUE_NUM > 128 > +struct action_rss_data { > + struct rte_flow_action_rss conf; > + uint16_t queue[MAX_ACTION_RSS_QUEUE_NUM]; > + uint8_t key[MAX_RSS_HASH_KEY_LENGTH]; }; > + > static struct rte_flow_action_rss * > add_flow_rss_action(struct flow_actions *actions, > struct netdev *netdev) { > int i; > - struct rte_flow_action_rss *rss; > + struct action_rss_data *rss_data; > > - rss = xmalloc(sizeof(*rss) + sizeof(uint16_t) * netdev->n_rxq); > - /* > - * Setting it to NULL will let the driver use the default RSS > - * configuration we have set: &port_conf.rx_adv_conf.rss_conf. > - */ > - rss->rss_conf = NULL; > - rss->num = netdev->n_rxq; > + if (netdev->n_rxq > MAX_ACTION_RSS_QUEUE_NUM) { > + VLOG_ERR("Num of rxq (%u) must not be greater " \ > + "than max rss num of queues (%u)", > + netdev->n_rxq, MAX_ACTION_RSS_QUEUE_NUM); > + return NULL; > + } > > - for (i = 0; i < rss->num; i++) { > - rss->queue[i] = i; > + rss_data = xmalloc(sizeof(struct action_rss_data)); > + *rss_data = (struct action_rss_data){ > + .conf = (struct rte_flow_action_rss){ > + .func = RTE_ETH_HASH_FUNCTION_DEFAULT, > + .level = 0, > + .types = ETH_RSS_IP, > + .key_len = 0, > + .queue_num = netdev->n_rxq, > + .queue = rss_data->queue, > + .key = rss_data->key, > + }, > + .key = { 0 }, > + .queue = { 0 }, > + }; > + > + /* TODO: Override key with default */ [Sugesh] Do we really need this TODO?? > + > + /* Override queue array with default */ > + for (i = 0; i < rss_data->conf.queue_num; i++) { > + rss_data->queue[i] = i; > } > > - add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, rss); > + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, > + &rss_data->conf); > > - return rss; > + return &rss_data->conf; > } > > static int > @@ -4365,7 +4417,7 @@ netdev_dpdk_add_rte_flow_offload(struct netdev > *netdev, > vlan_mask.tci = match->wc.masks.vlans[0].tci & ~htons(VLAN_CFI); > > /* match any protocols */ > - vlan_mask.tpid = 0; > + vlan_mask.inner_type = 0; > > add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_VLAN, > &vlan_spec, &vlan_mask); @@ -4516,6 +4568,11 @@ > end_proto_check: > > struct rte_flow_action_rss *rss; > rss = add_flow_rss_action(&actions, netdev); > + if (!rss) { > + VLOG_ERR("add_flow_rss_action error.\n"); I feel this error can be bit more verbose something like Failed to add rss_flow+actions or something?? What do you think? > + ret = -1; > + goto out; > + } > add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL); > > flow = rte_flow_create(dev->port_id, &flow_attr, patterns.items, > -- > 1.8.3.1
On 09/05/2018 07:44 AM, Chandran, Sugesh wrote: > Hi Ophir, > > Thank you for working on this. > I tried to run some tests and getting some errors while adding physical ports as below. I use Niantic NIC for this test > >>>>>>>>>>>>>>>> > 2018-09-05T06:34:58Z|00069|dpdk|INFO|Device with port_id=0 already stopped > 2018-09-05T06:34:58Z|00070|dpdk|ERR|Ethdev port_id=0 invalid rss_hf: 0x3afbc, valid value: 0x38d34 > 2018-09-05T06:34:58Z|00071|netdev_dpdk|WARN|Interface dpdk0 eth_dev setup error Invalid argument > 2018-09-05T06:34:58Z|00072|netdev_dpdk|ERR|Interface dpdk0(rxq:2 txq:2 lsc interrupt mode:false) configure error: Invalid argument > 2018-09-05T06:34:58Z|00073|dpif_netdev|ERR|Failed to set interface dpdk0 new configuration > 2018-09-05T06:34:58Z|00074|bridge|WARN|could not add network device dpdk0 to ofproto (No such device) > ovs-vsctl: Error detected while setting up 'dpdk0': could not add network device dpdk0 to ofproto (No such device). See ovs-vswitchd log for details. > ovs-vsctl: The default log directory is "/usr/local/var/log/openvswitch". > I didn't have time to compare the below patch with the ones that I did, but I saw that this patch was needed for niantic https://github.com/kevintraynor/ovs-dpdk-master/commit/88f46cc5ab338eb4f3ca5db1eacd0effefe4fa0c >>>>>>>>>>>>>>>>> > Please find below for my comments. > > > Regards > _Sugesh > >> -----Original Message----- >> From: Ophir Munk [mailto:ophirmu@mellanox.com] >> Sent: Monday, September 3, 2018 3:19 PM >> To: ovs-dev@openvswitch.org >> Cc: Asaf Penso <asafp@mellanox.com>; Chandran, Sugesh >> <sugesh.chandran@intel.com>; Stokes, Ian <ian.stokes@intel.com>; Ben Pfaff >> <blp@ovn.org>; Shahaf Shuler <shahafs@mellanox.com>; Thomas Monjalon >> <thomas@monjalon.net>; Olga Shern <olgas@mellanox.com>; Ophir Munk >> <ophirmu@mellanox.com>; Kevin Traynor <ktraynor@redhat.com> >> Subject: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk v18.08.0 >> >> 1. Enable compilation and linkage with dpdk 18.08.0 The following dpdk >> commits which were introduced after dpdk 17.11.x require OVS updates to >> accommodate to the dpdk changes. >> - ce17eddefc20 ("ethdev: introduce Rx queue offloads API") >> - ab3ce1e0c193 ("ethdev: remove old offload API") >> - c06ddf9698e0 ("meter: add configuration profile") >> - e58638c32411 ("ethdev: fix TPID handling in flow API") >> - cd8c7c7ce241 ("ethdev: replace bus specific struct with generic dev") >> - ac8d22de2394 ("ethdev: flatten RSS configuration in flow API") >> >> 2. Update references to DPDK version in Documentation >> >> 3. Update DPDK version in travis' linux-build script >> >> Signed-off-by: Ophir Munk <ophirmu@mellanox.com> >> --- >> .travis/linux-build.sh | 2 +- >> Documentation/faq/releases.rst | 2 +- >> lib/netdev-dpdk.c | 135 +++++++++++++++++++++++++++++------------ >> 3 files changed, 98 insertions(+), 41 deletions(-) >> >> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh index 4b9fc4a..c60ac71 >> 100755 >> --- a/.travis/linux-build.sh >> +++ b/.travis/linux-build.sh >> @@ -83,7 +83,7 @@ fi >> >> if [ "$DPDK" ]; then >> if [ -z "$DPDK_VER" ]; then >> - DPDK_VER="17.11.3" >> + DPDK_VER="18.08.0" >> fi >> install_dpdk $DPDK_VER >> if [ "$CC" = "clang" ]; then >> diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst >> index 41d41e3..646ae09 100644 >> --- a/Documentation/faq/releases.rst >> +++ b/Documentation/faq/releases.rst >> @@ -168,7 +168,7 @@ Q: What DPDK version does each Open vSwitch release >> work with? >> 2.7.x 16.11.7 >> 2.8.x 17.05.2 >> 2.9.x 17.11.3 >> - 2.10.x 17.11.3 >> + 2.10.x 18.08.0 >> ============ ======= >> >> Q: Are all the DPDK releases that OVS versions work with maintained? >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index f91aa27..80d2af9 >> 100644 >> --- a/lib/netdev-dpdk.c >> +++ b/lib/netdev-dpdk.c >> @@ -168,11 +168,7 @@ static const struct rte_eth_conf port_conf = { >> .rxmode = { >> .mq_mode = ETH_MQ_RX_RSS, >> .split_hdr_size = 0, >> - .header_split = 0, /* Header Split disabled */ >> - .hw_ip_checksum = 0, /* IP checksum offload disabled */ >> - .hw_vlan_filter = 0, /* VLAN filtering disabled */ >> - .jumbo_frame = 0, /* Jumbo Frame Support disabled */ >> - .hw_strip_crc = 0, >> + .offloads = 0, >> }, >> .rx_adv_conf = { >> .rss_conf = { >> @@ -364,6 +360,7 @@ struct dpdk_ring { >> struct ingress_policer { >> struct rte_meter_srtcm_params app_srtcm_params; >> struct rte_meter_srtcm in_policer; >> + struct rte_meter_srtcm_profile in_prof; >> rte_spinlock_t policer_lock; >> }; >> >> @@ -903,16 +900,17 @@ dpdk_eth_dev_port_config(struct netdev_dpdk *dev, >> int n_rxq, int n_txq) >> if (dev->mtu > ETHER_MTU) { >> rte_eth_dev_info_get(dev->port_id, &info); >> if (strncmp(info.driver_name, "net_nfp", 7)) { >> - conf.rxmode.enable_scatter = 1; >> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_SCATTER; >> } >> } >> >> conf.intr_conf.lsc = dev->lsc_interrupt_mode; >> - conf.rxmode.hw_ip_checksum = (dev->hw_ol_features & >> - NETDEV_RX_CHECKSUM_OFFLOAD) != 0; >> + conf.rxmode.offloads |= ((dev->hw_ol_features & >> + NETDEV_RX_CHECKSUM_OFFLOAD) != 0) ? >> + DEV_RX_OFFLOAD_CHECKSUM : 0; >> >> if (dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP) { >> - conf.rxmode.hw_strip_crc = 1; >> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP; >> } >> >> /* A device may report more queues than it makes available (this has @@ - >> 1932,16 +1930,18 @@ netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev, int >> qid, >> >> static inline bool >> netdev_dpdk_policer_pkt_handle(struct rte_meter_srtcm *meter, >> + struct rte_meter_srtcm_profile *profile, >> struct rte_mbuf *pkt, uint64_t time) { >> uint32_t pkt_len = rte_pktmbuf_pkt_len(pkt) - sizeof(struct ether_hdr); >> >> - return rte_meter_srtcm_color_blind_check(meter, time, pkt_len) == >> + return rte_meter_srtcm_color_blind_check(meter, profile, time, >> + pkt_len) == >> e_RTE_METER_GREEN; } >> >> static int >> netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, >> + struct rte_meter_srtcm_profile *profile, >> struct rte_mbuf **pkts, int pkt_cnt, >> bool should_steal) { @@ -1953,7 +1953,8 @@ >> netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, >> for (i = 0; i < pkt_cnt; i++) { >> pkt = pkts[i]; >> /* Handle current packet */ >> - if (netdev_dpdk_policer_pkt_handle(meter, pkt, current_time)) { >> + if (netdev_dpdk_policer_pkt_handle(meter, profile, pkt, >> + current_time)) { >> if (cnt != i) { >> pkts[cnt] = pkt; >> } >> @@ -1975,8 +1976,8 @@ ingress_policer_run(struct ingress_policer *policer, >> struct rte_mbuf **pkts, >> int cnt = 0; >> >> rte_spinlock_lock(&policer->policer_lock); >> - cnt = netdev_dpdk_policer_run(&policer->in_policer, pkts, >> - pkt_cnt, should_steal); >> + cnt = netdev_dpdk_policer_run(&policer->in_policer, &policer->in_prof, >> + pkts, pkt_cnt, should_steal); >> rte_spinlock_unlock(&policer->policer_lock); >> >> return cnt; >> @@ -2767,8 +2768,15 @@ netdev_dpdk_policer_construct(uint32_t rate, >> uint32_t burst) >> policer->app_srtcm_params.cir = rate_bytes; >> policer->app_srtcm_params.cbs = burst_bytes; >> policer->app_srtcm_params.ebs = 0; >> + err = rte_meter_srtcm_profile_config(&policer->in_prof, >> + &policer->app_srtcm_params); >> + if (err) { >> + VLOG_ERR("Could not create rte meter profile for ingress policer"); >> + free(policer); >> + return NULL; >> + } >> err = rte_meter_srtcm_config(&policer->in_policer, >> - &policer->app_srtcm_params); >> + &policer->in_prof); >> if (err) { >> VLOG_ERR("Could not create rte meter for ingress policer"); >> free(policer); >> @@ -3043,13 +3051,18 @@ netdev_dpdk_get_status(const struct netdev >> *netdev, struct smap *args) >> smap_add_format(args, "if_descr", "%s %s", rte_version(), >> dev_info.driver_name); >> >> - if (dev_info.pci_dev) { >> - smap_add_format(args, "pci-vendor_id", "0x%x", >> - dev_info.pci_dev->id.vendor_id); >> - smap_add_format(args, "pci-device_id", "0x%x", >> - dev_info.pci_dev->id.device_id); >> + const struct rte_bus *bus; >> + const struct rte_pci_device *pci_dev; >> + bus = rte_bus_find_by_device(dev_info.device); >> + if (bus && !strcmp(bus->name, "pci")) { >> + pci_dev = RTE_DEV_TO_PCI(dev_info.device); >> + if (pci_dev) { >> + smap_add_format(args, "pci-vendor_id", "0x%u", >> + pci_dev->id.vendor_id); >> + smap_add_format(args, "pci-device_id", "0x%x", >> + pci_dev->id.device_id); >> + } >> } >> - >> return 0; >> } >> >> @@ -3727,6 +3740,7 @@ struct egress_policer { >> struct qos_conf qos_conf; >> struct rte_meter_srtcm_params app_srtcm_params; >> struct rte_meter_srtcm egress_meter; >> + struct rte_meter_srtcm_profile egress_prof; >> }; >> >> static void >> @@ -3749,8 +3763,16 @@ egress_policer_qos_construct(const struct smap >> *details, >> policer = xmalloc(sizeof *policer); >> qos_conf_init(&policer->qos_conf, &egress_policer_ops); >> egress_policer_details_to_param(details, &policer->app_srtcm_params); >> + err = rte_meter_srtcm_profile_config(&policer->egress_prof, >> + &policer->app_srtcm_params); >> + if (err) { > [Sugesh] Should we error out the message on failing the profile config?? >> + free(policer); >> + *conf = NULL; >> + return -err; >> + } >> err = rte_meter_srtcm_config(&policer->egress_meter, >> - &policer->app_srtcm_params); >> + &policer->egress_prof); >> + >> if (!err) { >> *conf = &policer->qos_conf; >> } else { >> @@ -3803,7 +3825,8 @@ egress_policer_run(struct qos_conf *conf, struct >> rte_mbuf **pkts, int pkt_cnt, >> struct egress_policer *policer = >> CONTAINER_OF(conf, struct egress_policer, qos_conf); >> >> - cnt = netdev_dpdk_policer_run(&policer->egress_meter, pkts, >> + cnt = netdev_dpdk_policer_run(&policer->egress_meter, >> + &policer->egress_prof, pkts, >> pkt_cnt, should_steal); >> >> return cnt; >> @@ -4103,15 +4126,15 @@ dump_flow_pattern(struct rte_flow_item *item) >> >> VLOG_DBG("rte flow vlan pattern:\n"); >> if (vlan_spec) { >> - VLOG_DBG(" Spec: tpid=0x%"PRIx16", tci=0x%"PRIx16"\n", >> - ntohs(vlan_spec->tpid), ntohs(vlan_spec->tci)); >> + VLOG_DBG(" Spec: inner_type=0x%"PRIx16", tci=0x%"PRIx16"\n", >> + ntohs(vlan_spec->inner_type), >> + ntohs(vlan_spec->tci)); >> } else { >> VLOG_DBG(" Spec = null\n"); >> } >> >> if (vlan_mask) { >> - VLOG_DBG(" Mask: tpid=0x%"PRIx16", tci=0x%"PRIx16"\n", >> - vlan_mask->tpid, vlan_mask->tci); >> + VLOG_DBG(" Mask: inner_type=0x%"PRIx16", tci=0x%"PRIx16"\n", >> + vlan_mask->inner_type, vlan_mask->tci); >> } else { >> VLOG_DBG(" Mask = null\n"); >> } >> @@ -4281,27 +4304,56 @@ add_flow_action(struct flow_actions *actions, >> enum rte_flow_action_type type, >> actions->cnt++; >> } >> >> +/* >> + * Storage for struct rte_flow_action_rss >> + * including storage for key and queue array */ #define >> +MAX_RSS_HASH_KEY_LENGTH 128 #define MAX_ACTION_RSS_QUEUE_NUM >> 128 >> +struct action_rss_data { >> + struct rte_flow_action_rss conf; >> + uint16_t queue[MAX_ACTION_RSS_QUEUE_NUM]; >> + uint8_t key[MAX_RSS_HASH_KEY_LENGTH]; }; >> + >> static struct rte_flow_action_rss * >> add_flow_rss_action(struct flow_actions *actions, >> struct netdev *netdev) { >> int i; >> - struct rte_flow_action_rss *rss; >> + struct action_rss_data *rss_data; >> >> - rss = xmalloc(sizeof(*rss) + sizeof(uint16_t) * netdev->n_rxq); >> - /* >> - * Setting it to NULL will let the driver use the default RSS >> - * configuration we have set: &port_conf.rx_adv_conf.rss_conf. >> - */ >> - rss->rss_conf = NULL; >> - rss->num = netdev->n_rxq; >> + if (netdev->n_rxq > MAX_ACTION_RSS_QUEUE_NUM) { >> + VLOG_ERR("Num of rxq (%u) must not be greater " \ >> + "than max rss num of queues (%u)", >> + netdev->n_rxq, MAX_ACTION_RSS_QUEUE_NUM); >> + return NULL; >> + } >> >> - for (i = 0; i < rss->num; i++) { >> - rss->queue[i] = i; >> + rss_data = xmalloc(sizeof(struct action_rss_data)); >> + *rss_data = (struct action_rss_data){ >> + .conf = (struct rte_flow_action_rss){ >> + .func = RTE_ETH_HASH_FUNCTION_DEFAULT, >> + .level = 0, >> + .types = ETH_RSS_IP, >> + .key_len = 0, >> + .queue_num = netdev->n_rxq, >> + .queue = rss_data->queue, >> + .key = rss_data->key, >> + }, >> + .key = { 0 }, >> + .queue = { 0 }, >> + }; >> + >> + /* TODO: Override key with default */ > [Sugesh] Do we really need this TODO?? >> + >> + /* Override queue array with default */ >> + for (i = 0; i < rss_data->conf.queue_num; i++) { >> + rss_data->queue[i] = i; >> } >> >> - add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, rss); >> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, >> + &rss_data->conf); >> >> - return rss; >> + return &rss_data->conf; >> } >> >> static int >> @@ -4365,7 +4417,7 @@ netdev_dpdk_add_rte_flow_offload(struct netdev >> *netdev, >> vlan_mask.tci = match->wc.masks.vlans[0].tci & ~htons(VLAN_CFI); >> >> /* match any protocols */ >> - vlan_mask.tpid = 0; >> + vlan_mask.inner_type = 0; >> >> add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_VLAN, >> &vlan_spec, &vlan_mask); @@ -4516,6 +4568,11 @@ >> end_proto_check: >> >> struct rte_flow_action_rss *rss; >> rss = add_flow_rss_action(&actions, netdev); >> + if (!rss) { >> + VLOG_ERR("add_flow_rss_action error.\n"); > I feel this error can be bit more verbose something like Failed to add rss_flow+actions or something?? What do you think? >> + ret = -1; >> + goto out; >> + } >> add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL); >> >> flow = rte_flow_create(dev->port_id, &flow_attr, patterns.items, >> -- >> 1.8.3.1 >
Hi, I suggest the following trouble-shooting steps: 1. Try Kevin's fix for Niantic (attached here as a non-official patch on top of latest dpdk-hwol branch). Sugesh - can you please try it? 2. Sugesh - do you have the same issue with other NICs than Niantic on latest dpdk-hwol branch? (other NICs used to work a month ago) 3. I will try on my setup. Regards, Ophir > -----Original Message----- > From: Kevin Traynor [mailto:ktraynor@redhat.com] > Sent: Wednesday, September 05, 2018 9:28 AM > To: Chandran, Sugesh <sugesh.chandran@intel.com>; Ophir Munk > <ophirmu@mellanox.com>; ovs-dev@openvswitch.org > Cc: Asaf Penso <asafp@mellanox.com>; Stokes, Ian > <ian.stokes@intel.com>; Ben Pfaff <blp@ovn.org>; Shahaf Shuler > <shahafs@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>; > Olga Shern <olgas@mellanox.com> > Subject: Re: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk v18.08.0 > > On 09/05/2018 07:44 AM, Chandran, Sugesh wrote: > > Hi Ophir, > > > > Thank you for working on this. > > I tried to run some tests and getting some errors while adding > > physical ports as below. I use Niantic NIC for this test > > > >>>>>>>>>>>>>>>> > > 2018-09-05T06:34:58Z|00069|dpdk|INFO|Device with port_id=0 already > > stopped 2018-09-05T06:34:58Z|00070|dpdk|ERR|Ethdev port_id=0 invalid > > rss_hf: 0x3afbc, valid value: 0x38d34 > > 2018-09-05T06:34:58Z|00071|netdev_dpdk|WARN|Interface dpdk0 > eth_dev > > setup error Invalid argument > > 2018-09-05T06:34:58Z|00072|netdev_dpdk|ERR|Interface dpdk0(rxq:2 > txq:2 > > lsc interrupt mode:false) configure error: Invalid argument > > 2018-09-05T06:34:58Z|00073|dpif_netdev|ERR|Failed to set interface > > dpdk0 new configuration 2018-09- > 05T06:34:58Z|00074|bridge|WARN|could > > not add network device dpdk0 to ofproto (No such device) > > ovs-vsctl: Error detected while setting up 'dpdk0': could not add network > device dpdk0 to ofproto (No such device). See ovs-vswitchd log for details. > > ovs-vsctl: The default log directory is "/usr/local/var/log/openvswitch". > > > > I didn't have time to compare the below patch with the ones that I did, but I > saw that this patch was needed for niantic > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit > hub.com%2Fkevintraynor%2Fovs-dpdk- > master%2Fcommit%2F88f46cc5ab338eb4f3ca5db1eacd0effefe4fa0c&d > ata=02%7C01%7Cophirmu%40mellanox.com%7Cd203c89637bf4f27835508d > 61309798e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C63671 > 7328752336932&sdata=L7rWODNNEStIfibwXybu6Ub7Yvn%2BfLHk6VT > we14jxyY%3D&reserved=0 > > >>>>>>>>>>>>>>>>> > > Please find below for my comments. > > > > > > Regards > > _Sugesh > > > >> -----Original Message----- > >> From: Ophir Munk [mailto:ophirmu@mellanox.com] > >> Sent: Monday, September 3, 2018 3:19 PM > >> To: ovs-dev@openvswitch.org > >> Cc: Asaf Penso <asafp@mellanox.com>; Chandran, Sugesh > >> <sugesh.chandran@intel.com>; Stokes, Ian <ian.stokes@intel.com>; Ben > >> Pfaff <blp@ovn.org>; Shahaf Shuler <shahafs@mellanox.com>; Thomas > >> Monjalon <thomas@monjalon.net>; Olga Shern <olgas@mellanox.com>; > >> Ophir Munk <ophirmu@mellanox.com>; Kevin Traynor > >> <ktraynor@redhat.com> > >> Subject: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk v18.08.0 > >> > >> 1. Enable compilation and linkage with dpdk 18.08.0 The following > >> dpdk commits which were introduced after dpdk 17.11.x require OVS > >> updates to accommodate to the dpdk changes. > >> - ce17eddefc20 ("ethdev: introduce Rx queue offloads API") > >> - ab3ce1e0c193 ("ethdev: remove old offload API") > >> - c06ddf9698e0 ("meter: add configuration profile") > >> - e58638c32411 ("ethdev: fix TPID handling in flow API") > >> - cd8c7c7ce241 ("ethdev: replace bus specific struct with generic > >> dev") > >> - ac8d22de2394 ("ethdev: flatten RSS configuration in flow API") > >> > >> 2. Update references to DPDK version in Documentation > >> > >> 3. Update DPDK version in travis' linux-build script > >> > >> Signed-off-by: Ophir Munk <ophirmu@mellanox.com> > >> --- > >> .travis/linux-build.sh | 2 +- > >> Documentation/faq/releases.rst | 2 +- > >> lib/netdev-dpdk.c | 135 +++++++++++++++++++++++++++++-------- > ---- > >> 3 files changed, 98 insertions(+), 41 deletions(-) > >> > >> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh index > >> 4b9fc4a..c60ac71 > >> 100755 > >> --- a/.travis/linux-build.sh > >> +++ b/.travis/linux-build.sh > >> @@ -83,7 +83,7 @@ fi > >> > >> if [ "$DPDK" ]; then > >> if [ -z "$DPDK_VER" ]; then > >> - DPDK_VER="17.11.3" > >> + DPDK_VER="18.08.0" > >> fi > >> install_dpdk $DPDK_VER > >> if [ "$CC" = "clang" ]; then > >> diff --git a/Documentation/faq/releases.rst > >> b/Documentation/faq/releases.rst index 41d41e3..646ae09 100644 > >> --- a/Documentation/faq/releases.rst > >> +++ b/Documentation/faq/releases.rst > >> @@ -168,7 +168,7 @@ Q: What DPDK version does each Open vSwitch > >> release work with? > >> 2.7.x 16.11.7 > >> 2.8.x 17.05.2 > >> 2.9.x 17.11.3 > >> - 2.10.x 17.11.3 > >> + 2.10.x 18.08.0 > >> ============ ======= > >> > >> Q: Are all the DPDK releases that OVS versions work with maintained? > >> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > >> f91aa27..80d2af9 > >> 100644 > >> --- a/lib/netdev-dpdk.c > >> +++ b/lib/netdev-dpdk.c > >> @@ -168,11 +168,7 @@ static const struct rte_eth_conf port_conf = { > >> .rxmode = { > >> .mq_mode = ETH_MQ_RX_RSS, > >> .split_hdr_size = 0, > >> - .header_split = 0, /* Header Split disabled */ > >> - .hw_ip_checksum = 0, /* IP checksum offload disabled */ > >> - .hw_vlan_filter = 0, /* VLAN filtering disabled */ > >> - .jumbo_frame = 0, /* Jumbo Frame Support disabled */ > >> - .hw_strip_crc = 0, > >> + .offloads = 0, > >> }, > >> .rx_adv_conf = { > >> .rss_conf = { > >> @@ -364,6 +360,7 @@ struct dpdk_ring { struct ingress_policer { > >> struct rte_meter_srtcm_params app_srtcm_params; > >> struct rte_meter_srtcm in_policer; > >> + struct rte_meter_srtcm_profile in_prof; > >> rte_spinlock_t policer_lock; > >> }; > >> > >> @@ -903,16 +900,17 @@ dpdk_eth_dev_port_config(struct netdev_dpdk > >> *dev, int n_rxq, int n_txq) > >> if (dev->mtu > ETHER_MTU) { > >> rte_eth_dev_info_get(dev->port_id, &info); > >> if (strncmp(info.driver_name, "net_nfp", 7)) { > >> - conf.rxmode.enable_scatter = 1; > >> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_SCATTER; > >> } > >> } > >> > >> conf.intr_conf.lsc = dev->lsc_interrupt_mode; > >> - conf.rxmode.hw_ip_checksum = (dev->hw_ol_features & > >> - NETDEV_RX_CHECKSUM_OFFLOAD) != 0; > >> + conf.rxmode.offloads |= ((dev->hw_ol_features & > >> + NETDEV_RX_CHECKSUM_OFFLOAD) != 0) ? > >> + DEV_RX_OFFLOAD_CHECKSUM : 0; > >> > >> if (dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP) { > >> - conf.rxmode.hw_strip_crc = 1; > >> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP; > >> } > >> > >> /* A device may report more queues than it makes available (this > >> has @@ - > >> 1932,16 +1930,18 @@ netdev_dpdk_eth_tx_burst(struct netdev_dpdk > *dev, > >> int qid, > >> > >> static inline bool > >> netdev_dpdk_policer_pkt_handle(struct rte_meter_srtcm *meter, > >> + struct rte_meter_srtcm_profile > >> + *profile, > >> struct rte_mbuf *pkt, uint64_t time) { > >> uint32_t pkt_len = rte_pktmbuf_pkt_len(pkt) - sizeof(struct > >> ether_hdr); > >> > >> - return rte_meter_srtcm_color_blind_check(meter, time, pkt_len) == > >> + return rte_meter_srtcm_color_blind_check(meter, profile, time, > >> + pkt_len) == > >> e_RTE_METER_GREEN; > >> } > >> > >> static int > >> netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, > >> + struct rte_meter_srtcm_profile *profile, > >> struct rte_mbuf **pkts, int pkt_cnt, > >> bool should_steal) { @@ -1953,7 +1953,8 @@ > >> netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, > >> for (i = 0; i < pkt_cnt; i++) { > >> pkt = pkts[i]; > >> /* Handle current packet */ > >> - if (netdev_dpdk_policer_pkt_handle(meter, pkt, current_time)) { > >> + if (netdev_dpdk_policer_pkt_handle(meter, profile, pkt, > >> + current_time)) { > >> if (cnt != i) { > >> pkts[cnt] = pkt; > >> } > >> @@ -1975,8 +1976,8 @@ ingress_policer_run(struct ingress_policer > >> *policer, struct rte_mbuf **pkts, > >> int cnt = 0; > >> > >> rte_spinlock_lock(&policer->policer_lock); > >> - cnt = netdev_dpdk_policer_run(&policer->in_policer, pkts, > >> - pkt_cnt, should_steal); > >> + cnt = netdev_dpdk_policer_run(&policer->in_policer, &policer- > >in_prof, > >> + pkts, pkt_cnt, should_steal); > >> rte_spinlock_unlock(&policer->policer_lock); > >> > >> return cnt; > >> @@ -2767,8 +2768,15 @@ netdev_dpdk_policer_construct(uint32_t rate, > >> uint32_t burst) > >> policer->app_srtcm_params.cir = rate_bytes; > >> policer->app_srtcm_params.cbs = burst_bytes; > >> policer->app_srtcm_params.ebs = 0; > >> + err = rte_meter_srtcm_profile_config(&policer->in_prof, > >> + &policer->app_srtcm_params); > >> + if (err) { > >> + VLOG_ERR("Could not create rte meter profile for ingress policer"); > >> + free(policer); > >> + return NULL; > >> + } > >> err = rte_meter_srtcm_config(&policer->in_policer, > >> - &policer->app_srtcm_params); > >> + &policer->in_prof); > >> if (err) { > >> VLOG_ERR("Could not create rte meter for ingress policer"); > >> free(policer); > >> @@ -3043,13 +3051,18 @@ netdev_dpdk_get_status(const struct netdev > >> *netdev, struct smap *args) > >> smap_add_format(args, "if_descr", "%s %s", rte_version(), > >> > >> dev_info.driver_name); > >> > >> - if (dev_info.pci_dev) { > >> - smap_add_format(args, "pci-vendor_id", "0x%x", > >> - dev_info.pci_dev->id.vendor_id); > >> - smap_add_format(args, "pci-device_id", "0x%x", > >> - dev_info.pci_dev->id.device_id); > >> + const struct rte_bus *bus; > >> + const struct rte_pci_device *pci_dev; > >> + bus = rte_bus_find_by_device(dev_info.device); > >> + if (bus && !strcmp(bus->name, "pci")) { > >> + pci_dev = RTE_DEV_TO_PCI(dev_info.device); > >> + if (pci_dev) { > >> + smap_add_format(args, "pci-vendor_id", "0x%u", > >> + pci_dev->id.vendor_id); > >> + smap_add_format(args, "pci-device_id", "0x%x", > >> + pci_dev->id.device_id); > >> + } > >> } > >> - > >> return 0; > >> } > >> > >> @@ -3727,6 +3740,7 @@ struct egress_policer { > >> struct qos_conf qos_conf; > >> struct rte_meter_srtcm_params app_srtcm_params; > >> struct rte_meter_srtcm egress_meter; > >> + struct rte_meter_srtcm_profile egress_prof; > >> }; > >> > >> static void > >> @@ -3749,8 +3763,16 @@ egress_policer_qos_construct(const struct > smap > >> *details, > >> policer = xmalloc(sizeof *policer); > >> qos_conf_init(&policer->qos_conf, &egress_policer_ops); > >> egress_policer_details_to_param(details, > >> &policer->app_srtcm_params); > >> + err = rte_meter_srtcm_profile_config(&policer->egress_prof, > >> + &policer->app_srtcm_params); > >> + if (err) { > > [Sugesh] Should we error out the message on failing the profile config?? > >> + free(policer); > >> + *conf = NULL; > >> + return -err; > >> + } > >> err = rte_meter_srtcm_config(&policer->egress_meter, > >> - &policer->app_srtcm_params); > >> + &policer->egress_prof); > >> + > >> if (!err) { > >> *conf = &policer->qos_conf; > >> } else { > >> @@ -3803,7 +3825,8 @@ egress_policer_run(struct qos_conf *conf, > >> struct rte_mbuf **pkts, int pkt_cnt, > >> struct egress_policer *policer = > >> CONTAINER_OF(conf, struct egress_policer, qos_conf); > >> > >> - cnt = netdev_dpdk_policer_run(&policer->egress_meter, pkts, > >> + cnt = netdev_dpdk_policer_run(&policer->egress_meter, > >> + &policer->egress_prof, pkts, > >> pkt_cnt, should_steal); > >> > >> return cnt; > >> @@ -4103,15 +4126,15 @@ dump_flow_pattern(struct rte_flow_item > *item) > >> > >> VLOG_DBG("rte flow vlan pattern:\n"); > >> if (vlan_spec) { > >> - VLOG_DBG(" Spec: tpid=0x%"PRIx16", tci=0x%"PRIx16"\n", > >> - ntohs(vlan_spec->tpid), ntohs(vlan_spec->tci)); > >> + VLOG_DBG(" Spec: inner_type=0x%"PRIx16", tci=0x%"PRIx16"\n", > >> + ntohs(vlan_spec->inner_type), > >> + ntohs(vlan_spec->tci)); > >> } else { > >> VLOG_DBG(" Spec = null\n"); > >> } > >> > >> if (vlan_mask) { > >> - VLOG_DBG(" Mask: tpid=0x%"PRIx16", tci=0x%"PRIx16"\n", > >> - vlan_mask->tpid, vlan_mask->tci); > >> + VLOG_DBG(" Mask: inner_type=0x%"PRIx16", > tci=0x%"PRIx16"\n", > >> + vlan_mask->inner_type, vlan_mask->tci); > >> } else { > >> VLOG_DBG(" Mask = null\n"); > >> } > >> @@ -4281,27 +4304,56 @@ add_flow_action(struct flow_actions > *actions, > >> enum rte_flow_action_type type, > >> actions->cnt++; > >> } > >> > >> +/* > >> + * Storage for struct rte_flow_action_rss > >> + * including storage for key and queue array */ #define > >> +MAX_RSS_HASH_KEY_LENGTH 128 #define > MAX_ACTION_RSS_QUEUE_NUM > >> 128 > >> +struct action_rss_data { > >> + struct rte_flow_action_rss conf; > >> + uint16_t queue[MAX_ACTION_RSS_QUEUE_NUM]; > >> + uint8_t key[MAX_RSS_HASH_KEY_LENGTH]; }; > >> + > >> static struct rte_flow_action_rss * > >> add_flow_rss_action(struct flow_actions *actions, > >> struct netdev *netdev) { > >> int i; > >> - struct rte_flow_action_rss *rss; > >> + struct action_rss_data *rss_data; > >> > >> - rss = xmalloc(sizeof(*rss) + sizeof(uint16_t) * netdev->n_rxq); > >> - /* > >> - * Setting it to NULL will let the driver use the default RSS > >> - * configuration we have set: &port_conf.rx_adv_conf.rss_conf. > >> - */ > >> - rss->rss_conf = NULL; > >> - rss->num = netdev->n_rxq; > >> + if (netdev->n_rxq > MAX_ACTION_RSS_QUEUE_NUM) { > >> + VLOG_ERR("Num of rxq (%u) must not be greater " \ > >> + "than max rss num of queues (%u)", > >> + netdev->n_rxq, MAX_ACTION_RSS_QUEUE_NUM); > >> + return NULL; > >> + } > >> > >> - for (i = 0; i < rss->num; i++) { > >> - rss->queue[i] = i; > >> + rss_data = xmalloc(sizeof(struct action_rss_data)); > >> + *rss_data = (struct action_rss_data){ > >> + .conf = (struct rte_flow_action_rss){ > >> + .func = RTE_ETH_HASH_FUNCTION_DEFAULT, > >> + .level = 0, > >> + .types = ETH_RSS_IP, > >> + .key_len = 0, > >> + .queue_num = netdev->n_rxq, > >> + .queue = rss_data->queue, > >> + .key = rss_data->key, > >> + }, > >> + .key = { 0 }, > >> + .queue = { 0 }, > >> + }; > >> + > >> + /* TODO: Override key with default */ > > [Sugesh] Do we really need this TODO?? > >> + > >> + /* Override queue array with default */ > >> + for (i = 0; i < rss_data->conf.queue_num; i++) { > >> + rss_data->queue[i] = i; > >> } > >> > >> - add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, rss); > >> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, > >> + &rss_data->conf); > >> > >> - return rss; > >> + return &rss_data->conf; > >> } > >> > >> static int > >> @@ -4365,7 +4417,7 @@ netdev_dpdk_add_rte_flow_offload(struct > netdev > >> *netdev, > >> vlan_mask.tci = match->wc.masks.vlans[0].tci & > >> ~htons(VLAN_CFI); > >> > >> /* match any protocols */ > >> - vlan_mask.tpid = 0; > >> + vlan_mask.inner_type = 0; > >> > >> add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_VLAN, > >> &vlan_spec, &vlan_mask); @@ -4516,6 > >> +4568,11 @@ > >> end_proto_check: > >> > >> struct rte_flow_action_rss *rss; > >> rss = add_flow_rss_action(&actions, netdev); > >> + if (!rss) { > >> + VLOG_ERR("add_flow_rss_action error.\n"); > > I feel this error can be bit more verbose something like Failed to add > rss_flow+actions or something?? What do you think? > >> + ret = -1; > >> + goto out; > >> + } > >> add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL); > >> > >> flow = rte_flow_create(dev->port_id, &flow_attr, patterns.items, > >> -- > >> 1.8.3.1 > >
Hi Ophir,
I get the same error when trying this with an XL710, wanted to see if
the patch would work for non-root users.
Did not further troubleshoot it.
Also I get the following compile warnings(errors):
mv -f $depbase.Tpo $depbase.Po
lib/netdev-dpdk.c: In function 'netdev_dpdk_destruct':
lib/netdev-dpdk.c:1331:9: error: 'rte_eth_dev_detach' is deprecated
(declared at
/home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native-linuxapp-gcc/include/rte_ethdev.h:1451)
[-Werror=deprecated-declarations]
if (rte_eth_dev_detach(dev->port_id, devname) < 0) {
^
lib/netdev-dpdk.c: In function 'netdev_dpdk_process_devargs':
lib/netdev-dpdk.c:1624:13: error: 'rte_eth_dev_attach' is deprecated
(declared at
/home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native-linuxapp-gcc/include/rte_ethdev.h:1435)
[-Werror=deprecated-declarations]
if (!rte_eth_dev_attach(devargs, &new_port_id)) {
^
lib/netdev-dpdk.c: In function 'netdev_dpdk_detach':
lib/netdev-dpdk.c:3154:5: error: 'rte_eth_dev_detach' is deprecated
(declared at
/home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native-linuxapp-gcc/include/rte_ethdev.h:1451)
[-Werror=deprecated-declarations]
ret = rte_eth_dev_detach(port_id, devname);
^
lib/netdev-dpdk.c: At top level:
//Eelco
On 5 Sep 2018, at 12:44, Ophir Munk wrote:
> Hi,
> I suggest the following trouble-shooting steps:
> 1. Try Kevin's fix for Niantic (attached here as a non-official patch
> on top of latest dpdk-hwol branch).
> Sugesh - can you please try it?
> 2. Sugesh - do you have the same issue with other NICs than Niantic on
> latest dpdk-hwol branch? (other NICs used to work a month
> ago)
> 3. I will try on my setup.
>
> Regards,
> Ophir
>
>> -----Original Message-----
>> From: Kevin Traynor [mailto:ktraynor@redhat.com]
>> Sent: Wednesday, September 05, 2018 9:28 AM
>> To: Chandran, Sugesh <sugesh.chandran@intel.com>; Ophir Munk
>> <ophirmu@mellanox.com>; ovs-dev@openvswitch.org
>> Cc: Asaf Penso <asafp@mellanox.com>; Stokes, Ian
>> <ian.stokes@intel.com>; Ben Pfaff <blp@ovn.org>; Shahaf Shuler
>> <shahafs@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>;
>> Olga Shern <olgas@mellanox.com>
>> Subject: Re: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk
>> v18.08.0
>>
>> On 09/05/2018 07:44 AM, Chandran, Sugesh wrote:
>>> Hi Ophir,
>>>
>>> Thank you for working on this.
>>> I tried to run some tests and getting some errors while adding
>>> physical ports as below. I use Niantic NIC for this test
>>>
>>>>>>>>>>>>>>>>>>
>>> 2018-09-05T06:34:58Z|00069|dpdk|INFO|Device with port_id=0 already
>>> stopped 2018-09-05T06:34:58Z|00070|dpdk|ERR|Ethdev port_id=0 invalid
>>> rss_hf: 0x3afbc, valid value: 0x38d34
>>> 2018-09-05T06:34:58Z|00071|netdev_dpdk|WARN|Interface dpdk0
>> eth_dev
>>> setup error Invalid argument
>>> 2018-09-05T06:34:58Z|00072|netdev_dpdk|ERR|Interface dpdk0(rxq:2
>> txq:2
>>> lsc interrupt mode:false) configure error: Invalid argument
>>> 2018-09-05T06:34:58Z|00073|dpif_netdev|ERR|Failed to set interface
>>> dpdk0 new configuration 2018-09-
>> 05T06:34:58Z|00074|bridge|WARN|could
>>> not add network device dpdk0 to ofproto (No such device)
>>> ovs-vsctl: Error detected while setting up 'dpdk0': could not add
>>> network
>> device dpdk0 to ofproto (No such device). See ovs-vswitchd log for
>> details.
>>> ovs-vsctl: The default log directory is
>>> "/usr/local/var/log/openvswitch".
>>>
>>
>> I didn't have time to compare the below patch with the ones that I
>> did, but I
>> saw that this patch was needed for niantic
>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit
>> hub.com%2Fkevintraynor%2Fovs-dpdk-
>> master%2Fcommit%2F88f46cc5ab338eb4f3ca5db1eacd0effefe4fa0c&d
>> ata=02%7C01%7Cophirmu%40mellanox.com%7Cd203c89637bf4f27835508d
>> 61309798e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C63671
>> 7328752336932&sdata=L7rWODNNEStIfibwXybu6Ub7Yvn%2BfLHk6VT
>> we14jxyY%3D&reserved=0
>>
>>>>>>>>>>>>>>>>>>>
>>> Please find below for my comments.
>>>
>>>
>>> Regards
>>> _Sugesh
>>>
>>>> -----Original Message-----
>>>> From: Ophir Munk [mailto:ophirmu@mellanox.com]
>>>> Sent: Monday, September 3, 2018 3:19 PM
>>>> To: ovs-dev@openvswitch.org
>>>> Cc: Asaf Penso <asafp@mellanox.com>; Chandran, Sugesh
>>>> <sugesh.chandran@intel.com>; Stokes, Ian <ian.stokes@intel.com>;
>>>> Ben
>>>> Pfaff <blp@ovn.org>; Shahaf Shuler <shahafs@mellanox.com>; Thomas
>>>> Monjalon <thomas@monjalon.net>; Olga Shern <olgas@mellanox.com>;
>>>> Ophir Munk <ophirmu@mellanox.com>; Kevin Traynor
>>>> <ktraynor@redhat.com>
>>>> Subject: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk v18.08.0
>>>>
>>>> 1. Enable compilation and linkage with dpdk 18.08.0 The following
>>>> dpdk commits which were introduced after dpdk 17.11.x require OVS
>>>> updates to accommodate to the dpdk changes.
>>>> - ce17eddefc20 ("ethdev: introduce Rx queue offloads API")
>>>> - ab3ce1e0c193 ("ethdev: remove old offload API")
>>>> - c06ddf9698e0 ("meter: add configuration profile")
>>>> - e58638c32411 ("ethdev: fix TPID handling in flow API")
>>>> - cd8c7c7ce241 ("ethdev: replace bus specific struct with generic
>>>> dev")
>>>> - ac8d22de2394 ("ethdev: flatten RSS configuration in flow API")
>>>>
>>>> 2. Update references to DPDK version in Documentation
>>>>
>>>> 3. Update DPDK version in travis' linux-build script
>>>>
>>>> Signed-off-by: Ophir Munk <ophirmu@mellanox.com>
>>>> ---
>>>> .travis/linux-build.sh | 2 +-
>>>> Documentation/faq/releases.rst | 2 +-
>>>> lib/netdev-dpdk.c | 135
>>>> +++++++++++++++++++++++++++++--------
>> ----
>>>> 3 files changed, 98 insertions(+), 41 deletions(-)
>>>>
>>>> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh index
>>>> 4b9fc4a..c60ac71
>>>> 100755
>>>> --- a/.travis/linux-build.sh
>>>> +++ b/.travis/linux-build.sh
>>>> @@ -83,7 +83,7 @@ fi
>>>>
>>>> if [ "$DPDK" ]; then
>>>> if [ -z "$DPDK_VER" ]; then
>>>> - DPDK_VER="17.11.3"
>>>> + DPDK_VER="18.08.0"
>>>> fi
>>>> install_dpdk $DPDK_VER
>>>> if [ "$CC" = "clang" ]; then
>>>> diff --git a/Documentation/faq/releases.rst
>>>> b/Documentation/faq/releases.rst index 41d41e3..646ae09 100644
>>>> --- a/Documentation/faq/releases.rst
>>>> +++ b/Documentation/faq/releases.rst
>>>> @@ -168,7 +168,7 @@ Q: What DPDK version does each Open vSwitch
>>>> release work with?
>>>> 2.7.x 16.11.7
>>>> 2.8.x 17.05.2
>>>> 2.9.x 17.11.3
>>>> - 2.10.x 17.11.3
>>>> + 2.10.x 18.08.0
>>>> ============ =======
>>>>
>>>> Q: Are all the DPDK releases that OVS versions work with
>>>> maintained?
>>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
>>>> f91aa27..80d2af9
>>>> 100644
>>>> --- a/lib/netdev-dpdk.c
>>>> +++ b/lib/netdev-dpdk.c
>>>> @@ -168,11 +168,7 @@ static const struct rte_eth_conf port_conf = {
>>>> .rxmode = {
>>>> .mq_mode = ETH_MQ_RX_RSS,
>>>> .split_hdr_size = 0,
>>>> - .header_split = 0, /* Header Split disabled */
>>>> - .hw_ip_checksum = 0, /* IP checksum offload disabled */
>>>> - .hw_vlan_filter = 0, /* VLAN filtering disabled */
>>>> - .jumbo_frame = 0, /* Jumbo Frame Support disabled */
>>>> - .hw_strip_crc = 0,
>>>> + .offloads = 0,
>>>> },
>>>> .rx_adv_conf = {
>>>> .rss_conf = {
>>>> @@ -364,6 +360,7 @@ struct dpdk_ring { struct ingress_policer {
>>>> struct rte_meter_srtcm_params app_srtcm_params;
>>>> struct rte_meter_srtcm in_policer;
>>>> + struct rte_meter_srtcm_profile in_prof;
>>>> rte_spinlock_t policer_lock;
>>>> };
>>>>
>>>> @@ -903,16 +900,17 @@ dpdk_eth_dev_port_config(struct netdev_dpdk
>>>> *dev, int n_rxq, int n_txq)
>>>> if (dev->mtu > ETHER_MTU) {
>>>> rte_eth_dev_info_get(dev->port_id, &info);
>>>> if (strncmp(info.driver_name, "net_nfp", 7)) {
>>>> - conf.rxmode.enable_scatter = 1;
>>>> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_SCATTER;
>>>> }
>>>> }
>>>>
>>>> conf.intr_conf.lsc = dev->lsc_interrupt_mode;
>>>> - conf.rxmode.hw_ip_checksum = (dev->hw_ol_features &
>>>> - NETDEV_RX_CHECKSUM_OFFLOAD) !=
>>>> 0;
>>>> + conf.rxmode.offloads |= ((dev->hw_ol_features &
>>>> + NETDEV_RX_CHECKSUM_OFFLOAD) != 0) ?
>>>> + DEV_RX_OFFLOAD_CHECKSUM : 0;
>>>>
>>>> if (dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP) {
>>>> - conf.rxmode.hw_strip_crc = 1;
>>>> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP;
>>>> }
>>>>
>>>> /* A device may report more queues than it makes available
>>>> (this
>>>> has @@ -
>>>> 1932,16 +1930,18 @@ netdev_dpdk_eth_tx_burst(struct netdev_dpdk
>> *dev,
>>>> int qid,
>>>>
>>>> static inline bool
>>>> netdev_dpdk_policer_pkt_handle(struct rte_meter_srtcm *meter,
>>>> + struct rte_meter_srtcm_profile
>>>> + *profile,
>>>> struct rte_mbuf *pkt, uint64_t
>>>> time) {
>>>> uint32_t pkt_len = rte_pktmbuf_pkt_len(pkt) - sizeof(struct
>>>> ether_hdr);
>>>>
>>>> - return rte_meter_srtcm_color_blind_check(meter, time, pkt_len)
>>>> ==
>>>> + return rte_meter_srtcm_color_blind_check(meter, profile, time,
>>>> + pkt_len) ==
>>>> e_RTE_METER_GREEN;
>>>> }
>>>>
>>>> static int
>>>> netdev_dpdk_policer_run(struct rte_meter_srtcm *meter,
>>>> + struct rte_meter_srtcm_profile *profile,
>>>> struct rte_mbuf **pkts, int pkt_cnt,
>>>> bool should_steal) { @@ -1953,7 +1953,8
>>>> @@
>>>> netdev_dpdk_policer_run(struct rte_meter_srtcm *meter,
>>>> for (i = 0; i < pkt_cnt; i++) {
>>>> pkt = pkts[i];
>>>> /* Handle current packet */
>>>> - if (netdev_dpdk_policer_pkt_handle(meter, pkt,
>>>> current_time)) {
>>>> + if (netdev_dpdk_policer_pkt_handle(meter, profile, pkt,
>>>> + current_time)) {
>>>> if (cnt != i) {
>>>> pkts[cnt] = pkt;
>>>> }
>>>> @@ -1975,8 +1976,8 @@ ingress_policer_run(struct ingress_policer
>>>> *policer, struct rte_mbuf **pkts,
>>>> int cnt = 0;
>>>>
>>>> rte_spinlock_lock(&policer->policer_lock);
>>>> - cnt = netdev_dpdk_policer_run(&policer->in_policer, pkts,
>>>> - pkt_cnt, should_steal);
>>>> + cnt = netdev_dpdk_policer_run(&policer->in_policer, &policer-
>>> in_prof,
>>>> + pkts, pkt_cnt, should_steal);
>>>> rte_spinlock_unlock(&policer->policer_lock);
>>>>
>>>> return cnt;
>>>> @@ -2767,8 +2768,15 @@ netdev_dpdk_policer_construct(uint32_t rate,
>>>> uint32_t burst)
>>>> policer->app_srtcm_params.cir = rate_bytes;
>>>> policer->app_srtcm_params.cbs = burst_bytes;
>>>> policer->app_srtcm_params.ebs = 0;
>>>> + err = rte_meter_srtcm_profile_config(&policer->in_prof,
>>>> +
>>>> &policer->app_srtcm_params);
>>>> + if (err) {
>>>> + VLOG_ERR("Could not create rte meter profile for ingress
>>>> policer");
>>>> + free(policer);
>>>> + return NULL;
>>>> + }
>>>> err = rte_meter_srtcm_config(&policer->in_policer,
>>>> - &policer->app_srtcm_params);
>>>> + &policer->in_prof);
>>>> if (err) {
>>>> VLOG_ERR("Could not create rte meter for ingress
>>>> policer");
>>>> free(policer);
>>>> @@ -3043,13 +3051,18 @@ netdev_dpdk_get_status(const struct netdev
>>>> *netdev, struct smap *args)
>>>> smap_add_format(args, "if_descr", "%s %s", rte_version(),
>>>>
>>>> dev_info.driver_name);
>>>>
>>>> - if (dev_info.pci_dev) {
>>>> - smap_add_format(args, "pci-vendor_id", "0x%x",
>>>> - dev_info.pci_dev->id.vendor_id);
>>>> - smap_add_format(args, "pci-device_id", "0x%x",
>>>> - dev_info.pci_dev->id.device_id);
>>>> + const struct rte_bus *bus;
>>>> + const struct rte_pci_device *pci_dev;
>>>> + bus = rte_bus_find_by_device(dev_info.device);
>>>> + if (bus && !strcmp(bus->name, "pci")) {
>>>> + pci_dev = RTE_DEV_TO_PCI(dev_info.device);
>>>> + if (pci_dev) {
>>>> + smap_add_format(args, "pci-vendor_id", "0x%u",
>>>> + pci_dev->id.vendor_id);
>>>> + smap_add_format(args, "pci-device_id", "0x%x",
>>>> + pci_dev->id.device_id);
>>>> + }
>>>> }
>>>> -
>>>> return 0;
>>>> }
>>>>
>>>> @@ -3727,6 +3740,7 @@ struct egress_policer {
>>>> struct qos_conf qos_conf;
>>>> struct rte_meter_srtcm_params app_srtcm_params;
>>>> struct rte_meter_srtcm egress_meter;
>>>> + struct rte_meter_srtcm_profile egress_prof;
>>>> };
>>>>
>>>> static void
>>>> @@ -3749,8 +3763,16 @@ egress_policer_qos_construct(const struct
>> smap
>>>> *details,
>>>> policer = xmalloc(sizeof *policer);
>>>> qos_conf_init(&policer->qos_conf, &egress_policer_ops);
>>>> egress_policer_details_to_param(details,
>>>> &policer->app_srtcm_params);
>>>> + err = rte_meter_srtcm_profile_config(&policer->egress_prof,
>>>> +
>>>> &policer->app_srtcm_params);
>>>> + if (err) {
>>> [Sugesh] Should we error out the message on failing the profile
>>> config??
>>>> + free(policer);
>>>> + *conf = NULL;
>>>> + return -err;
>>>> + }
>>>> err = rte_meter_srtcm_config(&policer->egress_meter,
>>>> - &policer->app_srtcm_params);
>>>> + &policer->egress_prof);
>>>> +
>>>> if (!err) {
>>>> *conf = &policer->qos_conf;
>>>> } else {
>>>> @@ -3803,7 +3825,8 @@ egress_policer_run(struct qos_conf *conf,
>>>> struct rte_mbuf **pkts, int pkt_cnt,
>>>> struct egress_policer *policer =
>>>> CONTAINER_OF(conf, struct egress_policer, qos_conf);
>>>>
>>>> - cnt = netdev_dpdk_policer_run(&policer->egress_meter, pkts,
>>>> + cnt = netdev_dpdk_policer_run(&policer->egress_meter,
>>>> + &policer->egress_prof, pkts,
>>>> pkt_cnt, should_steal);
>>>>
>>>> return cnt;
>>>> @@ -4103,15 +4126,15 @@ dump_flow_pattern(struct rte_flow_item
>> *item)
>>>>
>>>> VLOG_DBG("rte flow vlan pattern:\n");
>>>> if (vlan_spec) {
>>>> - VLOG_DBG(" Spec: tpid=0x%"PRIx16",
>>>> tci=0x%"PRIx16"\n",
>>>> - ntohs(vlan_spec->tpid),
>>>> ntohs(vlan_spec->tci));
>>>> + VLOG_DBG(" Spec: inner_type=0x%"PRIx16",
>>>> tci=0x%"PRIx16"\n",
>>>> + ntohs(vlan_spec->inner_type),
>>>> + ntohs(vlan_spec->tci));
>>>> } else {
>>>> VLOG_DBG(" Spec = null\n");
>>>> }
>>>>
>>>> if (vlan_mask) {
>>>> - VLOG_DBG(" Mask: tpid=0x%"PRIx16",
>>>> tci=0x%"PRIx16"\n",
>>>> - vlan_mask->tpid, vlan_mask->tci);
>>>> + VLOG_DBG(" Mask: inner_type=0x%"PRIx16",
>> tci=0x%"PRIx16"\n",
>>>> + vlan_mask->inner_type, vlan_mask->tci);
>>>> } else {
>>>> VLOG_DBG(" Mask = null\n");
>>>> }
>>>> @@ -4281,27 +4304,56 @@ add_flow_action(struct flow_actions
>> *actions,
>>>> enum rte_flow_action_type type,
>>>> actions->cnt++;
>>>> }
>>>>
>>>> +/*
>>>> + * Storage for struct rte_flow_action_rss
>>>> + * including storage for key and queue array */ #define
>>>> +MAX_RSS_HASH_KEY_LENGTH 128 #define
>> MAX_ACTION_RSS_QUEUE_NUM
>>>> 128
>>>> +struct action_rss_data {
>>>> + struct rte_flow_action_rss conf;
>>>> + uint16_t queue[MAX_ACTION_RSS_QUEUE_NUM];
>>>> + uint8_t key[MAX_RSS_HASH_KEY_LENGTH]; };
>>>> +
>>>> static struct rte_flow_action_rss *
>>>> add_flow_rss_action(struct flow_actions *actions,
>>>> struct netdev *netdev) {
>>>> int i;
>>>> - struct rte_flow_action_rss *rss;
>>>> + struct action_rss_data *rss_data;
>>>>
>>>> - rss = xmalloc(sizeof(*rss) + sizeof(uint16_t) *
>>>> netdev->n_rxq);
>>>> - /*
>>>> - * Setting it to NULL will let the driver use the default RSS
>>>> - * configuration we have set: &port_conf.rx_adv_conf.rss_conf.
>>>> - */
>>>> - rss->rss_conf = NULL;
>>>> - rss->num = netdev->n_rxq;
>>>> + if (netdev->n_rxq > MAX_ACTION_RSS_QUEUE_NUM) {
>>>> + VLOG_ERR("Num of rxq (%u) must not be greater " \
>>>> + "than max rss num of queues (%u)",
>>>> + netdev->n_rxq, MAX_ACTION_RSS_QUEUE_NUM);
>>>> + return NULL;
>>>> + }
>>>>
>>>> - for (i = 0; i < rss->num; i++) {
>>>> - rss->queue[i] = i;
>>>> + rss_data = xmalloc(sizeof(struct action_rss_data));
>>>> + *rss_data = (struct action_rss_data){
>>>> + .conf = (struct rte_flow_action_rss){
>>>> + .func = RTE_ETH_HASH_FUNCTION_DEFAULT,
>>>> + .level = 0,
>>>> + .types = ETH_RSS_IP,
>>>> + .key_len = 0,
>>>> + .queue_num = netdev->n_rxq,
>>>> + .queue = rss_data->queue,
>>>> + .key = rss_data->key,
>>>> + },
>>>> + .key = { 0 },
>>>> + .queue = { 0 },
>>>> + };
>>>> +
>>>> + /* TODO: Override key with default */
>>> [Sugesh] Do we really need this TODO??
>>>> +
>>>> + /* Override queue array with default */
>>>> + for (i = 0; i < rss_data->conf.queue_num; i++) {
>>>> + rss_data->queue[i] = i;
>>>> }
>>>>
>>>> - add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, rss);
>>>> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS,
>>>> + &rss_data->conf);
>>>>
>>>> - return rss;
>>>> + return &rss_data->conf;
>>>> }
>>>>
>>>> static int
>>>> @@ -4365,7 +4417,7 @@ netdev_dpdk_add_rte_flow_offload(struct
>> netdev
>>>> *netdev,
>>>> vlan_mask.tci = match->wc.masks.vlans[0].tci &
>>>> ~htons(VLAN_CFI);
>>>>
>>>> /* match any protocols */
>>>> - vlan_mask.tpid = 0;
>>>> + vlan_mask.inner_type = 0;
>>>>
>>>> add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_VLAN,
>>>> &vlan_spec, &vlan_mask); @@ -4516,6
>>>> +4568,11 @@
>>>> end_proto_check:
>>>>
>>>> struct rte_flow_action_rss *rss;
>>>> rss = add_flow_rss_action(&actions, netdev);
>>>> + if (!rss) {
>>>> + VLOG_ERR("add_flow_rss_action error.\n");
>>> I feel this error can be bit more verbose something like Failed to
>>> add
>> rss_flow+actions or something?? What do you think?
>>>> + ret = -1;
>>>> + goto out;
>>>> + }
>>>> add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL);
>>>>
>>>> flow = rte_flow_create(dev->port_id, &flow_attr,
>>>> patterns.items,
>>>> --
>>>> 1.8.3.1
>>>
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Forgot to cut/paste the log: 2018-09-05T12:47:48.462Z|00132|dpdk|INFO|Device with port_id=1 already stopped 2018-09-05T12:47:48.462Z|00133|dpdk|ERR|Ethdev port_id=1 invalid rss_hf: 0x3afbc, valid value: 0x7ef8 2018-09-05T12:47:48.462Z|00134|netdev_dpdk|WARN|Interface dpdk1 eth_dev setup error Invalid argument 2018-09-05T12:47:48.462Z|00135|netdev_dpdk|ERR|Interface dpdk1(rxq:1 txq:3 lsc interrupt mode:false) configure error: Invalid argument 2018-09-05T12:47:48.462Z|00136|dpif_netdev|ERR|Failed to set interface dpdk1 new configuration Guess this will help… On 5 Sep 2018, at 14:35, Eelco Chaudron wrote: > Hi Ophir, > > I get the same error when trying this with an XL710, wanted to see if > the patch would work for non-root users. > Did not further troubleshoot it. > > Also I get the following compile warnings(errors): > > mv -f $depbase.Tpo $depbase.Po > lib/netdev-dpdk.c: In function 'netdev_dpdk_destruct': > lib/netdev-dpdk.c:1331:9: error: 'rte_eth_dev_detach' is deprecated > (declared at > /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native-linuxapp-gcc/include/rte_ethdev.h:1451) > [-Werror=deprecated-declarations] > if (rte_eth_dev_detach(dev->port_id, devname) < 0) { > ^ > lib/netdev-dpdk.c: In function 'netdev_dpdk_process_devargs': > lib/netdev-dpdk.c:1624:13: error: 'rte_eth_dev_attach' is deprecated > (declared at > /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native-linuxapp-gcc/include/rte_ethdev.h:1435) > [-Werror=deprecated-declarations] > if (!rte_eth_dev_attach(devargs, &new_port_id)) { > ^ > lib/netdev-dpdk.c: In function 'netdev_dpdk_detach': > lib/netdev-dpdk.c:3154:5: error: 'rte_eth_dev_detach' is deprecated > (declared at > /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native-linuxapp-gcc/include/rte_ethdev.h:1451) > [-Werror=deprecated-declarations] > ret = rte_eth_dev_detach(port_id, devname); > ^ > lib/netdev-dpdk.c: At top level: > > > //Eelco > > > On 5 Sep 2018, at 12:44, Ophir Munk wrote: > >> Hi, >> I suggest the following trouble-shooting steps: >> 1. Try Kevin's fix for Niantic (attached here as a non-official patch >> on top of latest dpdk-hwol branch). >> Sugesh - can you please try it? >> 2. Sugesh - do you have the same issue with other NICs than Niantic >> on latest dpdk-hwol branch? (other NICs used to work a month >> ago) >> 3. I will try on my setup. >> >> Regards, >> Ophir >> >>> -----Original Message----- >>> From: Kevin Traynor [mailto:ktraynor@redhat.com] >>> Sent: Wednesday, September 05, 2018 9:28 AM >>> To: Chandran, Sugesh <sugesh.chandran@intel.com>; Ophir Munk >>> <ophirmu@mellanox.com>; ovs-dev@openvswitch.org >>> Cc: Asaf Penso <asafp@mellanox.com>; Stokes, Ian >>> <ian.stokes@intel.com>; Ben Pfaff <blp@ovn.org>; Shahaf Shuler >>> <shahafs@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>; >>> Olga Shern <olgas@mellanox.com> >>> Subject: Re: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk >>> v18.08.0 >>> >>> On 09/05/2018 07:44 AM, Chandran, Sugesh wrote: >>>> Hi Ophir, >>>> >>>> Thank you for working on this. >>>> I tried to run some tests and getting some errors while adding >>>> physical ports as below. I use Niantic NIC for this test >>>> >>>>>>>>>>>>>>>>>>> >>>> 2018-09-05T06:34:58Z|00069|dpdk|INFO|Device with port_id=0 already >>>> stopped 2018-09-05T06:34:58Z|00070|dpdk|ERR|Ethdev port_id=0 >>>> invalid >>>> rss_hf: 0x3afbc, valid value: 0x38d34 >>>> 2018-09-05T06:34:58Z|00071|netdev_dpdk|WARN|Interface dpdk0 >>> eth_dev >>>> setup error Invalid argument >>>> 2018-09-05T06:34:58Z|00072|netdev_dpdk|ERR|Interface dpdk0(rxq:2 >>> txq:2 >>>> lsc interrupt mode:false) configure error: Invalid argument >>>> 2018-09-05T06:34:58Z|00073|dpif_netdev|ERR|Failed to set interface >>>> dpdk0 new configuration 2018-09- >>> 05T06:34:58Z|00074|bridge|WARN|could >>>> not add network device dpdk0 to ofproto (No such device) >>>> ovs-vsctl: Error detected while setting up 'dpdk0': could not add >>>> network >>> device dpdk0 to ofproto (No such device). See ovs-vswitchd log for >>> details. >>>> ovs-vsctl: The default log directory is >>>> "/usr/local/var/log/openvswitch". >>>> >>> >>> I didn't have time to compare the below patch with the ones that I >>> did, but I >>> saw that this patch was needed for niantic >>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit >>> hub.com%2Fkevintraynor%2Fovs-dpdk- >>> master%2Fcommit%2F88f46cc5ab338eb4f3ca5db1eacd0effefe4fa0c&d >>> ata=02%7C01%7Cophirmu%40mellanox.com%7Cd203c89637bf4f27835508d >>> 61309798e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C63671 >>> 7328752336932&sdata=L7rWODNNEStIfibwXybu6Ub7Yvn%2BfLHk6VT >>> we14jxyY%3D&reserved=0 >>> >>>>>>>>>>>>>>>>>>>> >>>> Please find below for my comments. >>>> >>>> >>>> Regards >>>> _Sugesh >>>> >>>>> -----Original Message----- >>>>> From: Ophir Munk [mailto:ophirmu@mellanox.com] >>>>> Sent: Monday, September 3, 2018 3:19 PM >>>>> To: ovs-dev@openvswitch.org >>>>> Cc: Asaf Penso <asafp@mellanox.com>; Chandran, Sugesh >>>>> <sugesh.chandran@intel.com>; Stokes, Ian <ian.stokes@intel.com>; >>>>> Ben >>>>> Pfaff <blp@ovn.org>; Shahaf Shuler <shahafs@mellanox.com>; Thomas >>>>> Monjalon <thomas@monjalon.net>; Olga Shern <olgas@mellanox.com>; >>>>> Ophir Munk <ophirmu@mellanox.com>; Kevin Traynor >>>>> <ktraynor@redhat.com> >>>>> Subject: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk >>>>> v18.08.0 >>>>> >>>>> 1. Enable compilation and linkage with dpdk 18.08.0 The following >>>>> dpdk commits which were introduced after dpdk 17.11.x require OVS >>>>> updates to accommodate to the dpdk changes. >>>>> - ce17eddefc20 ("ethdev: introduce Rx queue offloads API") >>>>> - ab3ce1e0c193 ("ethdev: remove old offload API") >>>>> - c06ddf9698e0 ("meter: add configuration profile") >>>>> - e58638c32411 ("ethdev: fix TPID handling in flow API") >>>>> - cd8c7c7ce241 ("ethdev: replace bus specific struct with generic >>>>> dev") >>>>> - ac8d22de2394 ("ethdev: flatten RSS configuration in flow API") >>>>> >>>>> 2. Update references to DPDK version in Documentation >>>>> >>>>> 3. Update DPDK version in travis' linux-build script >>>>> >>>>> Signed-off-by: Ophir Munk <ophirmu@mellanox.com> >>>>> --- >>>>> .travis/linux-build.sh | 2 +- >>>>> Documentation/faq/releases.rst | 2 +- >>>>> lib/netdev-dpdk.c | 135 >>>>> +++++++++++++++++++++++++++++-------- >>> ---- >>>>> 3 files changed, 98 insertions(+), 41 deletions(-) >>>>> >>>>> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh index >>>>> 4b9fc4a..c60ac71 >>>>> 100755 >>>>> --- a/.travis/linux-build.sh >>>>> +++ b/.travis/linux-build.sh >>>>> @@ -83,7 +83,7 @@ fi >>>>> >>>>> if [ "$DPDK" ]; then >>>>> if [ -z "$DPDK_VER" ]; then >>>>> - DPDK_VER="17.11.3" >>>>> + DPDK_VER="18.08.0" >>>>> fi >>>>> install_dpdk $DPDK_VER >>>>> if [ "$CC" = "clang" ]; then >>>>> diff --git a/Documentation/faq/releases.rst >>>>> b/Documentation/faq/releases.rst index 41d41e3..646ae09 100644 >>>>> --- a/Documentation/faq/releases.rst >>>>> +++ b/Documentation/faq/releases.rst >>>>> @@ -168,7 +168,7 @@ Q: What DPDK version does each Open vSwitch >>>>> release work with? >>>>> 2.7.x 16.11.7 >>>>> 2.8.x 17.05.2 >>>>> 2.9.x 17.11.3 >>>>> - 2.10.x 17.11.3 >>>>> + 2.10.x 18.08.0 >>>>> ============ ======= >>>>> >>>>> Q: Are all the DPDK releases that OVS versions work with >>>>> maintained? >>>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index >>>>> f91aa27..80d2af9 >>>>> 100644 >>>>> --- a/lib/netdev-dpdk.c >>>>> +++ b/lib/netdev-dpdk.c >>>>> @@ -168,11 +168,7 @@ static const struct rte_eth_conf port_conf = >>>>> { >>>>> .rxmode = { >>>>> .mq_mode = ETH_MQ_RX_RSS, >>>>> .split_hdr_size = 0, >>>>> - .header_split = 0, /* Header Split disabled */ >>>>> - .hw_ip_checksum = 0, /* IP checksum offload disabled */ >>>>> - .hw_vlan_filter = 0, /* VLAN filtering disabled */ >>>>> - .jumbo_frame = 0, /* Jumbo Frame Support disabled */ >>>>> - .hw_strip_crc = 0, >>>>> + .offloads = 0, >>>>> }, >>>>> .rx_adv_conf = { >>>>> .rss_conf = { >>>>> @@ -364,6 +360,7 @@ struct dpdk_ring { struct ingress_policer { >>>>> struct rte_meter_srtcm_params app_srtcm_params; >>>>> struct rte_meter_srtcm in_policer; >>>>> + struct rte_meter_srtcm_profile in_prof; >>>>> rte_spinlock_t policer_lock; >>>>> }; >>>>> >>>>> @@ -903,16 +900,17 @@ dpdk_eth_dev_port_config(struct netdev_dpdk >>>>> *dev, int n_rxq, int n_txq) >>>>> if (dev->mtu > ETHER_MTU) { >>>>> rte_eth_dev_info_get(dev->port_id, &info); >>>>> if (strncmp(info.driver_name, "net_nfp", 7)) { >>>>> - conf.rxmode.enable_scatter = 1; >>>>> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_SCATTER; >>>>> } >>>>> } >>>>> >>>>> conf.intr_conf.lsc = dev->lsc_interrupt_mode; >>>>> - conf.rxmode.hw_ip_checksum = (dev->hw_ol_features & >>>>> - NETDEV_RX_CHECKSUM_OFFLOAD) != >>>>> 0; >>>>> + conf.rxmode.offloads |= ((dev->hw_ol_features & >>>>> + NETDEV_RX_CHECKSUM_OFFLOAD) != 0) ? >>>>> + DEV_RX_OFFLOAD_CHECKSUM : 0; >>>>> >>>>> if (dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP) { >>>>> - conf.rxmode.hw_strip_crc = 1; >>>>> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP; >>>>> } >>>>> >>>>> /* A device may report more queues than it makes available >>>>> (this >>>>> has @@ - >>>>> 1932,16 +1930,18 @@ netdev_dpdk_eth_tx_burst(struct netdev_dpdk >>> *dev, >>>>> int qid, >>>>> >>>>> static inline bool >>>>> netdev_dpdk_policer_pkt_handle(struct rte_meter_srtcm *meter, >>>>> + struct rte_meter_srtcm_profile >>>>> + *profile, >>>>> struct rte_mbuf *pkt, uint64_t >>>>> time) { >>>>> uint32_t pkt_len = rte_pktmbuf_pkt_len(pkt) - sizeof(struct >>>>> ether_hdr); >>>>> >>>>> - return rte_meter_srtcm_color_blind_check(meter, time, >>>>> pkt_len) == >>>>> + return rte_meter_srtcm_color_blind_check(meter, profile, >>>>> time, >>>>> + pkt_len) == >>>>> e_RTE_METER_GREEN; >>>>> } >>>>> >>>>> static int >>>>> netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, >>>>> + struct rte_meter_srtcm_profile *profile, >>>>> struct rte_mbuf **pkts, int pkt_cnt, >>>>> bool should_steal) { @@ -1953,7 +1953,8 >>>>> @@ >>>>> netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, >>>>> for (i = 0; i < pkt_cnt; i++) { >>>>> pkt = pkts[i]; >>>>> /* Handle current packet */ >>>>> - if (netdev_dpdk_policer_pkt_handle(meter, pkt, >>>>> current_time)) { >>>>> + if (netdev_dpdk_policer_pkt_handle(meter, profile, pkt, >>>>> + current_time)) { >>>>> if (cnt != i) { >>>>> pkts[cnt] = pkt; >>>>> } >>>>> @@ -1975,8 +1976,8 @@ ingress_policer_run(struct ingress_policer >>>>> *policer, struct rte_mbuf **pkts, >>>>> int cnt = 0; >>>>> >>>>> rte_spinlock_lock(&policer->policer_lock); >>>>> - cnt = netdev_dpdk_policer_run(&policer->in_policer, pkts, >>>>> - pkt_cnt, should_steal); >>>>> + cnt = netdev_dpdk_policer_run(&policer->in_policer, &policer- >>>> in_prof, >>>>> + pkts, pkt_cnt, should_steal); >>>>> rte_spinlock_unlock(&policer->policer_lock); >>>>> >>>>> return cnt; >>>>> @@ -2767,8 +2768,15 @@ netdev_dpdk_policer_construct(uint32_t >>>>> rate, >>>>> uint32_t burst) >>>>> policer->app_srtcm_params.cir = rate_bytes; >>>>> policer->app_srtcm_params.cbs = burst_bytes; >>>>> policer->app_srtcm_params.ebs = 0; >>>>> + err = rte_meter_srtcm_profile_config(&policer->in_prof, >>>>> + >>>>> &policer->app_srtcm_params); >>>>> + if (err) { >>>>> + VLOG_ERR("Could not create rte meter profile for ingress >>>>> policer"); >>>>> + free(policer); >>>>> + return NULL; >>>>> + } >>>>> err = rte_meter_srtcm_config(&policer->in_policer, >>>>> - &policer->app_srtcm_params); >>>>> + &policer->in_prof); >>>>> if (err) { >>>>> VLOG_ERR("Could not create rte meter for ingress >>>>> policer"); >>>>> free(policer); >>>>> @@ -3043,13 +3051,18 @@ netdev_dpdk_get_status(const struct netdev >>>>> *netdev, struct smap *args) >>>>> smap_add_format(args, "if_descr", "%s %s", rte_version(), >>>>> >>>>> dev_info.driver_name); >>>>> >>>>> - if (dev_info.pci_dev) { >>>>> - smap_add_format(args, "pci-vendor_id", "0x%x", >>>>> - dev_info.pci_dev->id.vendor_id); >>>>> - smap_add_format(args, "pci-device_id", "0x%x", >>>>> - dev_info.pci_dev->id.device_id); >>>>> + const struct rte_bus *bus; >>>>> + const struct rte_pci_device *pci_dev; >>>>> + bus = rte_bus_find_by_device(dev_info.device); >>>>> + if (bus && !strcmp(bus->name, "pci")) { >>>>> + pci_dev = RTE_DEV_TO_PCI(dev_info.device); >>>>> + if (pci_dev) { >>>>> + smap_add_format(args, "pci-vendor_id", "0x%u", >>>>> + pci_dev->id.vendor_id); >>>>> + smap_add_format(args, "pci-device_id", "0x%x", >>>>> + pci_dev->id.device_id); >>>>> + } >>>>> } >>>>> - >>>>> return 0; >>>>> } >>>>> >>>>> @@ -3727,6 +3740,7 @@ struct egress_policer { >>>>> struct qos_conf qos_conf; >>>>> struct rte_meter_srtcm_params app_srtcm_params; >>>>> struct rte_meter_srtcm egress_meter; >>>>> + struct rte_meter_srtcm_profile egress_prof; >>>>> }; >>>>> >>>>> static void >>>>> @@ -3749,8 +3763,16 @@ egress_policer_qos_construct(const struct >>> smap >>>>> *details, >>>>> policer = xmalloc(sizeof *policer); >>>>> qos_conf_init(&policer->qos_conf, &egress_policer_ops); >>>>> egress_policer_details_to_param(details, >>>>> &policer->app_srtcm_params); >>>>> + err = rte_meter_srtcm_profile_config(&policer->egress_prof, >>>>> + >>>>> &policer->app_srtcm_params); >>>>> + if (err) { >>>> [Sugesh] Should we error out the message on failing the profile >>>> config?? >>>>> + free(policer); >>>>> + *conf = NULL; >>>>> + return -err; >>>>> + } >>>>> err = rte_meter_srtcm_config(&policer->egress_meter, >>>>> - &policer->app_srtcm_params); >>>>> + &policer->egress_prof); >>>>> + >>>>> if (!err) { >>>>> *conf = &policer->qos_conf; >>>>> } else { >>>>> @@ -3803,7 +3825,8 @@ egress_policer_run(struct qos_conf *conf, >>>>> struct rte_mbuf **pkts, int pkt_cnt, >>>>> struct egress_policer *policer = >>>>> CONTAINER_OF(conf, struct egress_policer, qos_conf); >>>>> >>>>> - cnt = netdev_dpdk_policer_run(&policer->egress_meter, pkts, >>>>> + cnt = netdev_dpdk_policer_run(&policer->egress_meter, >>>>> + &policer->egress_prof, pkts, >>>>> pkt_cnt, should_steal); >>>>> >>>>> return cnt; >>>>> @@ -4103,15 +4126,15 @@ dump_flow_pattern(struct rte_flow_item >>> *item) >>>>> >>>>> VLOG_DBG("rte flow vlan pattern:\n"); >>>>> if (vlan_spec) { >>>>> - VLOG_DBG(" Spec: tpid=0x%"PRIx16", >>>>> tci=0x%"PRIx16"\n", >>>>> - ntohs(vlan_spec->tpid), >>>>> ntohs(vlan_spec->tci)); >>>>> + VLOG_DBG(" Spec: inner_type=0x%"PRIx16", >>>>> tci=0x%"PRIx16"\n", >>>>> + ntohs(vlan_spec->inner_type), >>>>> + ntohs(vlan_spec->tci)); >>>>> } else { >>>>> VLOG_DBG(" Spec = null\n"); >>>>> } >>>>> >>>>> if (vlan_mask) { >>>>> - VLOG_DBG(" Mask: tpid=0x%"PRIx16", >>>>> tci=0x%"PRIx16"\n", >>>>> - vlan_mask->tpid, vlan_mask->tci); >>>>> + VLOG_DBG(" Mask: inner_type=0x%"PRIx16", >>> tci=0x%"PRIx16"\n", >>>>> + vlan_mask->inner_type, vlan_mask->tci); >>>>> } else { >>>>> VLOG_DBG(" Mask = null\n"); >>>>> } >>>>> @@ -4281,27 +4304,56 @@ add_flow_action(struct flow_actions >>> *actions, >>>>> enum rte_flow_action_type type, >>>>> actions->cnt++; >>>>> } >>>>> >>>>> +/* >>>>> + * Storage for struct rte_flow_action_rss >>>>> + * including storage for key and queue array */ #define >>>>> +MAX_RSS_HASH_KEY_LENGTH 128 #define >>> MAX_ACTION_RSS_QUEUE_NUM >>>>> 128 >>>>> +struct action_rss_data { >>>>> + struct rte_flow_action_rss conf; >>>>> + uint16_t queue[MAX_ACTION_RSS_QUEUE_NUM]; >>>>> + uint8_t key[MAX_RSS_HASH_KEY_LENGTH]; }; >>>>> + >>>>> static struct rte_flow_action_rss * >>>>> add_flow_rss_action(struct flow_actions *actions, >>>>> struct netdev *netdev) { >>>>> int i; >>>>> - struct rte_flow_action_rss *rss; >>>>> + struct action_rss_data *rss_data; >>>>> >>>>> - rss = xmalloc(sizeof(*rss) + sizeof(uint16_t) * >>>>> netdev->n_rxq); >>>>> - /* >>>>> - * Setting it to NULL will let the driver use the default RSS >>>>> - * configuration we have set: >>>>> &port_conf.rx_adv_conf.rss_conf. >>>>> - */ >>>>> - rss->rss_conf = NULL; >>>>> - rss->num = netdev->n_rxq; >>>>> + if (netdev->n_rxq > MAX_ACTION_RSS_QUEUE_NUM) { >>>>> + VLOG_ERR("Num of rxq (%u) must not be greater " \ >>>>> + "than max rss num of queues (%u)", >>>>> + netdev->n_rxq, MAX_ACTION_RSS_QUEUE_NUM); >>>>> + return NULL; >>>>> + } >>>>> >>>>> - for (i = 0; i < rss->num; i++) { >>>>> - rss->queue[i] = i; >>>>> + rss_data = xmalloc(sizeof(struct action_rss_data)); >>>>> + *rss_data = (struct action_rss_data){ >>>>> + .conf = (struct rte_flow_action_rss){ >>>>> + .func = RTE_ETH_HASH_FUNCTION_DEFAULT, >>>>> + .level = 0, >>>>> + .types = ETH_RSS_IP, >>>>> + .key_len = 0, >>>>> + .queue_num = netdev->n_rxq, >>>>> + .queue = rss_data->queue, >>>>> + .key = rss_data->key, >>>>> + }, >>>>> + .key = { 0 }, >>>>> + .queue = { 0 }, >>>>> + }; >>>>> + >>>>> + /* TODO: Override key with default */ >>>> [Sugesh] Do we really need this TODO?? >>>>> + >>>>> + /* Override queue array with default */ >>>>> + for (i = 0; i < rss_data->conf.queue_num; i++) { >>>>> + rss_data->queue[i] = i; >>>>> } >>>>> >>>>> - add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, rss); >>>>> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, >>>>> + &rss_data->conf); >>>>> >>>>> - return rss; >>>>> + return &rss_data->conf; >>>>> } >>>>> >>>>> static int >>>>> @@ -4365,7 +4417,7 @@ netdev_dpdk_add_rte_flow_offload(struct >>> netdev >>>>> *netdev, >>>>> vlan_mask.tci = match->wc.masks.vlans[0].tci & >>>>> ~htons(VLAN_CFI); >>>>> >>>>> /* match any protocols */ >>>>> - vlan_mask.tpid = 0; >>>>> + vlan_mask.inner_type = 0; >>>>> >>>>> add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_VLAN, >>>>> &vlan_spec, &vlan_mask); @@ -4516,6 >>>>> +4568,11 @@ >>>>> end_proto_check: >>>>> >>>>> struct rte_flow_action_rss *rss; >>>>> rss = add_flow_rss_action(&actions, netdev); >>>>> + if (!rss) { >>>>> + VLOG_ERR("add_flow_rss_action error.\n"); >>>> I feel this error can be bit more verbose something like Failed to >>>> add >>> rss_flow+actions or something?? What do you think? >>>>> + ret = -1; >>>>> + goto out; >>>>> + } >>>>> add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL); >>>>> >>>>> flow = rte_flow_create(dev->port_id, &flow_attr, >>>>> patterns.items, >>>>> -- >>>>> 1.8.3.1 >>>> >> >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
And one final note… I though I applied Kevin’s patch, but I applied it to the wrong directory… With Kevin’s patch applied it works, root and non root OVS. //Eelco On 5 Sep 2018, at 14:49, Eelco Chaudron wrote: > Forgot to cut/paste the log: > > 2018-09-05T12:47:48.462Z|00132|dpdk|INFO|Device with port_id=1 already > stopped > 2018-09-05T12:47:48.462Z|00133|dpdk|ERR|Ethdev port_id=1 invalid > rss_hf: 0x3afbc, valid value: 0x7ef8 > 2018-09-05T12:47:48.462Z|00134|netdev_dpdk|WARN|Interface dpdk1 > eth_dev setup error Invalid argument > 2018-09-05T12:47:48.462Z|00135|netdev_dpdk|ERR|Interface dpdk1(rxq:1 > txq:3 lsc interrupt mode:false) configure error: Invalid argument > 2018-09-05T12:47:48.462Z|00136|dpif_netdev|ERR|Failed to set interface > dpdk1 new configuration > > Guess this will help… > > On 5 Sep 2018, at 14:35, Eelco Chaudron wrote: > >> Hi Ophir, >> >> I get the same error when trying this with an XL710, wanted to see if >> the patch would work for non-root users. >> Did not further troubleshoot it. >> >> Also I get the following compile warnings(errors): >> >> mv -f $depbase.Tpo $depbase.Po >> lib/netdev-dpdk.c: In function 'netdev_dpdk_destruct': >> lib/netdev-dpdk.c:1331:9: error: 'rte_eth_dev_detach' is deprecated >> (declared at >> /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native-linuxapp-gcc/include/rte_ethdev.h:1451) >> [-Werror=deprecated-declarations] >> if (rte_eth_dev_detach(dev->port_id, devname) < 0) { >> ^ >> lib/netdev-dpdk.c: In function 'netdev_dpdk_process_devargs': >> lib/netdev-dpdk.c:1624:13: error: 'rte_eth_dev_attach' is deprecated >> (declared at >> /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native-linuxapp-gcc/include/rte_ethdev.h:1435) >> [-Werror=deprecated-declarations] >> if (!rte_eth_dev_attach(devargs, &new_port_id)) { >> ^ >> lib/netdev-dpdk.c: In function 'netdev_dpdk_detach': >> lib/netdev-dpdk.c:3154:5: error: 'rte_eth_dev_detach' is deprecated >> (declared at >> /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native-linuxapp-gcc/include/rte_ethdev.h:1451) >> [-Werror=deprecated-declarations] >> ret = rte_eth_dev_detach(port_id, devname); >> ^ >> lib/netdev-dpdk.c: At top level: >> >> >> //Eelco >> >> >> On 5 Sep 2018, at 12:44, Ophir Munk wrote: >> >>> Hi, >>> I suggest the following trouble-shooting steps: >>> 1. Try Kevin's fix for Niantic (attached here as a non-official >>> patch on top of latest dpdk-hwol branch). >>> Sugesh - can you please try it? >>> 2. Sugesh - do you have the same issue with other NICs than Niantic >>> on latest dpdk-hwol branch? (other NICs used to work a month >>> ago) >>> 3. I will try on my setup. >>> >>> Regards, >>> Ophir >>> >>>> -----Original Message----- >>>> From: Kevin Traynor [mailto:ktraynor@redhat.com] >>>> Sent: Wednesday, September 05, 2018 9:28 AM >>>> To: Chandran, Sugesh <sugesh.chandran@intel.com>; Ophir Munk >>>> <ophirmu@mellanox.com>; ovs-dev@openvswitch.org >>>> Cc: Asaf Penso <asafp@mellanox.com>; Stokes, Ian >>>> <ian.stokes@intel.com>; Ben Pfaff <blp@ovn.org>; Shahaf Shuler >>>> <shahafs@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>; >>>> Olga Shern <olgas@mellanox.com> >>>> Subject: Re: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk >>>> v18.08.0 >>>> >>>> On 09/05/2018 07:44 AM, Chandran, Sugesh wrote: >>>>> Hi Ophir, >>>>> >>>>> Thank you for working on this. >>>>> I tried to run some tests and getting some errors while adding >>>>> physical ports as below. I use Niantic NIC for this test >>>>> >>>>>>>>>>>>>>>>>>>> >>>>> 2018-09-05T06:34:58Z|00069|dpdk|INFO|Device with port_id=0 already >>>>> stopped 2018-09-05T06:34:58Z|00070|dpdk|ERR|Ethdev port_id=0 >>>>> invalid >>>>> rss_hf: 0x3afbc, valid value: 0x38d34 >>>>> 2018-09-05T06:34:58Z|00071|netdev_dpdk|WARN|Interface dpdk0 >>>> eth_dev >>>>> setup error Invalid argument >>>>> 2018-09-05T06:34:58Z|00072|netdev_dpdk|ERR|Interface dpdk0(rxq:2 >>>> txq:2 >>>>> lsc interrupt mode:false) configure error: Invalid argument >>>>> 2018-09-05T06:34:58Z|00073|dpif_netdev|ERR|Failed to set interface >>>>> dpdk0 new configuration 2018-09- >>>> 05T06:34:58Z|00074|bridge|WARN|could >>>>> not add network device dpdk0 to ofproto (No such device) >>>>> ovs-vsctl: Error detected while setting up 'dpdk0': could not add >>>>> network >>>> device dpdk0 to ofproto (No such device). See ovs-vswitchd log for >>>> details. >>>>> ovs-vsctl: The default log directory is >>>>> "/usr/local/var/log/openvswitch". >>>>> >>>> >>>> I didn't have time to compare the below patch with the ones that I >>>> did, but I >>>> saw that this patch was needed for niantic >>>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgit >>>> hub.com%2Fkevintraynor%2Fovs-dpdk- >>>> master%2Fcommit%2F88f46cc5ab338eb4f3ca5db1eacd0effefe4fa0c&d >>>> ata=02%7C01%7Cophirmu%40mellanox.com%7Cd203c89637bf4f27835508d >>>> 61309798e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C63671 >>>> 7328752336932&sdata=L7rWODNNEStIfibwXybu6Ub7Yvn%2BfLHk6VT >>>> we14jxyY%3D&reserved=0 >>>> >>>>>>>>>>>>>>>>>>>>> >>>>> Please find below for my comments. >>>>> >>>>> >>>>> Regards >>>>> _Sugesh >>>>> >>>>>> -----Original Message----- >>>>>> From: Ophir Munk [mailto:ophirmu@mellanox.com] >>>>>> Sent: Monday, September 3, 2018 3:19 PM >>>>>> To: ovs-dev@openvswitch.org >>>>>> Cc: Asaf Penso <asafp@mellanox.com>; Chandran, Sugesh >>>>>> <sugesh.chandran@intel.com>; Stokes, Ian <ian.stokes@intel.com>; >>>>>> Ben >>>>>> Pfaff <blp@ovn.org>; Shahaf Shuler <shahafs@mellanox.com>; Thomas >>>>>> Monjalon <thomas@monjalon.net>; Olga Shern <olgas@mellanox.com>; >>>>>> Ophir Munk <ophirmu@mellanox.com>; Kevin Traynor >>>>>> <ktraynor@redhat.com> >>>>>> Subject: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk >>>>>> v18.08.0 >>>>>> >>>>>> 1. Enable compilation and linkage with dpdk 18.08.0 The following >>>>>> dpdk commits which were introduced after dpdk 17.11.x require OVS >>>>>> updates to accommodate to the dpdk changes. >>>>>> - ce17eddefc20 ("ethdev: introduce Rx queue offloads API") >>>>>> - ab3ce1e0c193 ("ethdev: remove old offload API") >>>>>> - c06ddf9698e0 ("meter: add configuration profile") >>>>>> - e58638c32411 ("ethdev: fix TPID handling in flow API") >>>>>> - cd8c7c7ce241 ("ethdev: replace bus specific struct with generic >>>>>> dev") >>>>>> - ac8d22de2394 ("ethdev: flatten RSS configuration in flow API") >>>>>> >>>>>> 2. Update references to DPDK version in Documentation >>>>>> >>>>>> 3. Update DPDK version in travis' linux-build script >>>>>> >>>>>> Signed-off-by: Ophir Munk <ophirmu@mellanox.com> >>>>>> --- >>>>>> .travis/linux-build.sh | 2 +- >>>>>> Documentation/faq/releases.rst | 2 +- >>>>>> lib/netdev-dpdk.c | 135 >>>>>> +++++++++++++++++++++++++++++-------- >>>> ---- >>>>>> 3 files changed, 98 insertions(+), 41 deletions(-) >>>>>> >>>>>> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh >>>>>> index >>>>>> 4b9fc4a..c60ac71 >>>>>> 100755 >>>>>> --- a/.travis/linux-build.sh >>>>>> +++ b/.travis/linux-build.sh >>>>>> @@ -83,7 +83,7 @@ fi >>>>>> >>>>>> if [ "$DPDK" ]; then >>>>>> if [ -z "$DPDK_VER" ]; then >>>>>> - DPDK_VER="17.11.3" >>>>>> + DPDK_VER="18.08.0" >>>>>> fi >>>>>> install_dpdk $DPDK_VER >>>>>> if [ "$CC" = "clang" ]; then >>>>>> diff --git a/Documentation/faq/releases.rst >>>>>> b/Documentation/faq/releases.rst index 41d41e3..646ae09 100644 >>>>>> --- a/Documentation/faq/releases.rst >>>>>> +++ b/Documentation/faq/releases.rst >>>>>> @@ -168,7 +168,7 @@ Q: What DPDK version does each Open vSwitch >>>>>> release work with? >>>>>> 2.7.x 16.11.7 >>>>>> 2.8.x 17.05.2 >>>>>> 2.9.x 17.11.3 >>>>>> - 2.10.x 17.11.3 >>>>>> + 2.10.x 18.08.0 >>>>>> ============ ======= >>>>>> >>>>>> Q: Are all the DPDK releases that OVS versions work with >>>>>> maintained? >>>>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index >>>>>> f91aa27..80d2af9 >>>>>> 100644 >>>>>> --- a/lib/netdev-dpdk.c >>>>>> +++ b/lib/netdev-dpdk.c >>>>>> @@ -168,11 +168,7 @@ static const struct rte_eth_conf port_conf = >>>>>> { >>>>>> .rxmode = { >>>>>> .mq_mode = ETH_MQ_RX_RSS, >>>>>> .split_hdr_size = 0, >>>>>> - .header_split = 0, /* Header Split disabled */ >>>>>> - .hw_ip_checksum = 0, /* IP checksum offload disabled */ >>>>>> - .hw_vlan_filter = 0, /* VLAN filtering disabled */ >>>>>> - .jumbo_frame = 0, /* Jumbo Frame Support disabled */ >>>>>> - .hw_strip_crc = 0, >>>>>> + .offloads = 0, >>>>>> }, >>>>>> .rx_adv_conf = { >>>>>> .rss_conf = { >>>>>> @@ -364,6 +360,7 @@ struct dpdk_ring { struct ingress_policer { >>>>>> struct rte_meter_srtcm_params app_srtcm_params; >>>>>> struct rte_meter_srtcm in_policer; >>>>>> + struct rte_meter_srtcm_profile in_prof; >>>>>> rte_spinlock_t policer_lock; >>>>>> }; >>>>>> >>>>>> @@ -903,16 +900,17 @@ dpdk_eth_dev_port_config(struct netdev_dpdk >>>>>> *dev, int n_rxq, int n_txq) >>>>>> if (dev->mtu > ETHER_MTU) { >>>>>> rte_eth_dev_info_get(dev->port_id, &info); >>>>>> if (strncmp(info.driver_name, "net_nfp", 7)) { >>>>>> - conf.rxmode.enable_scatter = 1; >>>>>> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_SCATTER; >>>>>> } >>>>>> } >>>>>> >>>>>> conf.intr_conf.lsc = dev->lsc_interrupt_mode; >>>>>> - conf.rxmode.hw_ip_checksum = (dev->hw_ol_features & >>>>>> - NETDEV_RX_CHECKSUM_OFFLOAD) != >>>>>> 0; >>>>>> + conf.rxmode.offloads |= ((dev->hw_ol_features & >>>>>> + NETDEV_RX_CHECKSUM_OFFLOAD) != 0) ? >>>>>> + DEV_RX_OFFLOAD_CHECKSUM : 0; >>>>>> >>>>>> if (dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP) { >>>>>> - conf.rxmode.hw_strip_crc = 1; >>>>>> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP; >>>>>> } >>>>>> >>>>>> /* A device may report more queues than it makes available >>>>>> (this >>>>>> has @@ - >>>>>> 1932,16 +1930,18 @@ netdev_dpdk_eth_tx_burst(struct netdev_dpdk >>>> *dev, >>>>>> int qid, >>>>>> >>>>>> static inline bool >>>>>> netdev_dpdk_policer_pkt_handle(struct rte_meter_srtcm *meter, >>>>>> + struct rte_meter_srtcm_profile >>>>>> + *profile, >>>>>> struct rte_mbuf *pkt, uint64_t >>>>>> time) { >>>>>> uint32_t pkt_len = rte_pktmbuf_pkt_len(pkt) - sizeof(struct >>>>>> ether_hdr); >>>>>> >>>>>> - return rte_meter_srtcm_color_blind_check(meter, time, >>>>>> pkt_len) == >>>>>> + return rte_meter_srtcm_color_blind_check(meter, profile, >>>>>> time, >>>>>> + pkt_len) == >>>>>> e_RTE_METER_GREEN; >>>>>> } >>>>>> >>>>>> static int >>>>>> netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, >>>>>> + struct rte_meter_srtcm_profile *profile, >>>>>> struct rte_mbuf **pkts, int pkt_cnt, >>>>>> bool should_steal) { @@ -1953,7 +1953,8 >>>>>> @@ >>>>>> netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, >>>>>> for (i = 0; i < pkt_cnt; i++) { >>>>>> pkt = pkts[i]; >>>>>> /* Handle current packet */ >>>>>> - if (netdev_dpdk_policer_pkt_handle(meter, pkt, >>>>>> current_time)) { >>>>>> + if (netdev_dpdk_policer_pkt_handle(meter, profile, pkt, >>>>>> + current_time)) >>>>>> { >>>>>> if (cnt != i) { >>>>>> pkts[cnt] = pkt; >>>>>> } >>>>>> @@ -1975,8 +1976,8 @@ ingress_policer_run(struct ingress_policer >>>>>> *policer, struct rte_mbuf **pkts, >>>>>> int cnt = 0; >>>>>> >>>>>> rte_spinlock_lock(&policer->policer_lock); >>>>>> - cnt = netdev_dpdk_policer_run(&policer->in_policer, pkts, >>>>>> - pkt_cnt, should_steal); >>>>>> + cnt = netdev_dpdk_policer_run(&policer->in_policer, >>>>>> &policer- >>>>> in_prof, >>>>>> + pkts, pkt_cnt, should_steal); >>>>>> rte_spinlock_unlock(&policer->policer_lock); >>>>>> >>>>>> return cnt; >>>>>> @@ -2767,8 +2768,15 @@ netdev_dpdk_policer_construct(uint32_t >>>>>> rate, >>>>>> uint32_t burst) >>>>>> policer->app_srtcm_params.cir = rate_bytes; >>>>>> policer->app_srtcm_params.cbs = burst_bytes; >>>>>> policer->app_srtcm_params.ebs = 0; >>>>>> + err = rte_meter_srtcm_profile_config(&policer->in_prof, >>>>>> + >>>>>> &policer->app_srtcm_params); >>>>>> + if (err) { >>>>>> + VLOG_ERR("Could not create rte meter profile for ingress >>>>>> policer"); >>>>>> + free(policer); >>>>>> + return NULL; >>>>>> + } >>>>>> err = rte_meter_srtcm_config(&policer->in_policer, >>>>>> - &policer->app_srtcm_params); >>>>>> + &policer->in_prof); >>>>>> if (err) { >>>>>> VLOG_ERR("Could not create rte meter for ingress >>>>>> policer"); >>>>>> free(policer); >>>>>> @@ -3043,13 +3051,18 @@ netdev_dpdk_get_status(const struct >>>>>> netdev >>>>>> *netdev, struct smap *args) >>>>>> smap_add_format(args, "if_descr", "%s %s", rte_version(), >>>>>> >>>>>> dev_info.driver_name); >>>>>> >>>>>> - if (dev_info.pci_dev) { >>>>>> - smap_add_format(args, "pci-vendor_id", "0x%x", >>>>>> - dev_info.pci_dev->id.vendor_id); >>>>>> - smap_add_format(args, "pci-device_id", "0x%x", >>>>>> - dev_info.pci_dev->id.device_id); >>>>>> + const struct rte_bus *bus; >>>>>> + const struct rte_pci_device *pci_dev; >>>>>> + bus = rte_bus_find_by_device(dev_info.device); >>>>>> + if (bus && !strcmp(bus->name, "pci")) { >>>>>> + pci_dev = RTE_DEV_TO_PCI(dev_info.device); >>>>>> + if (pci_dev) { >>>>>> + smap_add_format(args, "pci-vendor_id", "0x%u", >>>>>> + pci_dev->id.vendor_id); >>>>>> + smap_add_format(args, "pci-device_id", "0x%x", >>>>>> + pci_dev->id.device_id); >>>>>> + } >>>>>> } >>>>>> - >>>>>> return 0; >>>>>> } >>>>>> >>>>>> @@ -3727,6 +3740,7 @@ struct egress_policer { >>>>>> struct qos_conf qos_conf; >>>>>> struct rte_meter_srtcm_params app_srtcm_params; >>>>>> struct rte_meter_srtcm egress_meter; >>>>>> + struct rte_meter_srtcm_profile egress_prof; >>>>>> }; >>>>>> >>>>>> static void >>>>>> @@ -3749,8 +3763,16 @@ egress_policer_qos_construct(const struct >>>> smap >>>>>> *details, >>>>>> policer = xmalloc(sizeof *policer); >>>>>> qos_conf_init(&policer->qos_conf, &egress_policer_ops); >>>>>> egress_policer_details_to_param(details, >>>>>> &policer->app_srtcm_params); >>>>>> + err = rte_meter_srtcm_profile_config(&policer->egress_prof, >>>>>> + >>>>>> &policer->app_srtcm_params); >>>>>> + if (err) { >>>>> [Sugesh] Should we error out the message on failing the profile >>>>> config?? >>>>>> + free(policer); >>>>>> + *conf = NULL; >>>>>> + return -err; >>>>>> + } >>>>>> err = rte_meter_srtcm_config(&policer->egress_meter, >>>>>> - &policer->app_srtcm_params); >>>>>> + &policer->egress_prof); >>>>>> + >>>>>> if (!err) { >>>>>> *conf = &policer->qos_conf; >>>>>> } else { >>>>>> @@ -3803,7 +3825,8 @@ egress_policer_run(struct qos_conf *conf, >>>>>> struct rte_mbuf **pkts, int pkt_cnt, >>>>>> struct egress_policer *policer = >>>>>> CONTAINER_OF(conf, struct egress_policer, qos_conf); >>>>>> >>>>>> - cnt = netdev_dpdk_policer_run(&policer->egress_meter, pkts, >>>>>> + cnt = netdev_dpdk_policer_run(&policer->egress_meter, >>>>>> + &policer->egress_prof, pkts, >>>>>> pkt_cnt, should_steal); >>>>>> >>>>>> return cnt; >>>>>> @@ -4103,15 +4126,15 @@ dump_flow_pattern(struct rte_flow_item >>>> *item) >>>>>> >>>>>> VLOG_DBG("rte flow vlan pattern:\n"); >>>>>> if (vlan_spec) { >>>>>> - VLOG_DBG(" Spec: tpid=0x%"PRIx16", >>>>>> tci=0x%"PRIx16"\n", >>>>>> - ntohs(vlan_spec->tpid), >>>>>> ntohs(vlan_spec->tci)); >>>>>> + VLOG_DBG(" Spec: inner_type=0x%"PRIx16", >>>>>> tci=0x%"PRIx16"\n", >>>>>> + ntohs(vlan_spec->inner_type), >>>>>> + ntohs(vlan_spec->tci)); >>>>>> } else { >>>>>> VLOG_DBG(" Spec = null\n"); >>>>>> } >>>>>> >>>>>> if (vlan_mask) { >>>>>> - VLOG_DBG(" Mask: tpid=0x%"PRIx16", >>>>>> tci=0x%"PRIx16"\n", >>>>>> - vlan_mask->tpid, vlan_mask->tci); >>>>>> + VLOG_DBG(" Mask: inner_type=0x%"PRIx16", >>>> tci=0x%"PRIx16"\n", >>>>>> + vlan_mask->inner_type, vlan_mask->tci); >>>>>> } else { >>>>>> VLOG_DBG(" Mask = null\n"); >>>>>> } >>>>>> @@ -4281,27 +4304,56 @@ add_flow_action(struct flow_actions >>>> *actions, >>>>>> enum rte_flow_action_type type, >>>>>> actions->cnt++; >>>>>> } >>>>>> >>>>>> +/* >>>>>> + * Storage for struct rte_flow_action_rss >>>>>> + * including storage for key and queue array */ #define >>>>>> +MAX_RSS_HASH_KEY_LENGTH 128 #define >>>> MAX_ACTION_RSS_QUEUE_NUM >>>>>> 128 >>>>>> +struct action_rss_data { >>>>>> + struct rte_flow_action_rss conf; >>>>>> + uint16_t queue[MAX_ACTION_RSS_QUEUE_NUM]; >>>>>> + uint8_t key[MAX_RSS_HASH_KEY_LENGTH]; }; >>>>>> + >>>>>> static struct rte_flow_action_rss * >>>>>> add_flow_rss_action(struct flow_actions *actions, >>>>>> struct netdev *netdev) { >>>>>> int i; >>>>>> - struct rte_flow_action_rss *rss; >>>>>> + struct action_rss_data *rss_data; >>>>>> >>>>>> - rss = xmalloc(sizeof(*rss) + sizeof(uint16_t) * >>>>>> netdev->n_rxq); >>>>>> - /* >>>>>> - * Setting it to NULL will let the driver use the default >>>>>> RSS >>>>>> - * configuration we have set: >>>>>> &port_conf.rx_adv_conf.rss_conf. >>>>>> - */ >>>>>> - rss->rss_conf = NULL; >>>>>> - rss->num = netdev->n_rxq; >>>>>> + if (netdev->n_rxq > MAX_ACTION_RSS_QUEUE_NUM) { >>>>>> + VLOG_ERR("Num of rxq (%u) must not be greater " \ >>>>>> + "than max rss num of queues (%u)", >>>>>> + netdev->n_rxq, MAX_ACTION_RSS_QUEUE_NUM); >>>>>> + return NULL; >>>>>> + } >>>>>> >>>>>> - for (i = 0; i < rss->num; i++) { >>>>>> - rss->queue[i] = i; >>>>>> + rss_data = xmalloc(sizeof(struct action_rss_data)); >>>>>> + *rss_data = (struct action_rss_data){ >>>>>> + .conf = (struct rte_flow_action_rss){ >>>>>> + .func = RTE_ETH_HASH_FUNCTION_DEFAULT, >>>>>> + .level = 0, >>>>>> + .types = ETH_RSS_IP, >>>>>> + .key_len = 0, >>>>>> + .queue_num = netdev->n_rxq, >>>>>> + .queue = rss_data->queue, >>>>>> + .key = rss_data->key, >>>>>> + }, >>>>>> + .key = { 0 }, >>>>>> + .queue = { 0 }, >>>>>> + }; >>>>>> + >>>>>> + /* TODO: Override key with default */ >>>>> [Sugesh] Do we really need this TODO?? >>>>>> + >>>>>> + /* Override queue array with default */ >>>>>> + for (i = 0; i < rss_data->conf.queue_num; i++) { >>>>>> + rss_data->queue[i] = i; >>>>>> } >>>>>> >>>>>> - add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, rss); >>>>>> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, >>>>>> + &rss_data->conf); >>>>>> >>>>>> - return rss; >>>>>> + return &rss_data->conf; >>>>>> } >>>>>> >>>>>> static int >>>>>> @@ -4365,7 +4417,7 @@ netdev_dpdk_add_rte_flow_offload(struct >>>> netdev >>>>>> *netdev, >>>>>> vlan_mask.tci = match->wc.masks.vlans[0].tci & >>>>>> ~htons(VLAN_CFI); >>>>>> >>>>>> /* match any protocols */ >>>>>> - vlan_mask.tpid = 0; >>>>>> + vlan_mask.inner_type = 0; >>>>>> >>>>>> add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_VLAN, >>>>>> &vlan_spec, &vlan_mask); @@ -4516,6 >>>>>> +4568,11 @@ >>>>>> end_proto_check: >>>>>> >>>>>> struct rte_flow_action_rss *rss; >>>>>> rss = add_flow_rss_action(&actions, netdev); >>>>>> + if (!rss) { >>>>>> + VLOG_ERR("add_flow_rss_action error.\n"); >>>>> I feel this error can be bit more verbose something like Failed to >>>>> add >>>> rss_flow+actions or something?? What do you think? >>>>>> + ret = -1; >>>>>> + goto out; >>>>>> + } >>>>>> add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL); >>>>>> >>>>>> flow = rte_flow_create(dev->port_id, &flow_attr, >>>>>> patterns.items, >>>>>> -- >>>>>> 1.8.3.1 >>>>> >>> >>> _______________________________________________ >>> dev mailing list >>> dev@openvswitch.org >>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> _______________________________________________ >> dev mailing list >> dev@openvswitch.org >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Great news. It works for me with Mellanox mlx5 as well. I will send a v2 patch with Kevin's fix. Regards, Ophir > -----Original Message----- > From: Eelco Chaudron [mailto:echaudro@redhat.com] > Sent: Wednesday, September 05, 2018 2:33 PM > To: Ophir Munk <ophirmu@mellanox.com> > Cc: Thomas Monjalon <thomas@monjalon.net>; ovs-dev@openvswitch.org; > Asaf Penso <asafp@mellanox.com>; Shahaf Shuler > <shahafs@mellanox.com> > Subject: Re: [ovs-dev] [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk > v18.08.0 > > And one final note… I though I applied Kevin’s patch, but I applied it to the > wrong directory… > > With Kevin’s patch applied it works, root and non root OVS. > > //Eelco > > > On 5 Sep 2018, at 14:49, Eelco Chaudron wrote: > > > Forgot to cut/paste the log: > > > > 2018-09-05T12:47:48.462Z|00132|dpdk|INFO|Device with port_id=1 > already > > stopped 2018-09-05T12:47:48.462Z|00133|dpdk|ERR|Ethdev port_id=1 > > invalid > > rss_hf: 0x3afbc, valid value: 0x7ef8 > > 2018-09-05T12:47:48.462Z|00134|netdev_dpdk|WARN|Interface dpdk1 > > eth_dev setup error Invalid argument > > 2018-09-05T12:47:48.462Z|00135|netdev_dpdk|ERR|Interface > dpdk1(rxq:1 > > txq:3 lsc interrupt mode:false) configure error: Invalid argument > > 2018-09-05T12:47:48.462Z|00136|dpif_netdev|ERR|Failed to set interface > > dpdk1 new configuration > > > > Guess this will help… > > > > On 5 Sep 2018, at 14:35, Eelco Chaudron wrote: > > > >> Hi Ophir, > >> > >> I get the same error when trying this with an XL710, wanted to see if > >> the patch would work for non-root users. > >> Did not further troubleshoot it. > >> > >> Also I get the following compile warnings(errors): > >> > >> mv -f $depbase.Tpo $depbase.Po > >> lib/netdev-dpdk.c: In function 'netdev_dpdk_destruct': > >> lib/netdev-dpdk.c:1331:9: error: 'rte_eth_dev_detach' is deprecated > >> (declared at > >> /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native- > linuxapp-gcc/ > >> include/rte_ethdev.h:1451) > >> [-Werror=deprecated-declarations] > >> if (rte_eth_dev_detach(dev->port_id, devname) < 0) { > >> ^ > >> lib/netdev-dpdk.c: In function 'netdev_dpdk_process_devargs': > >> lib/netdev-dpdk.c:1624:13: error: 'rte_eth_dev_attach' is deprecated > >> (declared at > >> /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native- > linuxapp-gcc/ > >> include/rte_ethdev.h:1435) > >> [-Werror=deprecated-declarations] > >> if (!rte_eth_dev_attach(devargs, &new_port_id)) { > >> ^ > >> lib/netdev-dpdk.c: In function 'netdev_dpdk_detach': > >> lib/netdev-dpdk.c:3154:5: error: 'rte_eth_dev_detach' is deprecated > >> (declared at > >> /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native- > linuxapp-gcc/ > >> include/rte_ethdev.h:1451) > >> [-Werror=deprecated-declarations] > >> ret = rte_eth_dev_detach(port_id, devname); > >> ^ > >> lib/netdev-dpdk.c: At top level: > >> > >> > >> //Eelco > >> > >> > >> On 5 Sep 2018, at 12:44, Ophir Munk wrote: > >> > >>> Hi, > >>> I suggest the following trouble-shooting steps: > >>> 1. Try Kevin's fix for Niantic (attached here as a non-official > >>> patch on top of latest dpdk-hwol branch). > >>> Sugesh - can you please try it? > >>> 2. Sugesh - do you have the same issue with other NICs than Niantic > >>> on latest dpdk-hwol branch? (other NICs used to work a month > >>> ago) > >>> 3. I will try on my setup. > >>> > >>> Regards, > >>> Ophir > >>> > >>>> -----Original Message----- > >>>> From: Kevin Traynor [mailto:ktraynor@redhat.com] > >>>> Sent: Wednesday, September 05, 2018 9:28 AM > >>>> To: Chandran, Sugesh <sugesh.chandran@intel.com>; Ophir Munk > >>>> <ophirmu@mellanox.com>; ovs-dev@openvswitch.org > >>>> Cc: Asaf Penso <asafp@mellanox.com>; Stokes, Ian > >>>> <ian.stokes@intel.com>; Ben Pfaff <blp@ovn.org>; Shahaf Shuler > >>>> <shahafs@mellanox.com>; Thomas Monjalon > <thomas@monjalon.net>; Olga > >>>> Shern <olgas@mellanox.com> > >>>> Subject: Re: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk > >>>> v18.08.0 > >>>> > >>>> On 09/05/2018 07:44 AM, Chandran, Sugesh wrote: > >>>>> Hi Ophir, > >>>>> > >>>>> Thank you for working on this. > >>>>> I tried to run some tests and getting some errors while adding > >>>>> physical ports as below. I use Niantic NIC for this test > >>>>> > >>>>>>>>>>>>>>>>>>>> > >>>>> 2018-09-05T06:34:58Z|00069|dpdk|INFO|Device with port_id=0 > already > >>>>> stopped 2018-09-05T06:34:58Z|00070|dpdk|ERR|Ethdev port_id=0 > >>>>> invalid > >>>>> rss_hf: 0x3afbc, valid value: 0x38d34 > >>>>> 2018-09-05T06:34:58Z|00071|netdev_dpdk|WARN|Interface dpdk0 > >>>> eth_dev > >>>>> setup error Invalid argument > >>>>> 2018-09-05T06:34:58Z|00072|netdev_dpdk|ERR|Interface > dpdk0(rxq:2 > >>>> txq:2 > >>>>> lsc interrupt mode:false) configure error: Invalid argument > >>>>> 2018-09-05T06:34:58Z|00073|dpif_netdev|ERR|Failed to set > interface > >>>>> dpdk0 new configuration 2018-09- > >>>> 05T06:34:58Z|00074|bridge|WARN|could > >>>>> not add network device dpdk0 to ofproto (No such device) > >>>>> ovs-vsctl: Error detected while setting up 'dpdk0': could not add > >>>>> network > >>>> device dpdk0 to ofproto (No such device). See ovs-vswitchd log for > >>>> details. > >>>>> ovs-vsctl: The default log directory is > >>>>> "/usr/local/var/log/openvswitch". > >>>>> > >>>> > >>>> I didn't have time to compare the below patch with the ones that I > >>>> did, but I saw that this patch was needed for niantic > >>>> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2F > >>>> git > >>>> hub.com%2Fkevintraynor%2Fovs-dpdk- > >>>> > master%2Fcommit%2F88f46cc5ab338eb4f3ca5db1eacd0effefe4fa0c&d > >>>> > ata=02%7C01%7Cophirmu%40mellanox.com%7Cd203c89637bf4f27835508d > >>>> > 61309798e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C63671 > >>>> > 7328752336932&sdata=L7rWODNNEStIfibwXybu6Ub7Yvn%2BfLHk6VT > >>>> we14jxyY%3D&reserved=0 > >>>> > >>>>>>>>>>>>>>>>>>>>> > >>>>> Please find below for my comments. > >>>>> > >>>>> > >>>>> Regards > >>>>> _Sugesh > >>>>> > >>>>>> -----Original Message----- > >>>>>> From: Ophir Munk [mailto:ophirmu@mellanox.com] > >>>>>> Sent: Monday, September 3, 2018 3:19 PM > >>>>>> To: ovs-dev@openvswitch.org > >>>>>> Cc: Asaf Penso <asafp@mellanox.com>; Chandran, Sugesh > >>>>>> <sugesh.chandran@intel.com>; Stokes, Ian <ian.stokes@intel.com>; > >>>>>> Ben Pfaff <blp@ovn.org>; Shahaf Shuler <shahafs@mellanox.com>; > >>>>>> Thomas Monjalon <thomas@monjalon.net>; Olga Shern > >>>>>> <olgas@mellanox.com>; Ophir Munk <ophirmu@mellanox.com>; > Kevin > >>>>>> Traynor <ktraynor@redhat.com> > >>>>>> Subject: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk > >>>>>> v18.08.0 > >>>>>> > >>>>>> 1. Enable compilation and linkage with dpdk 18.08.0 The following > >>>>>> dpdk commits which were introduced after dpdk 17.11.x require OVS > >>>>>> updates to accommodate to the dpdk changes. > >>>>>> - ce17eddefc20 ("ethdev: introduce Rx queue offloads API") > >>>>>> - ab3ce1e0c193 ("ethdev: remove old offload API") > >>>>>> - c06ddf9698e0 ("meter: add configuration profile") > >>>>>> - e58638c32411 ("ethdev: fix TPID handling in flow API") > >>>>>> - cd8c7c7ce241 ("ethdev: replace bus specific struct with generic > >>>>>> dev") > >>>>>> - ac8d22de2394 ("ethdev: flatten RSS configuration in flow API") > >>>>>> > >>>>>> 2. Update references to DPDK version in Documentation > >>>>>> > >>>>>> 3. Update DPDK version in travis' linux-build script > >>>>>> > >>>>>> Signed-off-by: Ophir Munk <ophirmu@mellanox.com> > >>>>>> --- > >>>>>> .travis/linux-build.sh | 2 +- > >>>>>> Documentation/faq/releases.rst | 2 +- > >>>>>> lib/netdev-dpdk.c | 135 > >>>>>> +++++++++++++++++++++++++++++-------- > >>>> ---- > >>>>>> 3 files changed, 98 insertions(+), 41 deletions(-) > >>>>>> > >>>>>> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh > >>>>>> index > >>>>>> 4b9fc4a..c60ac71 > >>>>>> 100755 > >>>>>> --- a/.travis/linux-build.sh > >>>>>> +++ b/.travis/linux-build.sh > >>>>>> @@ -83,7 +83,7 @@ fi > >>>>>> > >>>>>> if [ "$DPDK" ]; then > >>>>>> if [ -z "$DPDK_VER" ]; then > >>>>>> - DPDK_VER="17.11.3" > >>>>>> + DPDK_VER="18.08.0" > >>>>>> fi > >>>>>> install_dpdk $DPDK_VER > >>>>>> if [ "$CC" = "clang" ]; then diff --git > >>>>>> a/Documentation/faq/releases.rst > b/Documentation/faq/releases.rst > >>>>>> index 41d41e3..646ae09 100644 > >>>>>> --- a/Documentation/faq/releases.rst > >>>>>> +++ b/Documentation/faq/releases.rst > >>>>>> @@ -168,7 +168,7 @@ Q: What DPDK version does each Open > vSwitch > >>>>>> release work with? > >>>>>> 2.7.x 16.11.7 > >>>>>> 2.8.x 17.05.2 > >>>>>> 2.9.x 17.11.3 > >>>>>> - 2.10.x 17.11.3 > >>>>>> + 2.10.x 18.08.0 > >>>>>> ============ ======= > >>>>>> > >>>>>> Q: Are all the DPDK releases that OVS versions work with > >>>>>> maintained? > >>>>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > >>>>>> f91aa27..80d2af9 > >>>>>> 100644 > >>>>>> --- a/lib/netdev-dpdk.c > >>>>>> +++ b/lib/netdev-dpdk.c > >>>>>> @@ -168,11 +168,7 @@ static const struct rte_eth_conf port_conf > = > >>>>>> { > >>>>>> .rxmode = { > >>>>>> .mq_mode = ETH_MQ_RX_RSS, > >>>>>> .split_hdr_size = 0, > >>>>>> - .header_split = 0, /* Header Split disabled */ > >>>>>> - .hw_ip_checksum = 0, /* IP checksum offload disabled */ > >>>>>> - .hw_vlan_filter = 0, /* VLAN filtering disabled */ > >>>>>> - .jumbo_frame = 0, /* Jumbo Frame Support disabled */ > >>>>>> - .hw_strip_crc = 0, > >>>>>> + .offloads = 0, > >>>>>> }, > >>>>>> .rx_adv_conf = { > >>>>>> .rss_conf = { > >>>>>> @@ -364,6 +360,7 @@ struct dpdk_ring { struct ingress_policer { > >>>>>> struct rte_meter_srtcm_params app_srtcm_params; > >>>>>> struct rte_meter_srtcm in_policer; > >>>>>> + struct rte_meter_srtcm_profile in_prof; > >>>>>> rte_spinlock_t policer_lock; }; > >>>>>> > >>>>>> @@ -903,16 +900,17 @@ dpdk_eth_dev_port_config(struct > netdev_dpdk > >>>>>> *dev, int n_rxq, int n_txq) > >>>>>> if (dev->mtu > ETHER_MTU) { > >>>>>> rte_eth_dev_info_get(dev->port_id, &info); > >>>>>> if (strncmp(info.driver_name, "net_nfp", 7)) { > >>>>>> - conf.rxmode.enable_scatter = 1; > >>>>>> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_SCATTER; > >>>>>> } > >>>>>> } > >>>>>> > >>>>>> conf.intr_conf.lsc = dev->lsc_interrupt_mode; > >>>>>> - conf.rxmode.hw_ip_checksum = (dev->hw_ol_features & > >>>>>> - NETDEV_RX_CHECKSUM_OFFLOAD) != > >>>>>> 0; > >>>>>> + conf.rxmode.offloads |= ((dev->hw_ol_features & > >>>>>> + NETDEV_RX_CHECKSUM_OFFLOAD) != 0) ? > >>>>>> + DEV_RX_OFFLOAD_CHECKSUM : 0; > >>>>>> > >>>>>> if (dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP) { > >>>>>> - conf.rxmode.hw_strip_crc = 1; > >>>>>> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP; > >>>>>> } > >>>>>> > >>>>>> /* A device may report more queues than it makes available > >>>>>> (this has @@ - > >>>>>> 1932,16 +1930,18 @@ netdev_dpdk_eth_tx_burst(struct > netdev_dpdk > >>>> *dev, > >>>>>> int qid, > >>>>>> > >>>>>> static inline bool > >>>>>> netdev_dpdk_policer_pkt_handle(struct rte_meter_srtcm *meter, > >>>>>> + struct rte_meter_srtcm_profile > >>>>>> + *profile, > >>>>>> struct rte_mbuf *pkt, uint64_t > >>>>>> time) { > >>>>>> uint32_t pkt_len = rte_pktmbuf_pkt_len(pkt) - sizeof(struct > >>>>>> ether_hdr); > >>>>>> > >>>>>> - return rte_meter_srtcm_color_blind_check(meter, time, > >>>>>> pkt_len) == > >>>>>> + return rte_meter_srtcm_color_blind_check(meter, profile, > >>>>>> time, > >>>>>> + pkt_len) == > >>>>>> > >>>>>> e_RTE_METER_GREEN; } > >>>>>> > >>>>>> static int > >>>>>> netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, > >>>>>> + struct rte_meter_srtcm_profile *profile, > >>>>>> struct rte_mbuf **pkts, int pkt_cnt, > >>>>>> bool should_steal) { @@ -1953,7 +1953,8 > >>>>>> @@ netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, > >>>>>> for (i = 0; i < pkt_cnt; i++) { > >>>>>> pkt = pkts[i]; > >>>>>> /* Handle current packet */ > >>>>>> - if (netdev_dpdk_policer_pkt_handle(meter, pkt, > >>>>>> current_time)) { > >>>>>> + if (netdev_dpdk_policer_pkt_handle(meter, profile, pkt, > >>>>>> + current_time)) > >>>>>> { > >>>>>> if (cnt != i) { > >>>>>> pkts[cnt] = pkt; > >>>>>> } > >>>>>> @@ -1975,8 +1976,8 @@ ingress_policer_run(struct ingress_policer > >>>>>> *policer, struct rte_mbuf **pkts, > >>>>>> int cnt = 0; > >>>>>> > >>>>>> rte_spinlock_lock(&policer->policer_lock); > >>>>>> - cnt = netdev_dpdk_policer_run(&policer->in_policer, pkts, > >>>>>> - pkt_cnt, should_steal); > >>>>>> + cnt = netdev_dpdk_policer_run(&policer->in_policer, > >>>>>> &policer- > >>>>> in_prof, > >>>>>> + pkts, pkt_cnt, should_steal); > >>>>>> rte_spinlock_unlock(&policer->policer_lock); > >>>>>> > >>>>>> return cnt; > >>>>>> @@ -2767,8 +2768,15 @@ netdev_dpdk_policer_construct(uint32_t > >>>>>> rate, > >>>>>> uint32_t burst) > >>>>>> policer->app_srtcm_params.cir = rate_bytes; > >>>>>> policer->app_srtcm_params.cbs = burst_bytes; > >>>>>> policer->app_srtcm_params.ebs = 0; > >>>>>> + err = rte_meter_srtcm_profile_config(&policer->in_prof, > >>>>>> + > >>>>>> &policer->app_srtcm_params); > >>>>>> + if (err) { > >>>>>> + VLOG_ERR("Could not create rte meter profile for ingress > >>>>>> policer"); > >>>>>> + free(policer); > >>>>>> + return NULL; > >>>>>> + } > >>>>>> err = rte_meter_srtcm_config(&policer->in_policer, > >>>>>> - &policer->app_srtcm_params); > >>>>>> + &policer->in_prof); > >>>>>> if (err) { > >>>>>> VLOG_ERR("Could not create rte meter for ingress > >>>>>> policer"); > >>>>>> free(policer); > >>>>>> @@ -3043,13 +3051,18 @@ netdev_dpdk_get_status(const struct > >>>>>> netdev *netdev, struct smap *args) > >>>>>> smap_add_format(args, "if_descr", "%s %s", rte_version(), > >>>>>> > >>>>>> dev_info.driver_name); > >>>>>> > >>>>>> - if (dev_info.pci_dev) { > >>>>>> - smap_add_format(args, "pci-vendor_id", "0x%x", > >>>>>> - dev_info.pci_dev->id.vendor_id); > >>>>>> - smap_add_format(args, "pci-device_id", "0x%x", > >>>>>> - dev_info.pci_dev->id.device_id); > >>>>>> + const struct rte_bus *bus; > >>>>>> + const struct rte_pci_device *pci_dev; > >>>>>> + bus = rte_bus_find_by_device(dev_info.device); > >>>>>> + if (bus && !strcmp(bus->name, "pci")) { > >>>>>> + pci_dev = RTE_DEV_TO_PCI(dev_info.device); > >>>>>> + if (pci_dev) { > >>>>>> + smap_add_format(args, "pci-vendor_id", "0x%u", > >>>>>> + pci_dev->id.vendor_id); > >>>>>> + smap_add_format(args, "pci-device_id", "0x%x", > >>>>>> + pci_dev->id.device_id); > >>>>>> + } > >>>>>> } > >>>>>> - > >>>>>> return 0; > >>>>>> } > >>>>>> > >>>>>> @@ -3727,6 +3740,7 @@ struct egress_policer { > >>>>>> struct qos_conf qos_conf; > >>>>>> struct rte_meter_srtcm_params app_srtcm_params; > >>>>>> struct rte_meter_srtcm egress_meter; > >>>>>> + struct rte_meter_srtcm_profile egress_prof; > >>>>>> }; > >>>>>> > >>>>>> static void > >>>>>> @@ -3749,8 +3763,16 @@ egress_policer_qos_construct(const > struct > >>>> smap > >>>>>> *details, > >>>>>> policer = xmalloc(sizeof *policer); > >>>>>> qos_conf_init(&policer->qos_conf, &egress_policer_ops); > >>>>>> egress_policer_details_to_param(details, > >>>>>> &policer->app_srtcm_params); > >>>>>> + err = rte_meter_srtcm_profile_config(&policer->egress_prof, > >>>>>> + > >>>>>> &policer->app_srtcm_params); > >>>>>> + if (err) { > >>>>> [Sugesh] Should we error out the message on failing the profile > >>>>> config?? > >>>>>> + free(policer); > >>>>>> + *conf = NULL; > >>>>>> + return -err; > >>>>>> + } > >>>>>> err = rte_meter_srtcm_config(&policer->egress_meter, > >>>>>> - &policer->app_srtcm_params); > >>>>>> + &policer->egress_prof); > >>>>>> + > >>>>>> if (!err) { > >>>>>> *conf = &policer->qos_conf; > >>>>>> } else { > >>>>>> @@ -3803,7 +3825,8 @@ egress_policer_run(struct qos_conf *conf, > >>>>>> struct rte_mbuf **pkts, int pkt_cnt, > >>>>>> struct egress_policer *policer = > >>>>>> CONTAINER_OF(conf, struct egress_policer, qos_conf); > >>>>>> > >>>>>> - cnt = netdev_dpdk_policer_run(&policer->egress_meter, pkts, > >>>>>> + cnt = netdev_dpdk_policer_run(&policer->egress_meter, > >>>>>> + &policer->egress_prof, pkts, > >>>>>> pkt_cnt, should_steal); > >>>>>> > >>>>>> return cnt; > >>>>>> @@ -4103,15 +4126,15 @@ dump_flow_pattern(struct > rte_flow_item > >>>> *item) > >>>>>> > >>>>>> VLOG_DBG("rte flow vlan pattern:\n"); > >>>>>> if (vlan_spec) { > >>>>>> - VLOG_DBG(" Spec: tpid=0x%"PRIx16", > >>>>>> tci=0x%"PRIx16"\n", > >>>>>> - ntohs(vlan_spec->tpid), > >>>>>> ntohs(vlan_spec->tci)); > >>>>>> + VLOG_DBG(" Spec: inner_type=0x%"PRIx16", > >>>>>> tci=0x%"PRIx16"\n", > >>>>>> + ntohs(vlan_spec->inner_type), > >>>>>> + ntohs(vlan_spec->tci)); > >>>>>> } else { > >>>>>> VLOG_DBG(" Spec = null\n"); > >>>>>> } > >>>>>> > >>>>>> if (vlan_mask) { > >>>>>> - VLOG_DBG(" Mask: tpid=0x%"PRIx16", > >>>>>> tci=0x%"PRIx16"\n", > >>>>>> - vlan_mask->tpid, vlan_mask->tci); > >>>>>> + VLOG_DBG(" Mask: inner_type=0x%"PRIx16", > >>>> tci=0x%"PRIx16"\n", > >>>>>> + vlan_mask->inner_type, vlan_mask->tci); > >>>>>> } else { > >>>>>> VLOG_DBG(" Mask = null\n"); > >>>>>> } > >>>>>> @@ -4281,27 +4304,56 @@ add_flow_action(struct flow_actions > >>>> *actions, > >>>>>> enum rte_flow_action_type type, > >>>>>> actions->cnt++; > >>>>>> } > >>>>>> > >>>>>> +/* > >>>>>> + * Storage for struct rte_flow_action_rss > >>>>>> + * including storage for key and queue array */ #define > >>>>>> +MAX_RSS_HASH_KEY_LENGTH 128 #define > >>>> MAX_ACTION_RSS_QUEUE_NUM > >>>>>> 128 > >>>>>> +struct action_rss_data { > >>>>>> + struct rte_flow_action_rss conf; > >>>>>> + uint16_t queue[MAX_ACTION_RSS_QUEUE_NUM]; > >>>>>> + uint8_t key[MAX_RSS_HASH_KEY_LENGTH]; }; > >>>>>> + > >>>>>> static struct rte_flow_action_rss * add_flow_rss_action(struct > >>>>>> flow_actions *actions, > >>>>>> struct netdev *netdev) { > >>>>>> int i; > >>>>>> - struct rte_flow_action_rss *rss; > >>>>>> + struct action_rss_data *rss_data; > >>>>>> > >>>>>> - rss = xmalloc(sizeof(*rss) + sizeof(uint16_t) * > >>>>>> netdev->n_rxq); > >>>>>> - /* > >>>>>> - * Setting it to NULL will let the driver use the default > >>>>>> RSS > >>>>>> - * configuration we have set: > >>>>>> &port_conf.rx_adv_conf.rss_conf. > >>>>>> - */ > >>>>>> - rss->rss_conf = NULL; > >>>>>> - rss->num = netdev->n_rxq; > >>>>>> + if (netdev->n_rxq > MAX_ACTION_RSS_QUEUE_NUM) { > >>>>>> + VLOG_ERR("Num of rxq (%u) must not be greater " \ > >>>>>> + "than max rss num of queues (%u)", > >>>>>> + netdev->n_rxq, MAX_ACTION_RSS_QUEUE_NUM); > >>>>>> + return NULL; > >>>>>> + } > >>>>>> > >>>>>> - for (i = 0; i < rss->num; i++) { > >>>>>> - rss->queue[i] = i; > >>>>>> + rss_data = xmalloc(sizeof(struct action_rss_data)); > >>>>>> + *rss_data = (struct action_rss_data){ > >>>>>> + .conf = (struct rte_flow_action_rss){ > >>>>>> + .func = RTE_ETH_HASH_FUNCTION_DEFAULT, > >>>>>> + .level = 0, > >>>>>> + .types = ETH_RSS_IP, > >>>>>> + .key_len = 0, > >>>>>> + .queue_num = netdev->n_rxq, > >>>>>> + .queue = rss_data->queue, > >>>>>> + .key = rss_data->key, > >>>>>> + }, > >>>>>> + .key = { 0 }, > >>>>>> + .queue = { 0 }, > >>>>>> + }; > >>>>>> + > >>>>>> + /* TODO: Override key with default */ > >>>>> [Sugesh] Do we really need this TODO?? > >>>>>> + > >>>>>> + /* Override queue array with default */ > >>>>>> + for (i = 0; i < rss_data->conf.queue_num; i++) { > >>>>>> + rss_data->queue[i] = i; > >>>>>> } > >>>>>> > >>>>>> - add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, rss); > >>>>>> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, > >>>>>> + &rss_data->conf); > >>>>>> > >>>>>> - return rss; > >>>>>> + return &rss_data->conf; > >>>>>> } > >>>>>> > >>>>>> static int > >>>>>> @@ -4365,7 +4417,7 @@ > netdev_dpdk_add_rte_flow_offload(struct > >>>> netdev > >>>>>> *netdev, > >>>>>> vlan_mask.tci = match->wc.masks.vlans[0].tci & > >>>>>> ~htons(VLAN_CFI); > >>>>>> > >>>>>> /* match any protocols */ > >>>>>> - vlan_mask.tpid = 0; > >>>>>> + vlan_mask.inner_type = 0; > >>>>>> > >>>>>> add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_VLAN, > >>>>>> &vlan_spec, &vlan_mask); @@ -4516,6 > >>>>>> +4568,11 @@ > >>>>>> end_proto_check: > >>>>>> > >>>>>> struct rte_flow_action_rss *rss; > >>>>>> rss = add_flow_rss_action(&actions, netdev); > >>>>>> + if (!rss) { > >>>>>> + VLOG_ERR("add_flow_rss_action error.\n"); > >>>>> I feel this error can be bit more verbose something like Failed to > >>>>> add > >>>> rss_flow+actions or something?? What do you think? > >>>>>> + ret = -1; > >>>>>> + goto out; > >>>>>> + } > >>>>>> add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, > NULL); > >>>>>> > >>>>>> flow = rte_flow_create(dev->port_id, &flow_attr, > >>>>>> patterns.items, > >>>>>> -- > >>>>>> 1.8.3.1 > >>>>> > >>> > >>> _______________________________________________ > >>> dev mailing list > >>> dev@openvswitch.org > >>> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fm > >>> ail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs- > dev&data=02%7C01% > >>> > 7Cophirmu%40mellanox.com%7Ce6c08e7bb58d40807f1408d6133416c2%7C > a65297 > >>> > 1c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636717511775064323&s > data=qU > >>> > zRNGAXN%2F9EqDJplTPvs82QP8tRte6e3GW5i01IEYE%3D&reserved=0 > >> _______________________________________________ > >> dev mailing list > >> dev@openvswitch.org > >> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fma > >> il.openvswitch.org%2Fmailman%2Flistinfo%2Fovs- > dev&data=02%7C01%7C > >> > ophirmu%40mellanox.com%7Ce6c08e7bb58d40807f1408d6133416c2%7Ca6 > 52971c7 > >> > d2e4d9ba6a4d149256f461b%7C0%7C0%7C636717511775064323&sdat > a=qUzRNG > >> AXN%2F9EqDJplTPvs82QP8tRte6e3GW5i01IEYE%3D&reserved=0 > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmai > > l.openvswitch.org%2Fmailman%2Flistinfo%2Fovs- > dev&data=02%7C01%7Cop > > > hirmu%40mellanox.com%7Ce6c08e7bb58d40807f1408d6133416c2%7Ca652 > 971c7d2e > > > 4d9ba6a4d149256f461b%7C0%7C0%7C636717511775064323&sdata=q > UzRNGAXN% > > 2F9EqDJplTPvs82QP8tRte6e3GW5i01IEYE%3D&reserved=0
I’ll do a proper review on the v2, please also fix the deprecated warningsngs (see below). //Eelco On 5 Sep 2018, at 16:45, Ophir Munk wrote: > Great news. > It works for me with Mellanox mlx5 as well. > I will send a v2 patch with Kevin's fix. > > Regards, > Ophir > >> -----Original Message----- >> From: Eelco Chaudron [mailto:echaudro@redhat.com] >> Sent: Wednesday, September 05, 2018 2:33 PM >> To: Ophir Munk <ophirmu@mellanox.com> >> Cc: Thomas Monjalon <thomas@monjalon.net>; ovs-dev@openvswitch.org; >> Asaf Penso <asafp@mellanox.com>; Shahaf Shuler >> <shahafs@mellanox.com> >> Subject: Re: [ovs-dev] [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to >> dpdk >> v18.08.0 >> >> And one final note… I though I applied Kevin’s patch, but I >> applied it to the >> wrong directory… >> >> With Kevin’s patch applied it works, root and non root OVS. >> >> //Eelco >> >> >> On 5 Sep 2018, at 14:49, Eelco Chaudron wrote: >> >>> Forgot to cut/paste the log: >>> >>> 2018-09-05T12:47:48.462Z|00132|dpdk|INFO|Device with port_id=1 >> already >>> stopped 2018-09-05T12:47:48.462Z|00133|dpdk|ERR|Ethdev port_id=1 >>> invalid >>> rss_hf: 0x3afbc, valid value: 0x7ef8 >>> 2018-09-05T12:47:48.462Z|00134|netdev_dpdk|WARN|Interface dpdk1 >>> eth_dev setup error Invalid argument >>> 2018-09-05T12:47:48.462Z|00135|netdev_dpdk|ERR|Interface >> dpdk1(rxq:1 >>> txq:3 lsc interrupt mode:false) configure error: Invalid argument >>> 2018-09-05T12:47:48.462Z|00136|dpif_netdev|ERR|Failed to set >>> interface >>> dpdk1 new configuration >>> >>> Guess this will help… >>> >>> On 5 Sep 2018, at 14:35, Eelco Chaudron wrote: >>> >>>> Hi Ophir, >>>> >>>> I get the same error when trying this with an XL710, wanted to see >>>> if >>>> the patch would work for non-root users. >>>> Did not further troubleshoot it. >>>> >>>> Also I get the following compile warnings(errors): >>>> >>>> mv -f $depbase.Tpo $depbase.Po >>>> lib/netdev-dpdk.c: In function 'netdev_dpdk_destruct': >>>> lib/netdev-dpdk.c:1331:9: error: 'rte_eth_dev_detach' is deprecated >>>> (declared at >>>> /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native- >> linuxapp-gcc/ >>>> include/rte_ethdev.h:1451) >>>> [-Werror=deprecated-declarations] >>>> if (rte_eth_dev_detach(dev->port_id, devname) < 0) { >>>> ^ >>>> lib/netdev-dpdk.c: In function 'netdev_dpdk_process_devargs': >>>> lib/netdev-dpdk.c:1624:13: error: 'rte_eth_dev_attach' is >>>> deprecated >>>> (declared at >>>> /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native- >> linuxapp-gcc/ >>>> include/rte_ethdev.h:1435) >>>> [-Werror=deprecated-declarations] >>>> if (!rte_eth_dev_attach(devargs, &new_port_id)) { >>>> ^ >>>> lib/netdev-dpdk.c: In function 'netdev_dpdk_detach': >>>> lib/netdev-dpdk.c:3154:5: error: 'rte_eth_dev_detach' is deprecated >>>> (declared at >>>> /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native- >> linuxapp-gcc/ >>>> include/rte_ethdev.h:1451) >>>> [-Werror=deprecated-declarations] >>>> ret = rte_eth_dev_detach(port_id, devname); >>>> ^ >>>> lib/netdev-dpdk.c: At top level: >>>> >>>> >>>> //Eelco >>>> >>>> >>>> On 5 Sep 2018, at 12:44, Ophir Munk wrote: >>>> >>>>> Hi, >>>>> I suggest the following trouble-shooting steps: >>>>> 1. Try Kevin's fix for Niantic (attached here as a non-official >>>>> patch on top of latest dpdk-hwol branch). >>>>> Sugesh - can you please try it? >>>>> 2. Sugesh - do you have the same issue with other NICs than >>>>> Niantic >>>>> on latest dpdk-hwol branch? (other NICs used to work a month >>>>> ago) >>>>> 3. I will try on my setup. >>>>> >>>>> Regards, >>>>> Ophir >>>>> >>>>>> -----Original Message----- >>>>>> From: Kevin Traynor [mailto:ktraynor@redhat.com] >>>>>> Sent: Wednesday, September 05, 2018 9:28 AM >>>>>> To: Chandran, Sugesh <sugesh.chandran@intel.com>; Ophir Munk >>>>>> <ophirmu@mellanox.com>; ovs-dev@openvswitch.org >>>>>> Cc: Asaf Penso <asafp@mellanox.com>; Stokes, Ian >>>>>> <ian.stokes@intel.com>; Ben Pfaff <blp@ovn.org>; Shahaf Shuler >>>>>> <shahafs@mellanox.com>; Thomas Monjalon >> <thomas@monjalon.net>; Olga >>>>>> Shern <olgas@mellanox.com> >>>>>> Subject: Re: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk >>>>>> v18.08.0 >>>>>> >>>>>> On 09/05/2018 07:44 AM, Chandran, Sugesh wrote: >>>>>>> Hi Ophir, >>>>>>> >>>>>>> Thank you for working on this. >>>>>>> I tried to run some tests and getting some errors while adding >>>>>>> physical ports as below. I use Niantic NIC for this test >>>>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>>> 2018-09-05T06:34:58Z|00069|dpdk|INFO|Device with port_id=0 >> already >>>>>>> stopped 2018-09-05T06:34:58Z|00070|dpdk|ERR|Ethdev port_id=0 >>>>>>> invalid >>>>>>> rss_hf: 0x3afbc, valid value: 0x38d34 >>>>>>> 2018-09-05T06:34:58Z|00071|netdev_dpdk|WARN|Interface dpdk0 >>>>>> eth_dev >>>>>>> setup error Invalid argument >>>>>>> 2018-09-05T06:34:58Z|00072|netdev_dpdk|ERR|Interface >> dpdk0(rxq:2 >>>>>> txq:2 >>>>>>> lsc interrupt mode:false) configure error: Invalid argument >>>>>>> 2018-09-05T06:34:58Z|00073|dpif_netdev|ERR|Failed to set >> interface >>>>>>> dpdk0 new configuration 2018-09- >>>>>> 05T06:34:58Z|00074|bridge|WARN|could >>>>>>> not add network device dpdk0 to ofproto (No such device) >>>>>>> ovs-vsctl: Error detected while setting up 'dpdk0': could not >>>>>>> add >>>>>>> network >>>>>> device dpdk0 to ofproto (No such device). See ovs-vswitchd log >>>>>> for >>>>>> details. >>>>>>> ovs-vsctl: The default log directory is >>>>>>> "/usr/local/var/log/openvswitch". >>>>>>> >>>>>> >>>>>> I didn't have time to compare the below patch with the ones that >>>>>> I >>>>>> did, but I saw that this patch was needed for niantic >>>>>> >> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2F >>>>>> git >>>>>> hub.com%2Fkevintraynor%2Fovs-dpdk- >>>>>> >> master%2Fcommit%2F88f46cc5ab338eb4f3ca5db1eacd0effefe4fa0c&d >>>>>> >> ata=02%7C01%7Cophirmu%40mellanox.com%7Cd203c89637bf4f27835508d >>>>>> >> 61309798e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C63671 >>>>>> >> 7328752336932&sdata=L7rWODNNEStIfibwXybu6Ub7Yvn%2BfLHk6VT >>>>>> we14jxyY%3D&reserved=0 >>>>>> >>>>>>>>>>>>>>>>>>>>>>> >>>>>>> Please find below for my comments. >>>>>>> >>>>>>> >>>>>>> Regards >>>>>>> _Sugesh >>>>>>> >>>>>>>> -----Original Message----- >>>>>>>> From: Ophir Munk [mailto:ophirmu@mellanox.com] >>>>>>>> Sent: Monday, September 3, 2018 3:19 PM >>>>>>>> To: ovs-dev@openvswitch.org >>>>>>>> Cc: Asaf Penso <asafp@mellanox.com>; Chandran, Sugesh >>>>>>>> <sugesh.chandran@intel.com>; Stokes, Ian >>>>>>>> <ian.stokes@intel.com>; >>>>>>>> Ben Pfaff <blp@ovn.org>; Shahaf Shuler <shahafs@mellanox.com>; >>>>>>>> Thomas Monjalon <thomas@monjalon.net>; Olga Shern >>>>>>>> <olgas@mellanox.com>; Ophir Munk <ophirmu@mellanox.com>; >> Kevin >>>>>>>> Traynor <ktraynor@redhat.com> >>>>>>>> Subject: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk >>>>>>>> v18.08.0 >>>>>>>> >>>>>>>> 1. Enable compilation and linkage with dpdk 18.08.0 The >>>>>>>> following >>>>>>>> dpdk commits which were introduced after dpdk 17.11.x require >>>>>>>> OVS >>>>>>>> updates to accommodate to the dpdk changes. >>>>>>>> - ce17eddefc20 ("ethdev: introduce Rx queue offloads API") >>>>>>>> - ab3ce1e0c193 ("ethdev: remove old offload API") >>>>>>>> - c06ddf9698e0 ("meter: add configuration profile") >>>>>>>> - e58638c32411 ("ethdev: fix TPID handling in flow API") >>>>>>>> - cd8c7c7ce241 ("ethdev: replace bus specific struct with >>>>>>>> generic >>>>>>>> dev") >>>>>>>> - ac8d22de2394 ("ethdev: flatten RSS configuration in flow >>>>>>>> API") >>>>>>>> >>>>>>>> 2. Update references to DPDK version in Documentation >>>>>>>> >>>>>>>> 3. Update DPDK version in travis' linux-build script >>>>>>>> >>>>>>>> Signed-off-by: Ophir Munk <ophirmu@mellanox.com> >>>>>>>> --- >>>>>>>> .travis/linux-build.sh | 2 +- >>>>>>>> Documentation/faq/releases.rst | 2 +- >>>>>>>> lib/netdev-dpdk.c | 135 >>>>>>>> +++++++++++++++++++++++++++++-------- >>>>>> ---- >>>>>>>> 3 files changed, 98 insertions(+), 41 deletions(-) >>>>>>>> >>>>>>>> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh >>>>>>>> index >>>>>>>> 4b9fc4a..c60ac71 >>>>>>>> 100755 >>>>>>>> --- a/.travis/linux-build.sh >>>>>>>> +++ b/.travis/linux-build.sh >>>>>>>> @@ -83,7 +83,7 @@ fi >>>>>>>> >>>>>>>> if [ "$DPDK" ]; then >>>>>>>> if [ -z "$DPDK_VER" ]; then >>>>>>>> - DPDK_VER="17.11.3" >>>>>>>> + DPDK_VER="18.08.0" >>>>>>>> fi >>>>>>>> install_dpdk $DPDK_VER >>>>>>>> if [ "$CC" = "clang" ]; then diff --git >>>>>>>> a/Documentation/faq/releases.rst >> b/Documentation/faq/releases.rst >>>>>>>> index 41d41e3..646ae09 100644 >>>>>>>> --- a/Documentation/faq/releases.rst >>>>>>>> +++ b/Documentation/faq/releases.rst >>>>>>>> @@ -168,7 +168,7 @@ Q: What DPDK version does each Open >> vSwitch >>>>>>>> release work with? >>>>>>>> 2.7.x 16.11.7 >>>>>>>> 2.8.x 17.05.2 >>>>>>>> 2.9.x 17.11.3 >>>>>>>> - 2.10.x 17.11.3 >>>>>>>> + 2.10.x 18.08.0 >>>>>>>> ============ ======= >>>>>>>> >>>>>>>> Q: Are all the DPDK releases that OVS versions work with >>>>>>>> maintained? >>>>>>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index >>>>>>>> f91aa27..80d2af9 >>>>>>>> 100644 >>>>>>>> --- a/lib/netdev-dpdk.c >>>>>>>> +++ b/lib/netdev-dpdk.c >>>>>>>> @@ -168,11 +168,7 @@ static const struct rte_eth_conf port_conf >> = >>>>>>>> { >>>>>>>> .rxmode = { >>>>>>>> .mq_mode = ETH_MQ_RX_RSS, >>>>>>>> .split_hdr_size = 0, >>>>>>>> - .header_split = 0, /* Header Split disabled */ >>>>>>>> - .hw_ip_checksum = 0, /* IP checksum offload disabled >>>>>>>> */ >>>>>>>> - .hw_vlan_filter = 0, /* VLAN filtering disabled */ >>>>>>>> - .jumbo_frame = 0, /* Jumbo Frame Support disabled >>>>>>>> */ >>>>>>>> - .hw_strip_crc = 0, >>>>>>>> + .offloads = 0, >>>>>>>> }, >>>>>>>> .rx_adv_conf = { >>>>>>>> .rss_conf = { >>>>>>>> @@ -364,6 +360,7 @@ struct dpdk_ring { struct ingress_policer >>>>>>>> { >>>>>>>> struct rte_meter_srtcm_params app_srtcm_params; >>>>>>>> struct rte_meter_srtcm in_policer; >>>>>>>> + struct rte_meter_srtcm_profile in_prof; >>>>>>>> rte_spinlock_t policer_lock; }; >>>>>>>> >>>>>>>> @@ -903,16 +900,17 @@ dpdk_eth_dev_port_config(struct >> netdev_dpdk >>>>>>>> *dev, int n_rxq, int n_txq) >>>>>>>> if (dev->mtu > ETHER_MTU) { >>>>>>>> rte_eth_dev_info_get(dev->port_id, &info); >>>>>>>> if (strncmp(info.driver_name, "net_nfp", 7)) { >>>>>>>> - conf.rxmode.enable_scatter = 1; >>>>>>>> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_SCATTER; >>>>>>>> } >>>>>>>> } >>>>>>>> >>>>>>>> conf.intr_conf.lsc = dev->lsc_interrupt_mode; >>>>>>>> - conf.rxmode.hw_ip_checksum = (dev->hw_ol_features & >>>>>>>> - NETDEV_RX_CHECKSUM_OFFLOAD) >>>>>>>> != >>>>>>>> 0; >>>>>>>> + conf.rxmode.offloads |= ((dev->hw_ol_features & >>>>>>>> + NETDEV_RX_CHECKSUM_OFFLOAD) != 0) >>>>>>>> ? >>>>>>>> + DEV_RX_OFFLOAD_CHECKSUM : 0; >>>>>>>> >>>>>>>> if (dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP) { >>>>>>>> - conf.rxmode.hw_strip_crc = 1; >>>>>>>> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP; >>>>>>>> } >>>>>>>> >>>>>>>> /* A device may report more queues than it makes available >>>>>>>> (this has @@ - >>>>>>>> 1932,16 +1930,18 @@ netdev_dpdk_eth_tx_burst(struct >> netdev_dpdk >>>>>> *dev, >>>>>>>> int qid, >>>>>>>> >>>>>>>> static inline bool >>>>>>>> netdev_dpdk_policer_pkt_handle(struct rte_meter_srtcm *meter, >>>>>>>> + struct rte_meter_srtcm_profile >>>>>>>> + *profile, >>>>>>>> struct rte_mbuf *pkt, uint64_t >>>>>>>> time) { >>>>>>>> uint32_t pkt_len = rte_pktmbuf_pkt_len(pkt) - >>>>>>>> sizeof(struct >>>>>>>> ether_hdr); >>>>>>>> >>>>>>>> - return rte_meter_srtcm_color_blind_check(meter, time, >>>>>>>> pkt_len) == >>>>>>>> + return rte_meter_srtcm_color_blind_check(meter, profile, >>>>>>>> time, >>>>>>>> + pkt_len) == >>>>>>>> >>>>>>>> e_RTE_METER_GREEN; } >>>>>>>> >>>>>>>> static int >>>>>>>> netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, >>>>>>>> + struct rte_meter_srtcm_profile >>>>>>>> *profile, >>>>>>>> struct rte_mbuf **pkts, int pkt_cnt, >>>>>>>> bool should_steal) { @@ -1953,7 >>>>>>>> +1953,8 >>>>>>>> @@ netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, >>>>>>>> for (i = 0; i < pkt_cnt; i++) { >>>>>>>> pkt = pkts[i]; >>>>>>>> /* Handle current packet */ >>>>>>>> - if (netdev_dpdk_policer_pkt_handle(meter, pkt, >>>>>>>> current_time)) { >>>>>>>> + if (netdev_dpdk_policer_pkt_handle(meter, profile, >>>>>>>> pkt, >>>>>>>> + >>>>>>>> current_time)) >>>>>>>> { >>>>>>>> if (cnt != i) { >>>>>>>> pkts[cnt] = pkt; >>>>>>>> } >>>>>>>> @@ -1975,8 +1976,8 @@ ingress_policer_run(struct >>>>>>>> ingress_policer >>>>>>>> *policer, struct rte_mbuf **pkts, >>>>>>>> int cnt = 0; >>>>>>>> >>>>>>>> rte_spinlock_lock(&policer->policer_lock); >>>>>>>> - cnt = netdev_dpdk_policer_run(&policer->in_policer, pkts, >>>>>>>> - pkt_cnt, should_steal); >>>>>>>> + cnt = netdev_dpdk_policer_run(&policer->in_policer, >>>>>>>> &policer- >>>>>>> in_prof, >>>>>>>> + pkts, pkt_cnt, >>>>>>>> should_steal); >>>>>>>> rte_spinlock_unlock(&policer->policer_lock); >>>>>>>> >>>>>>>> return cnt; >>>>>>>> @@ -2767,8 +2768,15 @@ netdev_dpdk_policer_construct(uint32_t >>>>>>>> rate, >>>>>>>> uint32_t burst) >>>>>>>> policer->app_srtcm_params.cir = rate_bytes; >>>>>>>> policer->app_srtcm_params.cbs = burst_bytes; >>>>>>>> policer->app_srtcm_params.ebs = 0; >>>>>>>> + err = rte_meter_srtcm_profile_config(&policer->in_prof, >>>>>>>> + >>>>>>>> &policer->app_srtcm_params); >>>>>>>> + if (err) { >>>>>>>> + VLOG_ERR("Could not create rte meter profile for >>>>>>>> ingress >>>>>>>> policer"); >>>>>>>> + free(policer); >>>>>>>> + return NULL; >>>>>>>> + } >>>>>>>> err = rte_meter_srtcm_config(&policer->in_policer, >>>>>>>> - >>>>>>>> &policer->app_srtcm_params); >>>>>>>> + &policer->in_prof); >>>>>>>> if (err) { >>>>>>>> VLOG_ERR("Could not create rte meter for ingress >>>>>>>> policer"); >>>>>>>> free(policer); >>>>>>>> @@ -3043,13 +3051,18 @@ netdev_dpdk_get_status(const struct >>>>>>>> netdev *netdev, struct smap *args) >>>>>>>> smap_add_format(args, "if_descr", "%s %s", rte_version(), >>>>>>>> >>>>>>>> dev_info.driver_name); >>>>>>>> >>>>>>>> - if (dev_info.pci_dev) { >>>>>>>> - smap_add_format(args, "pci-vendor_id", "0x%x", >>>>>>>> - dev_info.pci_dev->id.vendor_id); >>>>>>>> - smap_add_format(args, "pci-device_id", "0x%x", >>>>>>>> - dev_info.pci_dev->id.device_id); >>>>>>>> + const struct rte_bus *bus; >>>>>>>> + const struct rte_pci_device *pci_dev; >>>>>>>> + bus = rte_bus_find_by_device(dev_info.device); >>>>>>>> + if (bus && !strcmp(bus->name, "pci")) { >>>>>>>> + pci_dev = RTE_DEV_TO_PCI(dev_info.device); >>>>>>>> + if (pci_dev) { >>>>>>>> + smap_add_format(args, "pci-vendor_id", "0x%u", >>>>>>>> + pci_dev->id.vendor_id); >>>>>>>> + smap_add_format(args, "pci-device_id", "0x%x", >>>>>>>> + pci_dev->id.device_id); >>>>>>>> + } >>>>>>>> } >>>>>>>> - >>>>>>>> return 0; >>>>>>>> } >>>>>>>> >>>>>>>> @@ -3727,6 +3740,7 @@ struct egress_policer { >>>>>>>> struct qos_conf qos_conf; >>>>>>>> struct rte_meter_srtcm_params app_srtcm_params; >>>>>>>> struct rte_meter_srtcm egress_meter; >>>>>>>> + struct rte_meter_srtcm_profile egress_prof; >>>>>>>> }; >>>>>>>> >>>>>>>> static void >>>>>>>> @@ -3749,8 +3763,16 @@ egress_policer_qos_construct(const >> struct >>>>>> smap >>>>>>>> *details, >>>>>>>> policer = xmalloc(sizeof *policer); >>>>>>>> qos_conf_init(&policer->qos_conf, &egress_policer_ops); >>>>>>>> egress_policer_details_to_param(details, >>>>>>>> &policer->app_srtcm_params); >>>>>>>> + err = >>>>>>>> rte_meter_srtcm_profile_config(&policer->egress_prof, >>>>>>>> + >>>>>>>> &policer->app_srtcm_params); >>>>>>>> + if (err) { >>>>>>> [Sugesh] Should we error out the message on failing the profile >>>>>>> config?? >>>>>>>> + free(policer); >>>>>>>> + *conf = NULL; >>>>>>>> + return -err; >>>>>>>> + } >>>>>>>> err = rte_meter_srtcm_config(&policer->egress_meter, >>>>>>>> - &policer->app_srtcm_params); >>>>>>>> + &policer->egress_prof); >>>>>>>> + >>>>>>>> if (!err) { >>>>>>>> *conf = &policer->qos_conf; >>>>>>>> } else { >>>>>>>> @@ -3803,7 +3825,8 @@ egress_policer_run(struct qos_conf *conf, >>>>>>>> struct rte_mbuf **pkts, int pkt_cnt, >>>>>>>> struct egress_policer *policer = >>>>>>>> CONTAINER_OF(conf, struct egress_policer, qos_conf); >>>>>>>> >>>>>>>> - cnt = netdev_dpdk_policer_run(&policer->egress_meter, >>>>>>>> pkts, >>>>>>>> + cnt = netdev_dpdk_policer_run(&policer->egress_meter, >>>>>>>> + &policer->egress_prof, pkts, >>>>>>>> pkt_cnt, should_steal); >>>>>>>> >>>>>>>> return cnt; >>>>>>>> @@ -4103,15 +4126,15 @@ dump_flow_pattern(struct >> rte_flow_item >>>>>> *item) >>>>>>>> >>>>>>>> VLOG_DBG("rte flow vlan pattern:\n"); >>>>>>>> if (vlan_spec) { >>>>>>>> - VLOG_DBG(" Spec: tpid=0x%"PRIx16", >>>>>>>> tci=0x%"PRIx16"\n", >>>>>>>> - ntohs(vlan_spec->tpid), >>>>>>>> ntohs(vlan_spec->tci)); >>>>>>>> + VLOG_DBG(" Spec: inner_type=0x%"PRIx16", >>>>>>>> tci=0x%"PRIx16"\n", >>>>>>>> + ntohs(vlan_spec->inner_type), >>>>>>>> + ntohs(vlan_spec->tci)); >>>>>>>> } else { >>>>>>>> VLOG_DBG(" Spec = null\n"); >>>>>>>> } >>>>>>>> >>>>>>>> if (vlan_mask) { >>>>>>>> - VLOG_DBG(" Mask: tpid=0x%"PRIx16", >>>>>>>> tci=0x%"PRIx16"\n", >>>>>>>> - vlan_mask->tpid, vlan_mask->tci); >>>>>>>> + VLOG_DBG(" Mask: inner_type=0x%"PRIx16", >>>>>> tci=0x%"PRIx16"\n", >>>>>>>> + vlan_mask->inner_type, vlan_mask->tci); >>>>>>>> } else { >>>>>>>> VLOG_DBG(" Mask = null\n"); >>>>>>>> } >>>>>>>> @@ -4281,27 +4304,56 @@ add_flow_action(struct flow_actions >>>>>> *actions, >>>>>>>> enum rte_flow_action_type type, >>>>>>>> actions->cnt++; >>>>>>>> } >>>>>>>> >>>>>>>> +/* >>>>>>>> + * Storage for struct rte_flow_action_rss >>>>>>>> + * including storage for key and queue array */ #define >>>>>>>> +MAX_RSS_HASH_KEY_LENGTH 128 #define >>>>>> MAX_ACTION_RSS_QUEUE_NUM >>>>>>>> 128 >>>>>>>> +struct action_rss_data { >>>>>>>> + struct rte_flow_action_rss conf; >>>>>>>> + uint16_t queue[MAX_ACTION_RSS_QUEUE_NUM]; >>>>>>>> + uint8_t key[MAX_RSS_HASH_KEY_LENGTH]; }; >>>>>>>> + >>>>>>>> static struct rte_flow_action_rss * >>>>>>>> add_flow_rss_action(struct >>>>>>>> flow_actions *actions, >>>>>>>> struct netdev *netdev) { >>>>>>>> int i; >>>>>>>> - struct rte_flow_action_rss *rss; >>>>>>>> + struct action_rss_data *rss_data; >>>>>>>> >>>>>>>> - rss = xmalloc(sizeof(*rss) + sizeof(uint16_t) * >>>>>>>> netdev->n_rxq); >>>>>>>> - /* >>>>>>>> - * Setting it to NULL will let the driver use the default >>>>>>>> RSS >>>>>>>> - * configuration we have set: >>>>>>>> &port_conf.rx_adv_conf.rss_conf. >>>>>>>> - */ >>>>>>>> - rss->rss_conf = NULL; >>>>>>>> - rss->num = netdev->n_rxq; >>>>>>>> + if (netdev->n_rxq > MAX_ACTION_RSS_QUEUE_NUM) { >>>>>>>> + VLOG_ERR("Num of rxq (%u) must not be greater " \ >>>>>>>> + "than max rss num of queues (%u)", >>>>>>>> + netdev->n_rxq, MAX_ACTION_RSS_QUEUE_NUM); >>>>>>>> + return NULL; >>>>>>>> + } >>>>>>>> >>>>>>>> - for (i = 0; i < rss->num; i++) { >>>>>>>> - rss->queue[i] = i; >>>>>>>> + rss_data = xmalloc(sizeof(struct action_rss_data)); >>>>>>>> + *rss_data = (struct action_rss_data){ >>>>>>>> + .conf = (struct rte_flow_action_rss){ >>>>>>>> + .func = RTE_ETH_HASH_FUNCTION_DEFAULT, >>>>>>>> + .level = 0, >>>>>>>> + .types = ETH_RSS_IP, >>>>>>>> + .key_len = 0, >>>>>>>> + .queue_num = netdev->n_rxq, >>>>>>>> + .queue = rss_data->queue, >>>>>>>> + .key = rss_data->key, >>>>>>>> + }, >>>>>>>> + .key = { 0 }, >>>>>>>> + .queue = { 0 }, >>>>>>>> + }; >>>>>>>> + >>>>>>>> + /* TODO: Override key with default */ >>>>>>> [Sugesh] Do we really need this TODO?? >>>>>>>> + >>>>>>>> + /* Override queue array with default */ >>>>>>>> + for (i = 0; i < rss_data->conf.queue_num; i++) { >>>>>>>> + rss_data->queue[i] = i; >>>>>>>> } >>>>>>>> >>>>>>>> - add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, rss); >>>>>>>> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, >>>>>>>> + &rss_data->conf); >>>>>>>> >>>>>>>> - return rss; >>>>>>>> + return &rss_data->conf; >>>>>>>> } >>>>>>>> >>>>>>>> static int >>>>>>>> @@ -4365,7 +4417,7 @@ >> netdev_dpdk_add_rte_flow_offload(struct >>>>>> netdev >>>>>>>> *netdev, >>>>>>>> vlan_mask.tci = match->wc.masks.vlans[0].tci & >>>>>>>> ~htons(VLAN_CFI); >>>>>>>> >>>>>>>> /* match any protocols */ >>>>>>>> - vlan_mask.tpid = 0; >>>>>>>> + vlan_mask.inner_type = 0; >>>>>>>> >>>>>>>> add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_VLAN, >>>>>>>> &vlan_spec, &vlan_mask); @@ -4516,6 >>>>>>>> +4568,11 @@ >>>>>>>> end_proto_check: >>>>>>>> >>>>>>>> struct rte_flow_action_rss *rss; >>>>>>>> rss = add_flow_rss_action(&actions, netdev); >>>>>>>> + if (!rss) { >>>>>>>> + VLOG_ERR("add_flow_rss_action error.\n"); >>>>>>> I feel this error can be bit more verbose something like Failed >>>>>>> to >>>>>>> add >>>>>> rss_flow+actions or something?? What do you think? >>>>>>>> + ret = -1; >>>>>>>> + goto out; >>>>>>>> + } >>>>>>>> add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, >> NULL); >>>>>>>> >>>>>>>> flow = rte_flow_create(dev->port_id, &flow_attr, >>>>>>>> patterns.items, >>>>>>>> -- >>>>>>>> 1.8.3.1 >>>>>>> >>>>> >>>>> _______________________________________________ >>>>> dev mailing list >>>>> dev@openvswitch.org >>>>> >> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fm >>>>> ail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs- >> dev&data=02%7C01% >>>>> >> 7Cophirmu%40mellanox.com%7Ce6c08e7bb58d40807f1408d6133416c2%7C >> a65297 >>>>> >> 1c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636717511775064323&s >> data=qU >>>>> >> zRNGAXN%2F9EqDJplTPvs82QP8tRte6e3GW5i01IEYE%3D&reserved=0 >>>> _______________________________________________ >>>> dev mailing list >>>> dev@openvswitch.org >>>> >> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fma >>>> il.openvswitch.org%2Fmailman%2Flistinfo%2Fovs- >> dev&data=02%7C01%7C >>>> >> ophirmu%40mellanox.com%7Ce6c08e7bb58d40807f1408d6133416c2%7Ca6 >> 52971c7 >>>> >> d2e4d9ba6a4d149256f461b%7C0%7C0%7C636717511775064323&sdat >> a=qUzRNG >>>> AXN%2F9EqDJplTPvs82QP8tRte6e3GW5i01IEYE%3D&reserved=0 >>> _______________________________________________ >>> dev mailing list >>> dev@openvswitch.org >>> >> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fmai >>> l.openvswitch.org%2Fmailman%2Flistinfo%2Fovs- >> dev&data=02%7C01%7Cop >>> >> hirmu%40mellanox.com%7Ce6c08e7bb58d40807f1408d6133416c2%7Ca652 >> 971c7d2e >>> >> 4d9ba6a4d149256f461b%7C0%7C0%7C636717511775064323&sdata=q >> UzRNGAXN% >>> 2F9EqDJplTPvs82QP8tRte6e3GW5i01IEYE%3D&reserved=0
Hi Ophir, Could you please send out v2 of this patch with the changes from Kevin? I will test it in our lab to see if its working Regards _Sugesh > -----Original Message----- > From: Eelco Chaudron [mailto:echaudro@redhat.com] > Sent: Wednesday, September 5, 2018 1:35 PM > To: Ophir Munk <ophirmu@mellanox.com> > Cc: Kevin Traynor <ktraynor@redhat.com>; Chandran, Sugesh > <sugesh.chandran@intel.com>; ovs-dev@openvswitch.org; Shahaf Shuler > <shahafs@mellanox.com>; Asaf Penso <asafp@mellanox.com>; Thomas > Monjalon <thomas@monjalon.net> > Subject: Re: [ovs-dev] [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk > v18.08.0 > > Hi Ophir, > > I get the same error when trying this with an XL710, wanted to see if the patch > would work for non-root users. > Did not further troubleshoot it. > > Also I get the following compile warnings(errors): > > mv -f $depbase.Tpo $depbase.Po > lib/netdev-dpdk.c: In function 'netdev_dpdk_destruct': > lib/netdev-dpdk.c:1331:9: error: 'rte_eth_dev_detach' is deprecated (declared at > /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native-linuxapp- > gcc/include/rte_ethdev.h:1451) > [-Werror=deprecated-declarations] > if (rte_eth_dev_detach(dev->port_id, devname) < 0) { > ^ > lib/netdev-dpdk.c: In function 'netdev_dpdk_process_devargs': > lib/netdev-dpdk.c:1624:13: error: 'rte_eth_dev_attach' is deprecated (declared > at > /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native-linuxapp- > gcc/include/rte_ethdev.h:1435) > [-Werror=deprecated-declarations] > if (!rte_eth_dev_attach(devargs, &new_port_id)) { > ^ > lib/netdev-dpdk.c: In function 'netdev_dpdk_detach': > lib/netdev-dpdk.c:3154:5: error: 'rte_eth_dev_detach' is deprecated (declared at > /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native-linuxapp- > gcc/include/rte_ethdev.h:1451) > [-Werror=deprecated-declarations] > ret = rte_eth_dev_detach(port_id, devname); > ^ > lib/netdev-dpdk.c: At top level: > > > //Eelco > > > On 5 Sep 2018, at 12:44, Ophir Munk wrote: > > > Hi, > > I suggest the following trouble-shooting steps: > > 1. Try Kevin's fix for Niantic (attached here as a non-official patch > > on top of latest dpdk-hwol branch). > > Sugesh - can you please try it? > > 2. Sugesh - do you have the same issue with other NICs than Niantic on > > latest dpdk-hwol branch? (other NICs used to work a month > > ago) > > 3. I will try on my setup. > > > > Regards, > > Ophir > > > >> -----Original Message----- > >> From: Kevin Traynor [mailto:ktraynor@redhat.com] > >> Sent: Wednesday, September 05, 2018 9:28 AM > >> To: Chandran, Sugesh <sugesh.chandran@intel.com>; Ophir Munk > >> <ophirmu@mellanox.com>; ovs-dev@openvswitch.org > >> Cc: Asaf Penso <asafp@mellanox.com>; Stokes, Ian > >> <ian.stokes@intel.com>; Ben Pfaff <blp@ovn.org>; Shahaf Shuler > >> <shahafs@mellanox.com>; Thomas Monjalon <thomas@monjalon.net>; > Olga > >> Shern <olgas@mellanox.com> > >> Subject: Re: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk > >> v18.08.0 > >> > >> On 09/05/2018 07:44 AM, Chandran, Sugesh wrote: > >>> Hi Ophir, > >>> > >>> Thank you for working on this. > >>> I tried to run some tests and getting some errors while adding > >>> physical ports as below. I use Niantic NIC for this test > >>> > >>>>>>>>>>>>>>>>>> > >>> 2018-09-05T06:34:58Z|00069|dpdk|INFO|Device with port_id=0 already > >>> stopped 2018-09-05T06:34:58Z|00070|dpdk|ERR|Ethdev port_id=0 invalid > >>> rss_hf: 0x3afbc, valid value: 0x38d34 > >>> 2018-09-05T06:34:58Z|00071|netdev_dpdk|WARN|Interface dpdk0 > >> eth_dev > >>> setup error Invalid argument > >>> 2018-09-05T06:34:58Z|00072|netdev_dpdk|ERR|Interface dpdk0(rxq:2 > >> txq:2 > >>> lsc interrupt mode:false) configure error: Invalid argument > >>> 2018-09-05T06:34:58Z|00073|dpif_netdev|ERR|Failed to set interface > >>> dpdk0 new configuration 2018-09- > >> 05T06:34:58Z|00074|bridge|WARN|could > >>> not add network device dpdk0 to ofproto (No such device) > >>> ovs-vsctl: Error detected while setting up 'dpdk0': could not add > >>> network > >> device dpdk0 to ofproto (No such device). See ovs-vswitchd log for > >> details. > >>> ovs-vsctl: The default log directory is > >>> "/usr/local/var/log/openvswitch". > >>> > >> > >> I didn't have time to compare the below patch with the ones that I > >> did, but I saw that this patch was needed for niantic > >> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi > >> t > >> hub.com%2Fkevintraynor%2Fovs-dpdk- > >> master%2Fcommit%2F88f46cc5ab338eb4f3ca5db1eacd0effefe4fa0c&d > >> ata=02%7C01%7Cophirmu%40mellanox.com%7Cd203c89637bf4f27835508d > >> 61309798e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C63671 > >> 7328752336932&sdata=L7rWODNNEStIfibwXybu6Ub7Yvn%2BfLHk6VT > >> we14jxyY%3D&reserved=0 > >> > >>>>>>>>>>>>>>>>>>> > >>> Please find below for my comments. > >>> > >>> > >>> Regards > >>> _Sugesh > >>> > >>>> -----Original Message----- > >>>> From: Ophir Munk [mailto:ophirmu@mellanox.com] > >>>> Sent: Monday, September 3, 2018 3:19 PM > >>>> To: ovs-dev@openvswitch.org > >>>> Cc: Asaf Penso <asafp@mellanox.com>; Chandran, Sugesh > >>>> <sugesh.chandran@intel.com>; Stokes, Ian <ian.stokes@intel.com>; > >>>> Ben Pfaff <blp@ovn.org>; Shahaf Shuler <shahafs@mellanox.com>; > >>>> Thomas Monjalon <thomas@monjalon.net>; Olga Shern > >>>> <olgas@mellanox.com>; Ophir Munk <ophirmu@mellanox.com>; Kevin > >>>> Traynor <ktraynor@redhat.com> > >>>> Subject: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk v18.08.0 > >>>> > >>>> 1. Enable compilation and linkage with dpdk 18.08.0 The following > >>>> dpdk commits which were introduced after dpdk 17.11.x require OVS > >>>> updates to accommodate to the dpdk changes. > >>>> - ce17eddefc20 ("ethdev: introduce Rx queue offloads API") > >>>> - ab3ce1e0c193 ("ethdev: remove old offload API") > >>>> - c06ddf9698e0 ("meter: add configuration profile") > >>>> - e58638c32411 ("ethdev: fix TPID handling in flow API") > >>>> - cd8c7c7ce241 ("ethdev: replace bus specific struct with generic > >>>> dev") > >>>> - ac8d22de2394 ("ethdev: flatten RSS configuration in flow API") > >>>> > >>>> 2. Update references to DPDK version in Documentation > >>>> > >>>> 3. Update DPDK version in travis' linux-build script > >>>> > >>>> Signed-off-by: Ophir Munk <ophirmu@mellanox.com> > >>>> --- > >>>> .travis/linux-build.sh | 2 +- > >>>> Documentation/faq/releases.rst | 2 +- > >>>> lib/netdev-dpdk.c | 135 > >>>> +++++++++++++++++++++++++++++-------- > >> ---- > >>>> 3 files changed, 98 insertions(+), 41 deletions(-) > >>>> > >>>> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh index > >>>> 4b9fc4a..c60ac71 > >>>> 100755 > >>>> --- a/.travis/linux-build.sh > >>>> +++ b/.travis/linux-build.sh > >>>> @@ -83,7 +83,7 @@ fi > >>>> > >>>> if [ "$DPDK" ]; then > >>>> if [ -z "$DPDK_VER" ]; then > >>>> - DPDK_VER="17.11.3" > >>>> + DPDK_VER="18.08.0" > >>>> fi > >>>> install_dpdk $DPDK_VER > >>>> if [ "$CC" = "clang" ]; then > >>>> diff --git a/Documentation/faq/releases.rst > >>>> b/Documentation/faq/releases.rst index 41d41e3..646ae09 100644 > >>>> --- a/Documentation/faq/releases.rst > >>>> +++ b/Documentation/faq/releases.rst > >>>> @@ -168,7 +168,7 @@ Q: What DPDK version does each Open vSwitch > >>>> release work with? > >>>> 2.7.x 16.11.7 > >>>> 2.8.x 17.05.2 > >>>> 2.9.x 17.11.3 > >>>> - 2.10.x 17.11.3 > >>>> + 2.10.x 18.08.0 > >>>> ============ ======= > >>>> > >>>> Q: Are all the DPDK releases that OVS versions work with > >>>> maintained? > >>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > >>>> f91aa27..80d2af9 > >>>> 100644 > >>>> --- a/lib/netdev-dpdk.c > >>>> +++ b/lib/netdev-dpdk.c > >>>> @@ -168,11 +168,7 @@ static const struct rte_eth_conf port_conf = { > >>>> .rxmode = { > >>>> .mq_mode = ETH_MQ_RX_RSS, > >>>> .split_hdr_size = 0, > >>>> - .header_split = 0, /* Header Split disabled */ > >>>> - .hw_ip_checksum = 0, /* IP checksum offload disabled */ > >>>> - .hw_vlan_filter = 0, /* VLAN filtering disabled */ > >>>> - .jumbo_frame = 0, /* Jumbo Frame Support disabled */ > >>>> - .hw_strip_crc = 0, > >>>> + .offloads = 0, > >>>> }, > >>>> .rx_adv_conf = { > >>>> .rss_conf = { > >>>> @@ -364,6 +360,7 @@ struct dpdk_ring { struct ingress_policer { > >>>> struct rte_meter_srtcm_params app_srtcm_params; > >>>> struct rte_meter_srtcm in_policer; > >>>> + struct rte_meter_srtcm_profile in_prof; > >>>> rte_spinlock_t policer_lock; > >>>> }; > >>>> > >>>> @@ -903,16 +900,17 @@ dpdk_eth_dev_port_config(struct netdev_dpdk > >>>> *dev, int n_rxq, int n_txq) > >>>> if (dev->mtu > ETHER_MTU) { > >>>> rte_eth_dev_info_get(dev->port_id, &info); > >>>> if (strncmp(info.driver_name, "net_nfp", 7)) { > >>>> - conf.rxmode.enable_scatter = 1; > >>>> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_SCATTER; > >>>> } > >>>> } > >>>> > >>>> conf.intr_conf.lsc = dev->lsc_interrupt_mode; > >>>> - conf.rxmode.hw_ip_checksum = (dev->hw_ol_features & > >>>> - NETDEV_RX_CHECKSUM_OFFLOAD) != > >>>> 0; > >>>> + conf.rxmode.offloads |= ((dev->hw_ol_features & > >>>> + NETDEV_RX_CHECKSUM_OFFLOAD) != 0) ? > >>>> + DEV_RX_OFFLOAD_CHECKSUM : 0; > >>>> > >>>> if (dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP) { > >>>> - conf.rxmode.hw_strip_crc = 1; > >>>> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP; > >>>> } > >>>> > >>>> /* A device may report more queues than it makes available > >>>> (this has @@ - > >>>> 1932,16 +1930,18 @@ netdev_dpdk_eth_tx_burst(struct netdev_dpdk > >> *dev, > >>>> int qid, > >>>> > >>>> static inline bool > >>>> netdev_dpdk_policer_pkt_handle(struct rte_meter_srtcm *meter, > >>>> + struct rte_meter_srtcm_profile > >>>> + *profile, > >>>> struct rte_mbuf *pkt, uint64_t > >>>> time) { > >>>> uint32_t pkt_len = rte_pktmbuf_pkt_len(pkt) - sizeof(struct > >>>> ether_hdr); > >>>> > >>>> - return rte_meter_srtcm_color_blind_check(meter, time, pkt_len) > >>>> == > >>>> + return rte_meter_srtcm_color_blind_check(meter, profile, time, > >>>> + pkt_len) == > >>>> e_RTE_METER_GREEN; > >>>> } > >>>> > >>>> static int > >>>> netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, > >>>> + struct rte_meter_srtcm_profile *profile, > >>>> struct rte_mbuf **pkts, int pkt_cnt, > >>>> bool should_steal) { @@ -1953,7 +1953,8 > >>>> @@ netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, > >>>> for (i = 0; i < pkt_cnt; i++) { > >>>> pkt = pkts[i]; > >>>> /* Handle current packet */ > >>>> - if (netdev_dpdk_policer_pkt_handle(meter, pkt, > >>>> current_time)) { > >>>> + if (netdev_dpdk_policer_pkt_handle(meter, profile, pkt, > >>>> + current_time)) { > >>>> if (cnt != i) { > >>>> pkts[cnt] = pkt; > >>>> } > >>>> @@ -1975,8 +1976,8 @@ ingress_policer_run(struct ingress_policer > >>>> *policer, struct rte_mbuf **pkts, > >>>> int cnt = 0; > >>>> > >>>> rte_spinlock_lock(&policer->policer_lock); > >>>> - cnt = netdev_dpdk_policer_run(&policer->in_policer, pkts, > >>>> - pkt_cnt, should_steal); > >>>> + cnt = netdev_dpdk_policer_run(&policer->in_policer, &policer- > >>> in_prof, > >>>> + pkts, pkt_cnt, should_steal); > >>>> rte_spinlock_unlock(&policer->policer_lock); > >>>> > >>>> return cnt; > >>>> @@ -2767,8 +2768,15 @@ netdev_dpdk_policer_construct(uint32_t rate, > >>>> uint32_t burst) > >>>> policer->app_srtcm_params.cir = rate_bytes; > >>>> policer->app_srtcm_params.cbs = burst_bytes; > >>>> policer->app_srtcm_params.ebs = 0; > >>>> + err = rte_meter_srtcm_profile_config(&policer->in_prof, > >>>> + > >>>> &policer->app_srtcm_params); > >>>> + if (err) { > >>>> + VLOG_ERR("Could not create rte meter profile for ingress > >>>> policer"); > >>>> + free(policer); > >>>> + return NULL; > >>>> + } > >>>> err = rte_meter_srtcm_config(&policer->in_policer, > >>>> - &policer->app_srtcm_params); > >>>> + &policer->in_prof); > >>>> if (err) { > >>>> VLOG_ERR("Could not create rte meter for ingress > >>>> policer"); > >>>> free(policer); > >>>> @@ -3043,13 +3051,18 @@ netdev_dpdk_get_status(const struct netdev > >>>> *netdev, struct smap *args) > >>>> smap_add_format(args, "if_descr", "%s %s", rte_version(), > >>>> > >>>> dev_info.driver_name); > >>>> > >>>> - if (dev_info.pci_dev) { > >>>> - smap_add_format(args, "pci-vendor_id", "0x%x", > >>>> - dev_info.pci_dev->id.vendor_id); > >>>> - smap_add_format(args, "pci-device_id", "0x%x", > >>>> - dev_info.pci_dev->id.device_id); > >>>> + const struct rte_bus *bus; > >>>> + const struct rte_pci_device *pci_dev; > >>>> + bus = rte_bus_find_by_device(dev_info.device); > >>>> + if (bus && !strcmp(bus->name, "pci")) { > >>>> + pci_dev = RTE_DEV_TO_PCI(dev_info.device); > >>>> + if (pci_dev) { > >>>> + smap_add_format(args, "pci-vendor_id", "0x%u", > >>>> + pci_dev->id.vendor_id); > >>>> + smap_add_format(args, "pci-device_id", "0x%x", > >>>> + pci_dev->id.device_id); > >>>> + } > >>>> } > >>>> - > >>>> return 0; > >>>> } > >>>> > >>>> @@ -3727,6 +3740,7 @@ struct egress_policer { > >>>> struct qos_conf qos_conf; > >>>> struct rte_meter_srtcm_params app_srtcm_params; > >>>> struct rte_meter_srtcm egress_meter; > >>>> + struct rte_meter_srtcm_profile egress_prof; > >>>> }; > >>>> > >>>> static void > >>>> @@ -3749,8 +3763,16 @@ egress_policer_qos_construct(const struct > >> smap > >>>> *details, > >>>> policer = xmalloc(sizeof *policer); > >>>> qos_conf_init(&policer->qos_conf, &egress_policer_ops); > >>>> egress_policer_details_to_param(details, > >>>> &policer->app_srtcm_params); > >>>> + err = rte_meter_srtcm_profile_config(&policer->egress_prof, > >>>> + > >>>> &policer->app_srtcm_params); > >>>> + if (err) { > >>> [Sugesh] Should we error out the message on failing the profile > >>> config?? > >>>> + free(policer); > >>>> + *conf = NULL; > >>>> + return -err; > >>>> + } > >>>> err = rte_meter_srtcm_config(&policer->egress_meter, > >>>> - &policer->app_srtcm_params); > >>>> + &policer->egress_prof); > >>>> + > >>>> if (!err) { > >>>> *conf = &policer->qos_conf; > >>>> } else { > >>>> @@ -3803,7 +3825,8 @@ egress_policer_run(struct qos_conf *conf, > >>>> struct rte_mbuf **pkts, int pkt_cnt, > >>>> struct egress_policer *policer = > >>>> CONTAINER_OF(conf, struct egress_policer, qos_conf); > >>>> > >>>> - cnt = netdev_dpdk_policer_run(&policer->egress_meter, pkts, > >>>> + cnt = netdev_dpdk_policer_run(&policer->egress_meter, > >>>> + &policer->egress_prof, pkts, > >>>> pkt_cnt, should_steal); > >>>> > >>>> return cnt; > >>>> @@ -4103,15 +4126,15 @@ dump_flow_pattern(struct rte_flow_item > >> *item) > >>>> > >>>> VLOG_DBG("rte flow vlan pattern:\n"); > >>>> if (vlan_spec) { > >>>> - VLOG_DBG(" Spec: tpid=0x%"PRIx16", > >>>> tci=0x%"PRIx16"\n", > >>>> - ntohs(vlan_spec->tpid), > >>>> ntohs(vlan_spec->tci)); > >>>> + VLOG_DBG(" Spec: inner_type=0x%"PRIx16", > >>>> tci=0x%"PRIx16"\n", > >>>> + ntohs(vlan_spec->inner_type), > >>>> + ntohs(vlan_spec->tci)); > >>>> } else { > >>>> VLOG_DBG(" Spec = null\n"); > >>>> } > >>>> > >>>> if (vlan_mask) { > >>>> - VLOG_DBG(" Mask: tpid=0x%"PRIx16", > >>>> tci=0x%"PRIx16"\n", > >>>> - vlan_mask->tpid, vlan_mask->tci); > >>>> + VLOG_DBG(" Mask: inner_type=0x%"PRIx16", > >> tci=0x%"PRIx16"\n", > >>>> + vlan_mask->inner_type, vlan_mask->tci); > >>>> } else { > >>>> VLOG_DBG(" Mask = null\n"); > >>>> } > >>>> @@ -4281,27 +4304,56 @@ add_flow_action(struct flow_actions > >> *actions, > >>>> enum rte_flow_action_type type, > >>>> actions->cnt++; > >>>> } > >>>> > >>>> +/* > >>>> + * Storage for struct rte_flow_action_rss > >>>> + * including storage for key and queue array */ #define > >>>> +MAX_RSS_HASH_KEY_LENGTH 128 #define > >> MAX_ACTION_RSS_QUEUE_NUM > >>>> 128 > >>>> +struct action_rss_data { > >>>> + struct rte_flow_action_rss conf; > >>>> + uint16_t queue[MAX_ACTION_RSS_QUEUE_NUM]; > >>>> + uint8_t key[MAX_RSS_HASH_KEY_LENGTH]; }; > >>>> + > >>>> static struct rte_flow_action_rss * add_flow_rss_action(struct > >>>> flow_actions *actions, > >>>> struct netdev *netdev) { > >>>> int i; > >>>> - struct rte_flow_action_rss *rss; > >>>> + struct action_rss_data *rss_data; > >>>> > >>>> - rss = xmalloc(sizeof(*rss) + sizeof(uint16_t) * > >>>> netdev->n_rxq); > >>>> - /* > >>>> - * Setting it to NULL will let the driver use the default RSS > >>>> - * configuration we have set: &port_conf.rx_adv_conf.rss_conf. > >>>> - */ > >>>> - rss->rss_conf = NULL; > >>>> - rss->num = netdev->n_rxq; > >>>> + if (netdev->n_rxq > MAX_ACTION_RSS_QUEUE_NUM) { > >>>> + VLOG_ERR("Num of rxq (%u) must not be greater " \ > >>>> + "than max rss num of queues (%u)", > >>>> + netdev->n_rxq, MAX_ACTION_RSS_QUEUE_NUM); > >>>> + return NULL; > >>>> + } > >>>> > >>>> - for (i = 0; i < rss->num; i++) { > >>>> - rss->queue[i] = i; > >>>> + rss_data = xmalloc(sizeof(struct action_rss_data)); > >>>> + *rss_data = (struct action_rss_data){ > >>>> + .conf = (struct rte_flow_action_rss){ > >>>> + .func = RTE_ETH_HASH_FUNCTION_DEFAULT, > >>>> + .level = 0, > >>>> + .types = ETH_RSS_IP, > >>>> + .key_len = 0, > >>>> + .queue_num = netdev->n_rxq, > >>>> + .queue = rss_data->queue, > >>>> + .key = rss_data->key, > >>>> + }, > >>>> + .key = { 0 }, > >>>> + .queue = { 0 }, > >>>> + }; > >>>> + > >>>> + /* TODO: Override key with default */ > >>> [Sugesh] Do we really need this TODO?? > >>>> + > >>>> + /* Override queue array with default */ > >>>> + for (i = 0; i < rss_data->conf.queue_num; i++) { > >>>> + rss_data->queue[i] = i; > >>>> } > >>>> > >>>> - add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, rss); > >>>> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, > >>>> + &rss_data->conf); > >>>> > >>>> - return rss; > >>>> + return &rss_data->conf; > >>>> } > >>>> > >>>> static int > >>>> @@ -4365,7 +4417,7 @@ netdev_dpdk_add_rte_flow_offload(struct > >> netdev > >>>> *netdev, > >>>> vlan_mask.tci = match->wc.masks.vlans[0].tci & > >>>> ~htons(VLAN_CFI); > >>>> > >>>> /* match any protocols */ > >>>> - vlan_mask.tpid = 0; > >>>> + vlan_mask.inner_type = 0; > >>>> > >>>> add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_VLAN, > >>>> &vlan_spec, &vlan_mask); @@ -4516,6 > >>>> +4568,11 @@ > >>>> end_proto_check: > >>>> > >>>> struct rte_flow_action_rss *rss; > >>>> rss = add_flow_rss_action(&actions, netdev); > >>>> + if (!rss) { > >>>> + VLOG_ERR("add_flow_rss_action error.\n"); > >>> I feel this error can be bit more verbose something like Failed to > >>> add > >> rss_flow+actions or something?? What do you think? > >>>> + ret = -1; > >>>> + goto out; > >>>> + } > >>>> add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL); > >>>> > >>>> flow = rte_flow_create(dev->port_id, &flow_attr, > >>>> patterns.items, > >>>> -- > >>>> 1.8.3.1 > >>> > > > > _______________________________________________ > > dev mailing list > > dev@openvswitch.org > > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
Hi Sugesh, Patch V2 was sent https://patchwork.ozlabs.org/patch/968292/ Looking forward to your test results. Kevin - I suggest adding you as co-author (will require your sign off). Please confirm. Regards, Ophir > -----Original Message----- > From: Chandran, Sugesh [mailto:sugesh.chandran@intel.com] > Sent: Monday, September 10, 2018 5:58 PM > To: Eelco Chaudron <echaudro@redhat.com>; Ophir Munk > <ophirmu@mellanox.com> > Cc: Kevin Traynor <ktraynor@redhat.com>; ovs-dev@openvswitch.org; > Shahaf Shuler <shahafs@mellanox.com>; Asaf Penso > <asafp@mellanox.com>; Thomas Monjalon <thomas@monjalon.net> > Subject: RE: [ovs-dev] [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk > v18.08.0 > > Hi Ophir, > Could you please send out v2 of this patch with the changes from Kevin? > I will test it in our lab to see if its working > > > Regards > _Sugesh > > > -----Original Message----- > > From: Eelco Chaudron [mailto:echaudro@redhat.com] > > Sent: Wednesday, September 5, 2018 1:35 PM > > To: Ophir Munk <ophirmu@mellanox.com> > > Cc: Kevin Traynor <ktraynor@redhat.com>; Chandran, Sugesh > > <sugesh.chandran@intel.com>; ovs-dev@openvswitch.org; Shahaf Shuler > > <shahafs@mellanox.com>; Asaf Penso <asafp@mellanox.com>; Thomas > > Monjalon <thomas@monjalon.net> > > Subject: Re: [ovs-dev] [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to > > dpdk > > v18.08.0 > > > > Hi Ophir, > > > > I get the same error when trying this with an XL710, wanted to see if > > the patch would work for non-root users. > > Did not further troubleshoot it. > > > > Also I get the following compile warnings(errors): > > > > mv -f $depbase.Tpo $depbase.Po > > lib/netdev-dpdk.c: In function 'netdev_dpdk_destruct': > > lib/netdev-dpdk.c:1331:9: error: 'rte_eth_dev_detach' is deprecated > > (declared at > > /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native-linuxapp- > > gcc/include/rte_ethdev.h:1451) > > [-Werror=deprecated-declarations] > > if (rte_eth_dev_detach(dev->port_id, devname) < 0) { > > ^ > > lib/netdev-dpdk.c: In function 'netdev_dpdk_process_devargs': > > lib/netdev-dpdk.c:1624:13: error: 'rte_eth_dev_attach' is deprecated > > (declared at > > /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native-linuxapp- > > gcc/include/rte_ethdev.h:1435) > > [-Werror=deprecated-declarations] > > if (!rte_eth_dev_attach(devargs, &new_port_id)) { > > ^ > > lib/netdev-dpdk.c: In function 'netdev_dpdk_detach': > > lib/netdev-dpdk.c:3154:5: error: 'rte_eth_dev_detach' is deprecated > > (declared at > > /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native-linuxapp- > > gcc/include/rte_ethdev.h:1451) > > [-Werror=deprecated-declarations] > > ret = rte_eth_dev_detach(port_id, devname); > > ^ > > lib/netdev-dpdk.c: At top level: > > > > > > //Eelco > > > > > > On 5 Sep 2018, at 12:44, Ophir Munk wrote: > > > > > Hi, > > > I suggest the following trouble-shooting steps: > > > 1. Try Kevin's fix for Niantic (attached here as a non-official > > > patch on top of latest dpdk-hwol branch). > > > Sugesh - can you please try it? > > > 2. Sugesh - do you have the same issue with other NICs than Niantic > > > on latest dpdk-hwol branch? (other NICs used to work a month > > > ago) > > > 3. I will try on my setup. > > > > > > Regards, > > > Ophir > > > > > >> -----Original Message----- > > >> From: Kevin Traynor [mailto:ktraynor@redhat.com] > > >> Sent: Wednesday, September 05, 2018 9:28 AM > > >> To: Chandran, Sugesh <sugesh.chandran@intel.com>; Ophir Munk > > >> <ophirmu@mellanox.com>; ovs-dev@openvswitch.org > > >> Cc: Asaf Penso <asafp@mellanox.com>; Stokes, Ian > > >> <ian.stokes@intel.com>; Ben Pfaff <blp@ovn.org>; Shahaf Shuler > > >> <shahafs@mellanox.com>; Thomas Monjalon > <thomas@monjalon.net>; > > Olga > > >> Shern <olgas@mellanox.com> > > >> Subject: Re: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk > > >> v18.08.0 > > >> > > >> On 09/05/2018 07:44 AM, Chandran, Sugesh wrote: > > >>> Hi Ophir, > > >>> > > >>> Thank you for working on this. > > >>> I tried to run some tests and getting some errors while adding > > >>> physical ports as below. I use Niantic NIC for this test > > >>> > > >>>>>>>>>>>>>>>>>> > > >>> 2018-09-05T06:34:58Z|00069|dpdk|INFO|Device with port_id=0 > already > > >>> stopped 2018-09-05T06:34:58Z|00070|dpdk|ERR|Ethdev port_id=0 > > >>> invalid > > >>> rss_hf: 0x3afbc, valid value: 0x38d34 > > >>> 2018-09-05T06:34:58Z|00071|netdev_dpdk|WARN|Interface dpdk0 > > >> eth_dev > > >>> setup error Invalid argument > > >>> 2018-09-05T06:34:58Z|00072|netdev_dpdk|ERR|Interface > dpdk0(rxq:2 > > >> txq:2 > > >>> lsc interrupt mode:false) configure error: Invalid argument > > >>> 2018-09-05T06:34:58Z|00073|dpif_netdev|ERR|Failed to set interface > > >>> dpdk0 new configuration 2018-09- > > >> 05T06:34:58Z|00074|bridge|WARN|could > > >>> not add network device dpdk0 to ofproto (No such device) > > >>> ovs-vsctl: Error detected while setting up 'dpdk0': could not add > > >>> network > > >> device dpdk0 to ofproto (No such device). See ovs-vswitchd log for > > >> details. > > >>> ovs-vsctl: The default log directory is > > >>> "/usr/local/var/log/openvswitch". > > >>> > > >> > > >> I didn't have time to compare the below patch with the ones that I > > >> did, but I saw that this patch was needed for niantic > > >> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2F > > >> gi > > >> t > > >> hub.com%2Fkevintraynor%2Fovs-dpdk- > > >> > master%2Fcommit%2F88f46cc5ab338eb4f3ca5db1eacd0effefe4fa0c&d > > >> > ata=02%7C01%7Cophirmu%40mellanox.com%7Cd203c89637bf4f27835508d > > >> > 61309798e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C63671 > > >> > 7328752336932&sdata=L7rWODNNEStIfibwXybu6Ub7Yvn%2BfLHk6VT > > >> we14jxyY%3D&reserved=0 > > >> > > >>>>>>>>>>>>>>>>>>> > > >>> Please find below for my comments. > > >>> > > >>> > > >>> Regards > > >>> _Sugesh > > >>> > > >>>> -----Original Message----- > > >>>> From: Ophir Munk [mailto:ophirmu@mellanox.com] > > >>>> Sent: Monday, September 3, 2018 3:19 PM > > >>>> To: ovs-dev@openvswitch.org > > >>>> Cc: Asaf Penso <asafp@mellanox.com>; Chandran, Sugesh > > >>>> <sugesh.chandran@intel.com>; Stokes, Ian <ian.stokes@intel.com>; > > >>>> Ben Pfaff <blp@ovn.org>; Shahaf Shuler <shahafs@mellanox.com>; > > >>>> Thomas Monjalon <thomas@monjalon.net>; Olga Shern > > >>>> <olgas@mellanox.com>; Ophir Munk <ophirmu@mellanox.com>; > Kevin > > >>>> Traynor <ktraynor@redhat.com> > > >>>> Subject: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk > > >>>> v18.08.0 > > >>>> > > >>>> 1. Enable compilation and linkage with dpdk 18.08.0 The following > > >>>> dpdk commits which were introduced after dpdk 17.11.x require OVS > > >>>> updates to accommodate to the dpdk changes. > > >>>> - ce17eddefc20 ("ethdev: introduce Rx queue offloads API") > > >>>> - ab3ce1e0c193 ("ethdev: remove old offload API") > > >>>> - c06ddf9698e0 ("meter: add configuration profile") > > >>>> - e58638c32411 ("ethdev: fix TPID handling in flow API") > > >>>> - cd8c7c7ce241 ("ethdev: replace bus specific struct with generic > > >>>> dev") > > >>>> - ac8d22de2394 ("ethdev: flatten RSS configuration in flow API") > > >>>> > > >>>> 2. Update references to DPDK version in Documentation > > >>>> > > >>>> 3. Update DPDK version in travis' linux-build script > > >>>> > > >>>> Signed-off-by: Ophir Munk <ophirmu@mellanox.com> > > >>>> --- > > >>>> .travis/linux-build.sh | 2 +- > > >>>> Documentation/faq/releases.rst | 2 +- > > >>>> lib/netdev-dpdk.c | 135 > > >>>> +++++++++++++++++++++++++++++-------- > > >> ---- > > >>>> 3 files changed, 98 insertions(+), 41 deletions(-) > > >>>> > > >>>> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh > > >>>> index > > >>>> 4b9fc4a..c60ac71 > > >>>> 100755 > > >>>> --- a/.travis/linux-build.sh > > >>>> +++ b/.travis/linux-build.sh > > >>>> @@ -83,7 +83,7 @@ fi > > >>>> > > >>>> if [ "$DPDK" ]; then > > >>>> if [ -z "$DPDK_VER" ]; then > > >>>> - DPDK_VER="17.11.3" > > >>>> + DPDK_VER="18.08.0" > > >>>> fi > > >>>> install_dpdk $DPDK_VER > > >>>> if [ "$CC" = "clang" ]; then diff --git > > >>>> a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst > > >>>> index 41d41e3..646ae09 100644 > > >>>> --- a/Documentation/faq/releases.rst > > >>>> +++ b/Documentation/faq/releases.rst > > >>>> @@ -168,7 +168,7 @@ Q: What DPDK version does each Open > vSwitch > > >>>> release work with? > > >>>> 2.7.x 16.11.7 > > >>>> 2.8.x 17.05.2 > > >>>> 2.9.x 17.11.3 > > >>>> - 2.10.x 17.11.3 > > >>>> + 2.10.x 18.08.0 > > >>>> ============ ======= > > >>>> > > >>>> Q: Are all the DPDK releases that OVS versions work with > > >>>> maintained? > > >>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > > >>>> f91aa27..80d2af9 > > >>>> 100644 > > >>>> --- a/lib/netdev-dpdk.c > > >>>> +++ b/lib/netdev-dpdk.c > > >>>> @@ -168,11 +168,7 @@ static const struct rte_eth_conf port_conf = > { > > >>>> .rxmode = { > > >>>> .mq_mode = ETH_MQ_RX_RSS, > > >>>> .split_hdr_size = 0, > > >>>> - .header_split = 0, /* Header Split disabled */ > > >>>> - .hw_ip_checksum = 0, /* IP checksum offload disabled */ > > >>>> - .hw_vlan_filter = 0, /* VLAN filtering disabled */ > > >>>> - .jumbo_frame = 0, /* Jumbo Frame Support disabled */ > > >>>> - .hw_strip_crc = 0, > > >>>> + .offloads = 0, > > >>>> }, > > >>>> .rx_adv_conf = { > > >>>> .rss_conf = { > > >>>> @@ -364,6 +360,7 @@ struct dpdk_ring { struct ingress_policer { > > >>>> struct rte_meter_srtcm_params app_srtcm_params; > > >>>> struct rte_meter_srtcm in_policer; > > >>>> + struct rte_meter_srtcm_profile in_prof; > > >>>> rte_spinlock_t policer_lock; }; > > >>>> > > >>>> @@ -903,16 +900,17 @@ dpdk_eth_dev_port_config(struct > netdev_dpdk > > >>>> *dev, int n_rxq, int n_txq) > > >>>> if (dev->mtu > ETHER_MTU) { > > >>>> rte_eth_dev_info_get(dev->port_id, &info); > > >>>> if (strncmp(info.driver_name, "net_nfp", 7)) { > > >>>> - conf.rxmode.enable_scatter = 1; > > >>>> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_SCATTER; > > >>>> } > > >>>> } > > >>>> > > >>>> conf.intr_conf.lsc = dev->lsc_interrupt_mode; > > >>>> - conf.rxmode.hw_ip_checksum = (dev->hw_ol_features & > > >>>> - NETDEV_RX_CHECKSUM_OFFLOAD) != > > >>>> 0; > > >>>> + conf.rxmode.offloads |= ((dev->hw_ol_features & > > >>>> + NETDEV_RX_CHECKSUM_OFFLOAD) != 0) ? > > >>>> + DEV_RX_OFFLOAD_CHECKSUM : 0; > > >>>> > > >>>> if (dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP) { > > >>>> - conf.rxmode.hw_strip_crc = 1; > > >>>> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP; > > >>>> } > > >>>> > > >>>> /* A device may report more queues than it makes available > > >>>> (this has @@ - > > >>>> 1932,16 +1930,18 @@ netdev_dpdk_eth_tx_burst(struct > netdev_dpdk > > >> *dev, > > >>>> int qid, > > >>>> > > >>>> static inline bool > > >>>> netdev_dpdk_policer_pkt_handle(struct rte_meter_srtcm *meter, > > >>>> + struct rte_meter_srtcm_profile > > >>>> + *profile, > > >>>> struct rte_mbuf *pkt, uint64_t > > >>>> time) { > > >>>> uint32_t pkt_len = rte_pktmbuf_pkt_len(pkt) - sizeof(struct > > >>>> ether_hdr); > > >>>> > > >>>> - return rte_meter_srtcm_color_blind_check(meter, time, pkt_len) > > >>>> == > > >>>> + return rte_meter_srtcm_color_blind_check(meter, profile, > > >>>> + time, > > >>>> + pkt_len) == > > >>>> > > >>>> e_RTE_METER_GREEN; } > > >>>> > > >>>> static int > > >>>> netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, > > >>>> + struct rte_meter_srtcm_profile *profile, > > >>>> struct rte_mbuf **pkts, int pkt_cnt, > > >>>> bool should_steal) { @@ -1953,7 +1953,8 > > >>>> @@ netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, > > >>>> for (i = 0; i < pkt_cnt; i++) { > > >>>> pkt = pkts[i]; > > >>>> /* Handle current packet */ > > >>>> - if (netdev_dpdk_policer_pkt_handle(meter, pkt, > > >>>> current_time)) { > > >>>> + if (netdev_dpdk_policer_pkt_handle(meter, profile, pkt, > > >>>> + current_time)) > > >>>> + { > > >>>> if (cnt != i) { > > >>>> pkts[cnt] = pkt; > > >>>> } > > >>>> @@ -1975,8 +1976,8 @@ ingress_policer_run(struct ingress_policer > > >>>> *policer, struct rte_mbuf **pkts, > > >>>> int cnt = 0; > > >>>> > > >>>> rte_spinlock_lock(&policer->policer_lock); > > >>>> - cnt = netdev_dpdk_policer_run(&policer->in_policer, pkts, > > >>>> - pkt_cnt, should_steal); > > >>>> + cnt = netdev_dpdk_policer_run(&policer->in_policer, > > >>>> + &policer- > > >>> in_prof, > > >>>> + pkts, pkt_cnt, should_steal); > > >>>> rte_spinlock_unlock(&policer->policer_lock); > > >>>> > > >>>> return cnt; > > >>>> @@ -2767,8 +2768,15 @@ netdev_dpdk_policer_construct(uint32_t > > >>>> rate, uint32_t burst) > > >>>> policer->app_srtcm_params.cir = rate_bytes; > > >>>> policer->app_srtcm_params.cbs = burst_bytes; > > >>>> policer->app_srtcm_params.ebs = 0; > > >>>> + err = rte_meter_srtcm_profile_config(&policer->in_prof, > > >>>> + > > >>>> &policer->app_srtcm_params); > > >>>> + if (err) { > > >>>> + VLOG_ERR("Could not create rte meter profile for ingress > > >>>> policer"); > > >>>> + free(policer); > > >>>> + return NULL; > > >>>> + } > > >>>> err = rte_meter_srtcm_config(&policer->in_policer, > > >>>> - &policer->app_srtcm_params); > > >>>> + &policer->in_prof); > > >>>> if (err) { > > >>>> VLOG_ERR("Could not create rte meter for ingress > > >>>> policer"); > > >>>> free(policer); > > >>>> @@ -3043,13 +3051,18 @@ netdev_dpdk_get_status(const struct > > >>>> netdev *netdev, struct smap *args) > > >>>> smap_add_format(args, "if_descr", "%s %s", rte_version(), > > >>>> > > >>>> dev_info.driver_name); > > >>>> > > >>>> - if (dev_info.pci_dev) { > > >>>> - smap_add_format(args, "pci-vendor_id", "0x%x", > > >>>> - dev_info.pci_dev->id.vendor_id); > > >>>> - smap_add_format(args, "pci-device_id", "0x%x", > > >>>> - dev_info.pci_dev->id.device_id); > > >>>> + const struct rte_bus *bus; > > >>>> + const struct rte_pci_device *pci_dev; > > >>>> + bus = rte_bus_find_by_device(dev_info.device); > > >>>> + if (bus && !strcmp(bus->name, "pci")) { > > >>>> + pci_dev = RTE_DEV_TO_PCI(dev_info.device); > > >>>> + if (pci_dev) { > > >>>> + smap_add_format(args, "pci-vendor_id", "0x%u", > > >>>> + pci_dev->id.vendor_id); > > >>>> + smap_add_format(args, "pci-device_id", "0x%x", > > >>>> + pci_dev->id.device_id); > > >>>> + } > > >>>> } > > >>>> - > > >>>> return 0; > > >>>> } > > >>>> > > >>>> @@ -3727,6 +3740,7 @@ struct egress_policer { > > >>>> struct qos_conf qos_conf; > > >>>> struct rte_meter_srtcm_params app_srtcm_params; > > >>>> struct rte_meter_srtcm egress_meter; > > >>>> + struct rte_meter_srtcm_profile egress_prof; > > >>>> }; > > >>>> > > >>>> static void > > >>>> @@ -3749,8 +3763,16 @@ egress_policer_qos_construct(const > struct > > >> smap > > >>>> *details, > > >>>> policer = xmalloc(sizeof *policer); > > >>>> qos_conf_init(&policer->qos_conf, &egress_policer_ops); > > >>>> egress_policer_details_to_param(details, > > >>>> &policer->app_srtcm_params); > > >>>> + err = rte_meter_srtcm_profile_config(&policer->egress_prof, > > >>>> + > > >>>> &policer->app_srtcm_params); > > >>>> + if (err) { > > >>> [Sugesh] Should we error out the message on failing the profile > > >>> config?? > > >>>> + free(policer); > > >>>> + *conf = NULL; > > >>>> + return -err; > > >>>> + } > > >>>> err = rte_meter_srtcm_config(&policer->egress_meter, > > >>>> - &policer->app_srtcm_params); > > >>>> + &policer->egress_prof); > > >>>> + > > >>>> if (!err) { > > >>>> *conf = &policer->qos_conf; > > >>>> } else { > > >>>> @@ -3803,7 +3825,8 @@ egress_policer_run(struct qos_conf *conf, > > >>>> struct rte_mbuf **pkts, int pkt_cnt, > > >>>> struct egress_policer *policer = > > >>>> CONTAINER_OF(conf, struct egress_policer, qos_conf); > > >>>> > > >>>> - cnt = netdev_dpdk_policer_run(&policer->egress_meter, pkts, > > >>>> + cnt = netdev_dpdk_policer_run(&policer->egress_meter, > > >>>> + &policer->egress_prof, pkts, > > >>>> pkt_cnt, should_steal); > > >>>> > > >>>> return cnt; > > >>>> @@ -4103,15 +4126,15 @@ dump_flow_pattern(struct > rte_flow_item > > >> *item) > > >>>> > > >>>> VLOG_DBG("rte flow vlan pattern:\n"); > > >>>> if (vlan_spec) { > > >>>> - VLOG_DBG(" Spec: tpid=0x%"PRIx16", > > >>>> tci=0x%"PRIx16"\n", > > >>>> - ntohs(vlan_spec->tpid), > > >>>> ntohs(vlan_spec->tci)); > > >>>> + VLOG_DBG(" Spec: inner_type=0x%"PRIx16", > > >>>> tci=0x%"PRIx16"\n", > > >>>> + ntohs(vlan_spec->inner_type), > > >>>> + ntohs(vlan_spec->tci)); > > >>>> } else { > > >>>> VLOG_DBG(" Spec = null\n"); > > >>>> } > > >>>> > > >>>> if (vlan_mask) { > > >>>> - VLOG_DBG(" Mask: tpid=0x%"PRIx16", > > >>>> tci=0x%"PRIx16"\n", > > >>>> - vlan_mask->tpid, vlan_mask->tci); > > >>>> + VLOG_DBG(" Mask: inner_type=0x%"PRIx16", > > >> tci=0x%"PRIx16"\n", > > >>>> + vlan_mask->inner_type, vlan_mask->tci); > > >>>> } else { > > >>>> VLOG_DBG(" Mask = null\n"); > > >>>> } > > >>>> @@ -4281,27 +4304,56 @@ add_flow_action(struct flow_actions > > >> *actions, > > >>>> enum rte_flow_action_type type, > > >>>> actions->cnt++; > > >>>> } > > >>>> > > >>>> +/* > > >>>> + * Storage for struct rte_flow_action_rss > > >>>> + * including storage for key and queue array */ #define > > >>>> +MAX_RSS_HASH_KEY_LENGTH 128 #define > > >> MAX_ACTION_RSS_QUEUE_NUM > > >>>> 128 > > >>>> +struct action_rss_data { > > >>>> + struct rte_flow_action_rss conf; > > >>>> + uint16_t queue[MAX_ACTION_RSS_QUEUE_NUM]; > > >>>> + uint8_t key[MAX_RSS_HASH_KEY_LENGTH]; }; > > >>>> + > > >>>> static struct rte_flow_action_rss * add_flow_rss_action(struct > > >>>> flow_actions *actions, > > >>>> struct netdev *netdev) { > > >>>> int i; > > >>>> - struct rte_flow_action_rss *rss; > > >>>> + struct action_rss_data *rss_data; > > >>>> > > >>>> - rss = xmalloc(sizeof(*rss) + sizeof(uint16_t) * > > >>>> netdev->n_rxq); > > >>>> - /* > > >>>> - * Setting it to NULL will let the driver use the default RSS > > >>>> - * configuration we have set: &port_conf.rx_adv_conf.rss_conf. > > >>>> - */ > > >>>> - rss->rss_conf = NULL; > > >>>> - rss->num = netdev->n_rxq; > > >>>> + if (netdev->n_rxq > MAX_ACTION_RSS_QUEUE_NUM) { > > >>>> + VLOG_ERR("Num of rxq (%u) must not be greater " \ > > >>>> + "than max rss num of queues (%u)", > > >>>> + netdev->n_rxq, MAX_ACTION_RSS_QUEUE_NUM); > > >>>> + return NULL; > > >>>> + } > > >>>> > > >>>> - for (i = 0; i < rss->num; i++) { > > >>>> - rss->queue[i] = i; > > >>>> + rss_data = xmalloc(sizeof(struct action_rss_data)); > > >>>> + *rss_data = (struct action_rss_data){ > > >>>> + .conf = (struct rte_flow_action_rss){ > > >>>> + .func = RTE_ETH_HASH_FUNCTION_DEFAULT, > > >>>> + .level = 0, > > >>>> + .types = ETH_RSS_IP, > > >>>> + .key_len = 0, > > >>>> + .queue_num = netdev->n_rxq, > > >>>> + .queue = rss_data->queue, > > >>>> + .key = rss_data->key, > > >>>> + }, > > >>>> + .key = { 0 }, > > >>>> + .queue = { 0 }, > > >>>> + }; > > >>>> + > > >>>> + /* TODO: Override key with default */ > > >>> [Sugesh] Do we really need this TODO?? > > >>>> + > > >>>> + /* Override queue array with default */ > > >>>> + for (i = 0; i < rss_data->conf.queue_num; i++) { > > >>>> + rss_data->queue[i] = i; > > >>>> } > > >>>> > > >>>> - add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, rss); > > >>>> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, > > >>>> + &rss_data->conf); > > >>>> > > >>>> - return rss; > > >>>> + return &rss_data->conf; > > >>>> } > > >>>> > > >>>> static int > > >>>> @@ -4365,7 +4417,7 @@ netdev_dpdk_add_rte_flow_offload(struct > > >> netdev > > >>>> *netdev, > > >>>> vlan_mask.tci = match->wc.masks.vlans[0].tci & > > >>>> ~htons(VLAN_CFI); > > >>>> > > >>>> /* match any protocols */ > > >>>> - vlan_mask.tpid = 0; > > >>>> + vlan_mask.inner_type = 0; > > >>>> > > >>>> add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_VLAN, > > >>>> &vlan_spec, &vlan_mask); @@ -4516,6 > > >>>> +4568,11 @@ > > >>>> end_proto_check: > > >>>> > > >>>> struct rte_flow_action_rss *rss; > > >>>> rss = add_flow_rss_action(&actions, netdev); > > >>>> + if (!rss) { > > >>>> + VLOG_ERR("add_flow_rss_action error.\n"); > > >>> I feel this error can be bit more verbose something like Failed to > > >>> add > > >> rss_flow+actions or something?? What do you think? > > >>>> + ret = -1; > > >>>> + goto out; > > >>>> + } > > >>>> add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, > NULL); > > >>>> > > >>>> flow = rte_flow_create(dev->port_id, &flow_attr, > > >>>> patterns.items, > > >>>> -- > > >>>> 1.8.3.1 > > >>> > > > > > > _______________________________________________ > > > dev mailing list > > > dev@openvswitch.org > > > > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fm > > > ail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs- > dev&data=02%7C01% > > > > 7Cophirmu%40mellanox.com%7C028cdfe3e1154f5cc99e08d6172dc231%7C > a65297 > > > > 1c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636721882650055912&s > data=Wx > > > > VUAkghNz3pgtAqJ%2FyH0tBkgnQcBaMBIx3RI1FM%2BG4%3D&reserve > d=0
On 09/11/2018 12:10 AM, Ophir Munk wrote: > Hi Sugesh, > > Patch V2 was sent https://patchwork.ozlabs.org/patch/968292/ > Looking forward to your test results. > > Kevin - I suggest adding you as co-author (will require your sign off). Please confirm. > Hi Ophir, I don't think it's needed. Actually, you made a simpler way to fix it, based on the idea :-) Kevin. > Regards, > Ophir > >> -----Original Message----- >> From: Chandran, Sugesh [mailto:sugesh.chandran@intel.com] >> Sent: Monday, September 10, 2018 5:58 PM >> To: Eelco Chaudron <echaudro@redhat.com>; Ophir Munk >> <ophirmu@mellanox.com> >> Cc: Kevin Traynor <ktraynor@redhat.com>; ovs-dev@openvswitch.org; >> Shahaf Shuler <shahafs@mellanox.com>; Asaf Penso >> <asafp@mellanox.com>; Thomas Monjalon <thomas@monjalon.net> >> Subject: RE: [ovs-dev] [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk >> v18.08.0 >> >> Hi Ophir, >> Could you please send out v2 of this patch with the changes from Kevin? >> I will test it in our lab to see if its working >> >> >> Regards >> _Sugesh >> >>> -----Original Message----- >>> From: Eelco Chaudron [mailto:echaudro@redhat.com] >>> Sent: Wednesday, September 5, 2018 1:35 PM >>> To: Ophir Munk <ophirmu@mellanox.com> >>> Cc: Kevin Traynor <ktraynor@redhat.com>; Chandran, Sugesh >>> <sugesh.chandran@intel.com>; ovs-dev@openvswitch.org; Shahaf Shuler >>> <shahafs@mellanox.com>; Asaf Penso <asafp@mellanox.com>; Thomas >>> Monjalon <thomas@monjalon.net> >>> Subject: Re: [ovs-dev] [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to >>> dpdk >>> v18.08.0 >>> >>> Hi Ophir, >>> >>> I get the same error when trying this with an XL710, wanted to see if >>> the patch would work for non-root users. >>> Did not further troubleshoot it. >>> >>> Also I get the following compile warnings(errors): >>> >>> mv -f $depbase.Tpo $depbase.Po >>> lib/netdev-dpdk.c: In function 'netdev_dpdk_destruct': >>> lib/netdev-dpdk.c:1331:9: error: 'rte_eth_dev_detach' is deprecated >>> (declared at >>> /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native-linuxapp- >>> gcc/include/rte_ethdev.h:1451) >>> [-Werror=deprecated-declarations] >>> if (rte_eth_dev_detach(dev->port_id, devname) < 0) { >>> ^ >>> lib/netdev-dpdk.c: In function 'netdev_dpdk_process_devargs': >>> lib/netdev-dpdk.c:1624:13: error: 'rte_eth_dev_attach' is deprecated >>> (declared at >>> /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native-linuxapp- >>> gcc/include/rte_ethdev.h:1435) >>> [-Werror=deprecated-declarations] >>> if (!rte_eth_dev_attach(devargs, &new_port_id)) { >>> ^ >>> lib/netdev-dpdk.c: In function 'netdev_dpdk_detach': >>> lib/netdev-dpdk.c:3154:5: error: 'rte_eth_dev_detach' is deprecated >>> (declared at >>> /home/root/OVS_dpdk-hwol_DPDK_v18.08/dpdk/x86_64-native-linuxapp- >>> gcc/include/rte_ethdev.h:1451) >>> [-Werror=deprecated-declarations] >>> ret = rte_eth_dev_detach(port_id, devname); >>> ^ >>> lib/netdev-dpdk.c: At top level: >>> >>> >>> //Eelco >>> >>> >>> On 5 Sep 2018, at 12:44, Ophir Munk wrote: >>> >>>> Hi, >>>> I suggest the following trouble-shooting steps: >>>> 1. Try Kevin's fix for Niantic (attached here as a non-official >>>> patch on top of latest dpdk-hwol branch). >>>> Sugesh - can you please try it? >>>> 2. Sugesh - do you have the same issue with other NICs than Niantic >>>> on latest dpdk-hwol branch? (other NICs used to work a month >>>> ago) >>>> 3. I will try on my setup. >>>> >>>> Regards, >>>> Ophir >>>> >>>>> -----Original Message----- >>>>> From: Kevin Traynor [mailto:ktraynor@redhat.com] >>>>> Sent: Wednesday, September 05, 2018 9:28 AM >>>>> To: Chandran, Sugesh <sugesh.chandran@intel.com>; Ophir Munk >>>>> <ophirmu@mellanox.com>; ovs-dev@openvswitch.org >>>>> Cc: Asaf Penso <asafp@mellanox.com>; Stokes, Ian >>>>> <ian.stokes@intel.com>; Ben Pfaff <blp@ovn.org>; Shahaf Shuler >>>>> <shahafs@mellanox.com>; Thomas Monjalon >> <thomas@monjalon.net>; >>> Olga >>>>> Shern <olgas@mellanox.com> >>>>> Subject: Re: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk >>>>> v18.08.0 >>>>> >>>>> On 09/05/2018 07:44 AM, Chandran, Sugesh wrote: >>>>>> Hi Ophir, >>>>>> >>>>>> Thank you for working on this. >>>>>> I tried to run some tests and getting some errors while adding >>>>>> physical ports as below. I use Niantic NIC for this test >>>>>> >>>>>>>>>>>>>>>>>>>>> >>>>>> 2018-09-05T06:34:58Z|00069|dpdk|INFO|Device with port_id=0 >> already >>>>>> stopped 2018-09-05T06:34:58Z|00070|dpdk|ERR|Ethdev port_id=0 >>>>>> invalid >>>>>> rss_hf: 0x3afbc, valid value: 0x38d34 >>>>>> 2018-09-05T06:34:58Z|00071|netdev_dpdk|WARN|Interface dpdk0 >>>>> eth_dev >>>>>> setup error Invalid argument >>>>>> 2018-09-05T06:34:58Z|00072|netdev_dpdk|ERR|Interface >> dpdk0(rxq:2 >>>>> txq:2 >>>>>> lsc interrupt mode:false) configure error: Invalid argument >>>>>> 2018-09-05T06:34:58Z|00073|dpif_netdev|ERR|Failed to set interface >>>>>> dpdk0 new configuration 2018-09- >>>>> 05T06:34:58Z|00074|bridge|WARN|could >>>>>> not add network device dpdk0 to ofproto (No such device) >>>>>> ovs-vsctl: Error detected while setting up 'dpdk0': could not add >>>>>> network >>>>> device dpdk0 to ofproto (No such device). See ovs-vswitchd log for >>>>> details. >>>>>> ovs-vsctl: The default log directory is >>>>>> "/usr/local/var/log/openvswitch". >>>>>> >>>>> >>>>> I didn't have time to compare the below patch with the ones that I >>>>> did, but I saw that this patch was needed for niantic >>>>> >> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2F >>>>> gi >>>>> t >>>>> hub.com%2Fkevintraynor%2Fovs-dpdk- >>>>> >> master%2Fcommit%2F88f46cc5ab338eb4f3ca5db1eacd0effefe4fa0c&d >>>>> >> ata=02%7C01%7Cophirmu%40mellanox.com%7Cd203c89637bf4f27835508d >>>>> >> 61309798e%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C63671 >>>>> >> 7328752336932&sdata=L7rWODNNEStIfibwXybu6Ub7Yvn%2BfLHk6VT >>>>> we14jxyY%3D&reserved=0 >>>>> >>>>>>>>>>>>>>>>>>>>>> >>>>>> Please find below for my comments. >>>>>> >>>>>> >>>>>> Regards >>>>>> _Sugesh >>>>>> >>>>>>> -----Original Message----- >>>>>>> From: Ophir Munk [mailto:ophirmu@mellanox.com] >>>>>>> Sent: Monday, September 3, 2018 3:19 PM >>>>>>> To: ovs-dev@openvswitch.org >>>>>>> Cc: Asaf Penso <asafp@mellanox.com>; Chandran, Sugesh >>>>>>> <sugesh.chandran@intel.com>; Stokes, Ian <ian.stokes@intel.com>; >>>>>>> Ben Pfaff <blp@ovn.org>; Shahaf Shuler <shahafs@mellanox.com>; >>>>>>> Thomas Monjalon <thomas@monjalon.net>; Olga Shern >>>>>>> <olgas@mellanox.com>; Ophir Munk <ophirmu@mellanox.com>; >> Kevin >>>>>>> Traynor <ktraynor@redhat.com> >>>>>>> Subject: [dpdk-hwol PATCH v1] netdev-dpdk: Upgrade to dpdk >>>>>>> v18.08.0 >>>>>>> >>>>>>> 1. Enable compilation and linkage with dpdk 18.08.0 The following >>>>>>> dpdk commits which were introduced after dpdk 17.11.x require OVS >>>>>>> updates to accommodate to the dpdk changes. >>>>>>> - ce17eddefc20 ("ethdev: introduce Rx queue offloads API") >>>>>>> - ab3ce1e0c193 ("ethdev: remove old offload API") >>>>>>> - c06ddf9698e0 ("meter: add configuration profile") >>>>>>> - e58638c32411 ("ethdev: fix TPID handling in flow API") >>>>>>> - cd8c7c7ce241 ("ethdev: replace bus specific struct with generic >>>>>>> dev") >>>>>>> - ac8d22de2394 ("ethdev: flatten RSS configuration in flow API") >>>>>>> >>>>>>> 2. Update references to DPDK version in Documentation >>>>>>> >>>>>>> 3. Update DPDK version in travis' linux-build script >>>>>>> >>>>>>> Signed-off-by: Ophir Munk <ophirmu@mellanox.com> >>>>>>> --- >>>>>>> .travis/linux-build.sh | 2 +- >>>>>>> Documentation/faq/releases.rst | 2 +- >>>>>>> lib/netdev-dpdk.c | 135 >>>>>>> +++++++++++++++++++++++++++++-------- >>>>> ---- >>>>>>> 3 files changed, 98 insertions(+), 41 deletions(-) >>>>>>> >>>>>>> diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh >>>>>>> index >>>>>>> 4b9fc4a..c60ac71 >>>>>>> 100755 >>>>>>> --- a/.travis/linux-build.sh >>>>>>> +++ b/.travis/linux-build.sh >>>>>>> @@ -83,7 +83,7 @@ fi >>>>>>> >>>>>>> if [ "$DPDK" ]; then >>>>>>> if [ -z "$DPDK_VER" ]; then >>>>>>> - DPDK_VER="17.11.3" >>>>>>> + DPDK_VER="18.08.0" >>>>>>> fi >>>>>>> install_dpdk $DPDK_VER >>>>>>> if [ "$CC" = "clang" ]; then diff --git >>>>>>> a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst >>>>>>> index 41d41e3..646ae09 100644 >>>>>>> --- a/Documentation/faq/releases.rst >>>>>>> +++ b/Documentation/faq/releases.rst >>>>>>> @@ -168,7 +168,7 @@ Q: What DPDK version does each Open >> vSwitch >>>>>>> release work with? >>>>>>> 2.7.x 16.11.7 >>>>>>> 2.8.x 17.05.2 >>>>>>> 2.9.x 17.11.3 >>>>>>> - 2.10.x 17.11.3 >>>>>>> + 2.10.x 18.08.0 >>>>>>> ============ ======= >>>>>>> >>>>>>> Q: Are all the DPDK releases that OVS versions work with >>>>>>> maintained? >>>>>>> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index >>>>>>> f91aa27..80d2af9 >>>>>>> 100644 >>>>>>> --- a/lib/netdev-dpdk.c >>>>>>> +++ b/lib/netdev-dpdk.c >>>>>>> @@ -168,11 +168,7 @@ static const struct rte_eth_conf port_conf = >> { >>>>>>> .rxmode = { >>>>>>> .mq_mode = ETH_MQ_RX_RSS, >>>>>>> .split_hdr_size = 0, >>>>>>> - .header_split = 0, /* Header Split disabled */ >>>>>>> - .hw_ip_checksum = 0, /* IP checksum offload disabled */ >>>>>>> - .hw_vlan_filter = 0, /* VLAN filtering disabled */ >>>>>>> - .jumbo_frame = 0, /* Jumbo Frame Support disabled */ >>>>>>> - .hw_strip_crc = 0, >>>>>>> + .offloads = 0, >>>>>>> }, >>>>>>> .rx_adv_conf = { >>>>>>> .rss_conf = { >>>>>>> @@ -364,6 +360,7 @@ struct dpdk_ring { struct ingress_policer { >>>>>>> struct rte_meter_srtcm_params app_srtcm_params; >>>>>>> struct rte_meter_srtcm in_policer; >>>>>>> + struct rte_meter_srtcm_profile in_prof; >>>>>>> rte_spinlock_t policer_lock; }; >>>>>>> >>>>>>> @@ -903,16 +900,17 @@ dpdk_eth_dev_port_config(struct >> netdev_dpdk >>>>>>> *dev, int n_rxq, int n_txq) >>>>>>> if (dev->mtu > ETHER_MTU) { >>>>>>> rte_eth_dev_info_get(dev->port_id, &info); >>>>>>> if (strncmp(info.driver_name, "net_nfp", 7)) { >>>>>>> - conf.rxmode.enable_scatter = 1; >>>>>>> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_SCATTER; >>>>>>> } >>>>>>> } >>>>>>> >>>>>>> conf.intr_conf.lsc = dev->lsc_interrupt_mode; >>>>>>> - conf.rxmode.hw_ip_checksum = (dev->hw_ol_features & >>>>>>> - NETDEV_RX_CHECKSUM_OFFLOAD) != >>>>>>> 0; >>>>>>> + conf.rxmode.offloads |= ((dev->hw_ol_features & >>>>>>> + NETDEV_RX_CHECKSUM_OFFLOAD) != 0) ? >>>>>>> + DEV_RX_OFFLOAD_CHECKSUM : 0; >>>>>>> >>>>>>> if (dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP) { >>>>>>> - conf.rxmode.hw_strip_crc = 1; >>>>>>> + conf.rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP; >>>>>>> } >>>>>>> >>>>>>> /* A device may report more queues than it makes available >>>>>>> (this has @@ - >>>>>>> 1932,16 +1930,18 @@ netdev_dpdk_eth_tx_burst(struct >> netdev_dpdk >>>>> *dev, >>>>>>> int qid, >>>>>>> >>>>>>> static inline bool >>>>>>> netdev_dpdk_policer_pkt_handle(struct rte_meter_srtcm *meter, >>>>>>> + struct rte_meter_srtcm_profile >>>>>>> + *profile, >>>>>>> struct rte_mbuf *pkt, uint64_t >>>>>>> time) { >>>>>>> uint32_t pkt_len = rte_pktmbuf_pkt_len(pkt) - sizeof(struct >>>>>>> ether_hdr); >>>>>>> >>>>>>> - return rte_meter_srtcm_color_blind_check(meter, time, pkt_len) >>>>>>> == >>>>>>> + return rte_meter_srtcm_color_blind_check(meter, profile, >>>>>>> + time, >>>>>>> + pkt_len) == >>>>>>> >>>>>>> e_RTE_METER_GREEN; } >>>>>>> >>>>>>> static int >>>>>>> netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, >>>>>>> + struct rte_meter_srtcm_profile *profile, >>>>>>> struct rte_mbuf **pkts, int pkt_cnt, >>>>>>> bool should_steal) { @@ -1953,7 +1953,8 >>>>>>> @@ netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, >>>>>>> for (i = 0; i < pkt_cnt; i++) { >>>>>>> pkt = pkts[i]; >>>>>>> /* Handle current packet */ >>>>>>> - if (netdev_dpdk_policer_pkt_handle(meter, pkt, >>>>>>> current_time)) { >>>>>>> + if (netdev_dpdk_policer_pkt_handle(meter, profile, pkt, >>>>>>> + current_time)) >>>>>>> + { >>>>>>> if (cnt != i) { >>>>>>> pkts[cnt] = pkt; >>>>>>> } >>>>>>> @@ -1975,8 +1976,8 @@ ingress_policer_run(struct ingress_policer >>>>>>> *policer, struct rte_mbuf **pkts, >>>>>>> int cnt = 0; >>>>>>> >>>>>>> rte_spinlock_lock(&policer->policer_lock); >>>>>>> - cnt = netdev_dpdk_policer_run(&policer->in_policer, pkts, >>>>>>> - pkt_cnt, should_steal); >>>>>>> + cnt = netdev_dpdk_policer_run(&policer->in_policer, >>>>>>> + &policer- >>>>>> in_prof, >>>>>>> + pkts, pkt_cnt, should_steal); >>>>>>> rte_spinlock_unlock(&policer->policer_lock); >>>>>>> >>>>>>> return cnt; >>>>>>> @@ -2767,8 +2768,15 @@ netdev_dpdk_policer_construct(uint32_t >>>>>>> rate, uint32_t burst) >>>>>>> policer->app_srtcm_params.cir = rate_bytes; >>>>>>> policer->app_srtcm_params.cbs = burst_bytes; >>>>>>> policer->app_srtcm_params.ebs = 0; >>>>>>> + err = rte_meter_srtcm_profile_config(&policer->in_prof, >>>>>>> + >>>>>>> &policer->app_srtcm_params); >>>>>>> + if (err) { >>>>>>> + VLOG_ERR("Could not create rte meter profile for ingress >>>>>>> policer"); >>>>>>> + free(policer); >>>>>>> + return NULL; >>>>>>> + } >>>>>>> err = rte_meter_srtcm_config(&policer->in_policer, >>>>>>> - &policer->app_srtcm_params); >>>>>>> + &policer->in_prof); >>>>>>> if (err) { >>>>>>> VLOG_ERR("Could not create rte meter for ingress >>>>>>> policer"); >>>>>>> free(policer); >>>>>>> @@ -3043,13 +3051,18 @@ netdev_dpdk_get_status(const struct >>>>>>> netdev *netdev, struct smap *args) >>>>>>> smap_add_format(args, "if_descr", "%s %s", rte_version(), >>>>>>> >>>>>>> dev_info.driver_name); >>>>>>> >>>>>>> - if (dev_info.pci_dev) { >>>>>>> - smap_add_format(args, "pci-vendor_id", "0x%x", >>>>>>> - dev_info.pci_dev->id.vendor_id); >>>>>>> - smap_add_format(args, "pci-device_id", "0x%x", >>>>>>> - dev_info.pci_dev->id.device_id); >>>>>>> + const struct rte_bus *bus; >>>>>>> + const struct rte_pci_device *pci_dev; >>>>>>> + bus = rte_bus_find_by_device(dev_info.device); >>>>>>> + if (bus && !strcmp(bus->name, "pci")) { >>>>>>> + pci_dev = RTE_DEV_TO_PCI(dev_info.device); >>>>>>> + if (pci_dev) { >>>>>>> + smap_add_format(args, "pci-vendor_id", "0x%u", >>>>>>> + pci_dev->id.vendor_id); >>>>>>> + smap_add_format(args, "pci-device_id", "0x%x", >>>>>>> + pci_dev->id.device_id); >>>>>>> + } >>>>>>> } >>>>>>> - >>>>>>> return 0; >>>>>>> } >>>>>>> >>>>>>> @@ -3727,6 +3740,7 @@ struct egress_policer { >>>>>>> struct qos_conf qos_conf; >>>>>>> struct rte_meter_srtcm_params app_srtcm_params; >>>>>>> struct rte_meter_srtcm egress_meter; >>>>>>> + struct rte_meter_srtcm_profile egress_prof; >>>>>>> }; >>>>>>> >>>>>>> static void >>>>>>> @@ -3749,8 +3763,16 @@ egress_policer_qos_construct(const >> struct >>>>> smap >>>>>>> *details, >>>>>>> policer = xmalloc(sizeof *policer); >>>>>>> qos_conf_init(&policer->qos_conf, &egress_policer_ops); >>>>>>> egress_policer_details_to_param(details, >>>>>>> &policer->app_srtcm_params); >>>>>>> + err = rte_meter_srtcm_profile_config(&policer->egress_prof, >>>>>>> + >>>>>>> &policer->app_srtcm_params); >>>>>>> + if (err) { >>>>>> [Sugesh] Should we error out the message on failing the profile >>>>>> config?? >>>>>>> + free(policer); >>>>>>> + *conf = NULL; >>>>>>> + return -err; >>>>>>> + } >>>>>>> err = rte_meter_srtcm_config(&policer->egress_meter, >>>>>>> - &policer->app_srtcm_params); >>>>>>> + &policer->egress_prof); >>>>>>> + >>>>>>> if (!err) { >>>>>>> *conf = &policer->qos_conf; >>>>>>> } else { >>>>>>> @@ -3803,7 +3825,8 @@ egress_policer_run(struct qos_conf *conf, >>>>>>> struct rte_mbuf **pkts, int pkt_cnt, >>>>>>> struct egress_policer *policer = >>>>>>> CONTAINER_OF(conf, struct egress_policer, qos_conf); >>>>>>> >>>>>>> - cnt = netdev_dpdk_policer_run(&policer->egress_meter, pkts, >>>>>>> + cnt = netdev_dpdk_policer_run(&policer->egress_meter, >>>>>>> + &policer->egress_prof, pkts, >>>>>>> pkt_cnt, should_steal); >>>>>>> >>>>>>> return cnt; >>>>>>> @@ -4103,15 +4126,15 @@ dump_flow_pattern(struct >> rte_flow_item >>>>> *item) >>>>>>> >>>>>>> VLOG_DBG("rte flow vlan pattern:\n"); >>>>>>> if (vlan_spec) { >>>>>>> - VLOG_DBG(" Spec: tpid=0x%"PRIx16", >>>>>>> tci=0x%"PRIx16"\n", >>>>>>> - ntohs(vlan_spec->tpid), >>>>>>> ntohs(vlan_spec->tci)); >>>>>>> + VLOG_DBG(" Spec: inner_type=0x%"PRIx16", >>>>>>> tci=0x%"PRIx16"\n", >>>>>>> + ntohs(vlan_spec->inner_type), >>>>>>> + ntohs(vlan_spec->tci)); >>>>>>> } else { >>>>>>> VLOG_DBG(" Spec = null\n"); >>>>>>> } >>>>>>> >>>>>>> if (vlan_mask) { >>>>>>> - VLOG_DBG(" Mask: tpid=0x%"PRIx16", >>>>>>> tci=0x%"PRIx16"\n", >>>>>>> - vlan_mask->tpid, vlan_mask->tci); >>>>>>> + VLOG_DBG(" Mask: inner_type=0x%"PRIx16", >>>>> tci=0x%"PRIx16"\n", >>>>>>> + vlan_mask->inner_type, vlan_mask->tci); >>>>>>> } else { >>>>>>> VLOG_DBG(" Mask = null\n"); >>>>>>> } >>>>>>> @@ -4281,27 +4304,56 @@ add_flow_action(struct flow_actions >>>>> *actions, >>>>>>> enum rte_flow_action_type type, >>>>>>> actions->cnt++; >>>>>>> } >>>>>>> >>>>>>> +/* >>>>>>> + * Storage for struct rte_flow_action_rss >>>>>>> + * including storage for key and queue array */ #define >>>>>>> +MAX_RSS_HASH_KEY_LENGTH 128 #define >>>>> MAX_ACTION_RSS_QUEUE_NUM >>>>>>> 128 >>>>>>> +struct action_rss_data { >>>>>>> + struct rte_flow_action_rss conf; >>>>>>> + uint16_t queue[MAX_ACTION_RSS_QUEUE_NUM]; >>>>>>> + uint8_t key[MAX_RSS_HASH_KEY_LENGTH]; }; >>>>>>> + >>>>>>> static struct rte_flow_action_rss * add_flow_rss_action(struct >>>>>>> flow_actions *actions, >>>>>>> struct netdev *netdev) { >>>>>>> int i; >>>>>>> - struct rte_flow_action_rss *rss; >>>>>>> + struct action_rss_data *rss_data; >>>>>>> >>>>>>> - rss = xmalloc(sizeof(*rss) + sizeof(uint16_t) * >>>>>>> netdev->n_rxq); >>>>>>> - /* >>>>>>> - * Setting it to NULL will let the driver use the default RSS >>>>>>> - * configuration we have set: &port_conf.rx_adv_conf.rss_conf. >>>>>>> - */ >>>>>>> - rss->rss_conf = NULL; >>>>>>> - rss->num = netdev->n_rxq; >>>>>>> + if (netdev->n_rxq > MAX_ACTION_RSS_QUEUE_NUM) { >>>>>>> + VLOG_ERR("Num of rxq (%u) must not be greater " \ >>>>>>> + "than max rss num of queues (%u)", >>>>>>> + netdev->n_rxq, MAX_ACTION_RSS_QUEUE_NUM); >>>>>>> + return NULL; >>>>>>> + } >>>>>>> >>>>>>> - for (i = 0; i < rss->num; i++) { >>>>>>> - rss->queue[i] = i; >>>>>>> + rss_data = xmalloc(sizeof(struct action_rss_data)); >>>>>>> + *rss_data = (struct action_rss_data){ >>>>>>> + .conf = (struct rte_flow_action_rss){ >>>>>>> + .func = RTE_ETH_HASH_FUNCTION_DEFAULT, >>>>>>> + .level = 0, >>>>>>> + .types = ETH_RSS_IP, >>>>>>> + .key_len = 0, >>>>>>> + .queue_num = netdev->n_rxq, >>>>>>> + .queue = rss_data->queue, >>>>>>> + .key = rss_data->key, >>>>>>> + }, >>>>>>> + .key = { 0 }, >>>>>>> + .queue = { 0 }, >>>>>>> + }; >>>>>>> + >>>>>>> + /* TODO: Override key with default */ >>>>>> [Sugesh] Do we really need this TODO?? >>>>>>> + >>>>>>> + /* Override queue array with default */ >>>>>>> + for (i = 0; i < rss_data->conf.queue_num; i++) { >>>>>>> + rss_data->queue[i] = i; >>>>>>> } >>>>>>> >>>>>>> - add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, rss); >>>>>>> + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, >>>>>>> + &rss_data->conf); >>>>>>> >>>>>>> - return rss; >>>>>>> + return &rss_data->conf; >>>>>>> } >>>>>>> >>>>>>> static int >>>>>>> @@ -4365,7 +4417,7 @@ netdev_dpdk_add_rte_flow_offload(struct >>>>> netdev >>>>>>> *netdev, >>>>>>> vlan_mask.tci = match->wc.masks.vlans[0].tci & >>>>>>> ~htons(VLAN_CFI); >>>>>>> >>>>>>> /* match any protocols */ >>>>>>> - vlan_mask.tpid = 0; >>>>>>> + vlan_mask.inner_type = 0; >>>>>>> >>>>>>> add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_VLAN, >>>>>>> &vlan_spec, &vlan_mask); @@ -4516,6 >>>>>>> +4568,11 @@ >>>>>>> end_proto_check: >>>>>>> >>>>>>> struct rte_flow_action_rss *rss; >>>>>>> rss = add_flow_rss_action(&actions, netdev); >>>>>>> + if (!rss) { >>>>>>> + VLOG_ERR("add_flow_rss_action error.\n"); >>>>>> I feel this error can be bit more verbose something like Failed to >>>>>> add >>>>> rss_flow+actions or something?? What do you think? >>>>>>> + ret = -1; >>>>>>> + goto out; >>>>>>> + } >>>>>>> add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, >> NULL); >>>>>>> >>>>>>> flow = rte_flow_create(dev->port_id, &flow_attr, >>>>>>> patterns.items, >>>>>>> -- >>>>>>> 1.8.3.1 >>>>>> >>>> >>>> _______________________________________________ >>>> dev mailing list >>>> dev@openvswitch.org >>>> >> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fm >>>> ail.openvswitch.org%2Fmailman%2Flistinfo%2Fovs- >> dev&data=02%7C01% >>>> >> 7Cophirmu%40mellanox.com%7C028cdfe3e1154f5cc99e08d6172dc231%7C >> a65297 >>>> >> 1c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636721882650055912&s >> data=Wx >>>> >> VUAkghNz3pgtAqJ%2FyH0tBkgnQcBaMBIx3RI1FM%2BG4%3D&reserve >> d=0
diff --git a/.travis/linux-build.sh b/.travis/linux-build.sh index 4b9fc4a..c60ac71 100755 --- a/.travis/linux-build.sh +++ b/.travis/linux-build.sh @@ -83,7 +83,7 @@ fi if [ "$DPDK" ]; then if [ -z "$DPDK_VER" ]; then - DPDK_VER="17.11.3" + DPDK_VER="18.08.0" fi install_dpdk $DPDK_VER if [ "$CC" = "clang" ]; then diff --git a/Documentation/faq/releases.rst b/Documentation/faq/releases.rst index 41d41e3..646ae09 100644 --- a/Documentation/faq/releases.rst +++ b/Documentation/faq/releases.rst @@ -168,7 +168,7 @@ Q: What DPDK version does each Open vSwitch release work with? 2.7.x 16.11.7 2.8.x 17.05.2 2.9.x 17.11.3 - 2.10.x 17.11.3 + 2.10.x 18.08.0 ============ ======= Q: Are all the DPDK releases that OVS versions work with maintained? diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index f91aa27..80d2af9 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -168,11 +168,7 @@ static const struct rte_eth_conf port_conf = { .rxmode = { .mq_mode = ETH_MQ_RX_RSS, .split_hdr_size = 0, - .header_split = 0, /* Header Split disabled */ - .hw_ip_checksum = 0, /* IP checksum offload disabled */ - .hw_vlan_filter = 0, /* VLAN filtering disabled */ - .jumbo_frame = 0, /* Jumbo Frame Support disabled */ - .hw_strip_crc = 0, + .offloads = 0, }, .rx_adv_conf = { .rss_conf = { @@ -364,6 +360,7 @@ struct dpdk_ring { struct ingress_policer { struct rte_meter_srtcm_params app_srtcm_params; struct rte_meter_srtcm in_policer; + struct rte_meter_srtcm_profile in_prof; rte_spinlock_t policer_lock; }; @@ -903,16 +900,17 @@ dpdk_eth_dev_port_config(struct netdev_dpdk *dev, int n_rxq, int n_txq) if (dev->mtu > ETHER_MTU) { rte_eth_dev_info_get(dev->port_id, &info); if (strncmp(info.driver_name, "net_nfp", 7)) { - conf.rxmode.enable_scatter = 1; + conf.rxmode.offloads |= DEV_RX_OFFLOAD_SCATTER; } } conf.intr_conf.lsc = dev->lsc_interrupt_mode; - conf.rxmode.hw_ip_checksum = (dev->hw_ol_features & - NETDEV_RX_CHECKSUM_OFFLOAD) != 0; + conf.rxmode.offloads |= ((dev->hw_ol_features & + NETDEV_RX_CHECKSUM_OFFLOAD) != 0) ? + DEV_RX_OFFLOAD_CHECKSUM : 0; if (dev->hw_ol_features & NETDEV_RX_HW_CRC_STRIP) { - conf.rxmode.hw_strip_crc = 1; + conf.rxmode.offloads |= DEV_RX_OFFLOAD_CRC_STRIP; } /* A device may report more queues than it makes available (this has @@ -1932,16 +1930,18 @@ netdev_dpdk_eth_tx_burst(struct netdev_dpdk *dev, int qid, static inline bool netdev_dpdk_policer_pkt_handle(struct rte_meter_srtcm *meter, + struct rte_meter_srtcm_profile *profile, struct rte_mbuf *pkt, uint64_t time) { uint32_t pkt_len = rte_pktmbuf_pkt_len(pkt) - sizeof(struct ether_hdr); - return rte_meter_srtcm_color_blind_check(meter, time, pkt_len) == + return rte_meter_srtcm_color_blind_check(meter, profile, time, pkt_len) == e_RTE_METER_GREEN; } static int netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, + struct rte_meter_srtcm_profile *profile, struct rte_mbuf **pkts, int pkt_cnt, bool should_steal) { @@ -1953,7 +1953,8 @@ netdev_dpdk_policer_run(struct rte_meter_srtcm *meter, for (i = 0; i < pkt_cnt; i++) { pkt = pkts[i]; /* Handle current packet */ - if (netdev_dpdk_policer_pkt_handle(meter, pkt, current_time)) { + if (netdev_dpdk_policer_pkt_handle(meter, profile, pkt, + current_time)) { if (cnt != i) { pkts[cnt] = pkt; } @@ -1975,8 +1976,8 @@ ingress_policer_run(struct ingress_policer *policer, struct rte_mbuf **pkts, int cnt = 0; rte_spinlock_lock(&policer->policer_lock); - cnt = netdev_dpdk_policer_run(&policer->in_policer, pkts, - pkt_cnt, should_steal); + cnt = netdev_dpdk_policer_run(&policer->in_policer, &policer->in_prof, + pkts, pkt_cnt, should_steal); rte_spinlock_unlock(&policer->policer_lock); return cnt; @@ -2767,8 +2768,15 @@ netdev_dpdk_policer_construct(uint32_t rate, uint32_t burst) policer->app_srtcm_params.cir = rate_bytes; policer->app_srtcm_params.cbs = burst_bytes; policer->app_srtcm_params.ebs = 0; + err = rte_meter_srtcm_profile_config(&policer->in_prof, + &policer->app_srtcm_params); + if (err) { + VLOG_ERR("Could not create rte meter profile for ingress policer"); + free(policer); + return NULL; + } err = rte_meter_srtcm_config(&policer->in_policer, - &policer->app_srtcm_params); + &policer->in_prof); if (err) { VLOG_ERR("Could not create rte meter for ingress policer"); free(policer); @@ -3043,13 +3051,18 @@ netdev_dpdk_get_status(const struct netdev *netdev, struct smap *args) smap_add_format(args, "if_descr", "%s %s", rte_version(), dev_info.driver_name); - if (dev_info.pci_dev) { - smap_add_format(args, "pci-vendor_id", "0x%x", - dev_info.pci_dev->id.vendor_id); - smap_add_format(args, "pci-device_id", "0x%x", - dev_info.pci_dev->id.device_id); + const struct rte_bus *bus; + const struct rte_pci_device *pci_dev; + bus = rte_bus_find_by_device(dev_info.device); + if (bus && !strcmp(bus->name, "pci")) { + pci_dev = RTE_DEV_TO_PCI(dev_info.device); + if (pci_dev) { + smap_add_format(args, "pci-vendor_id", "0x%u", + pci_dev->id.vendor_id); + smap_add_format(args, "pci-device_id", "0x%x", + pci_dev->id.device_id); + } } - return 0; } @@ -3727,6 +3740,7 @@ struct egress_policer { struct qos_conf qos_conf; struct rte_meter_srtcm_params app_srtcm_params; struct rte_meter_srtcm egress_meter; + struct rte_meter_srtcm_profile egress_prof; }; static void @@ -3749,8 +3763,16 @@ egress_policer_qos_construct(const struct smap *details, policer = xmalloc(sizeof *policer); qos_conf_init(&policer->qos_conf, &egress_policer_ops); egress_policer_details_to_param(details, &policer->app_srtcm_params); + err = rte_meter_srtcm_profile_config(&policer->egress_prof, + &policer->app_srtcm_params); + if (err) { + free(policer); + *conf = NULL; + return -err; + } err = rte_meter_srtcm_config(&policer->egress_meter, - &policer->app_srtcm_params); + &policer->egress_prof); + if (!err) { *conf = &policer->qos_conf; } else { @@ -3803,7 +3825,8 @@ egress_policer_run(struct qos_conf *conf, struct rte_mbuf **pkts, int pkt_cnt, struct egress_policer *policer = CONTAINER_OF(conf, struct egress_policer, qos_conf); - cnt = netdev_dpdk_policer_run(&policer->egress_meter, pkts, + cnt = netdev_dpdk_policer_run(&policer->egress_meter, + &policer->egress_prof, pkts, pkt_cnt, should_steal); return cnt; @@ -4103,15 +4126,15 @@ dump_flow_pattern(struct rte_flow_item *item) VLOG_DBG("rte flow vlan pattern:\n"); if (vlan_spec) { - VLOG_DBG(" Spec: tpid=0x%"PRIx16", tci=0x%"PRIx16"\n", - ntohs(vlan_spec->tpid), ntohs(vlan_spec->tci)); + VLOG_DBG(" Spec: inner_type=0x%"PRIx16", tci=0x%"PRIx16"\n", + ntohs(vlan_spec->inner_type), ntohs(vlan_spec->tci)); } else { VLOG_DBG(" Spec = null\n"); } if (vlan_mask) { - VLOG_DBG(" Mask: tpid=0x%"PRIx16", tci=0x%"PRIx16"\n", - vlan_mask->tpid, vlan_mask->tci); + VLOG_DBG(" Mask: inner_type=0x%"PRIx16", tci=0x%"PRIx16"\n", + vlan_mask->inner_type, vlan_mask->tci); } else { VLOG_DBG(" Mask = null\n"); } @@ -4281,27 +4304,56 @@ add_flow_action(struct flow_actions *actions, enum rte_flow_action_type type, actions->cnt++; } +/* + * Storage for struct rte_flow_action_rss + * including storage for key and queue array + */ +#define MAX_RSS_HASH_KEY_LENGTH 128 +#define MAX_ACTION_RSS_QUEUE_NUM 128 +struct action_rss_data { + struct rte_flow_action_rss conf; + uint16_t queue[MAX_ACTION_RSS_QUEUE_NUM]; + uint8_t key[MAX_RSS_HASH_KEY_LENGTH]; +}; + static struct rte_flow_action_rss * add_flow_rss_action(struct flow_actions *actions, struct netdev *netdev) { int i; - struct rte_flow_action_rss *rss; + struct action_rss_data *rss_data; - rss = xmalloc(sizeof(*rss) + sizeof(uint16_t) * netdev->n_rxq); - /* - * Setting it to NULL will let the driver use the default RSS - * configuration we have set: &port_conf.rx_adv_conf.rss_conf. - */ - rss->rss_conf = NULL; - rss->num = netdev->n_rxq; + if (netdev->n_rxq > MAX_ACTION_RSS_QUEUE_NUM) { + VLOG_ERR("Num of rxq (%u) must not be greater " \ + "than max rss num of queues (%u)", + netdev->n_rxq, MAX_ACTION_RSS_QUEUE_NUM); + return NULL; + } - for (i = 0; i < rss->num; i++) { - rss->queue[i] = i; + rss_data = xmalloc(sizeof(struct action_rss_data)); + *rss_data = (struct action_rss_data){ + .conf = (struct rte_flow_action_rss){ + .func = RTE_ETH_HASH_FUNCTION_DEFAULT, + .level = 0, + .types = ETH_RSS_IP, + .key_len = 0, + .queue_num = netdev->n_rxq, + .queue = rss_data->queue, + .key = rss_data->key, + }, + .key = { 0 }, + .queue = { 0 }, + }; + + /* TODO: Override key with default */ + + /* Override queue array with default */ + for (i = 0; i < rss_data->conf.queue_num; i++) { + rss_data->queue[i] = i; } - add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, rss); + add_flow_action(actions, RTE_FLOW_ACTION_TYPE_RSS, &rss_data->conf); - return rss; + return &rss_data->conf; } static int @@ -4365,7 +4417,7 @@ netdev_dpdk_add_rte_flow_offload(struct netdev *netdev, vlan_mask.tci = match->wc.masks.vlans[0].tci & ~htons(VLAN_CFI); /* match any protocols */ - vlan_mask.tpid = 0; + vlan_mask.inner_type = 0; add_flow_pattern(&patterns, RTE_FLOW_ITEM_TYPE_VLAN, &vlan_spec, &vlan_mask); @@ -4516,6 +4568,11 @@ end_proto_check: struct rte_flow_action_rss *rss; rss = add_flow_rss_action(&actions, netdev); + if (!rss) { + VLOG_ERR("add_flow_rss_action error.\n"); + ret = -1; + goto out; + } add_flow_action(&actions, RTE_FLOW_ACTION_TYPE_END, NULL); flow = rte_flow_create(dev->port_id, &flow_attr, patterns.items,
1. Enable compilation and linkage with dpdk 18.08.0 The following dpdk commits which were introduced after dpdk 17.11.x require OVS updates to accommodate to the dpdk changes. - ce17eddefc20 ("ethdev: introduce Rx queue offloads API") - ab3ce1e0c193 ("ethdev: remove old offload API") - c06ddf9698e0 ("meter: add configuration profile") - e58638c32411 ("ethdev: fix TPID handling in flow API") - cd8c7c7ce241 ("ethdev: replace bus specific struct with generic dev") - ac8d22de2394 ("ethdev: flatten RSS configuration in flow API") 2. Update references to DPDK version in Documentation 3. Update DPDK version in travis' linux-build script Signed-off-by: Ophir Munk <ophirmu@mellanox.com> --- .travis/linux-build.sh | 2 +- Documentation/faq/releases.rst | 2 +- lib/netdev-dpdk.c | 135 +++++++++++++++++++++++++++++------------ 3 files changed, 98 insertions(+), 41 deletions(-)