| Message ID | 1462368501-9820-1-git-send-email-przemyslawx.lal@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Przemek, Apologies for the churn, but some additional points occurred to me - please refer to comments in netdev_dpdk_get_ifindex. Thanks, Mark > >This patch adds sFlow support for DPDK vHost-user interfaces by assigning >ifindex value. Values of ifindexes for vHost-user interfaces start with 2000 >to avoid overlapping with kernel datapath interfaces. > >Patch also fixes issue with 'dpdk0' interface being ignored by sFlow agent, >because of ifindex 0. Ifindexes values for physical DPDK interfaces start >with 1000 to avoid overlapping with kernel datapath interfaces. > >Signed-off-by: Przemyslaw Lal <przemyslawx.lal@intel.com> >--- > lib/netdev-dpdk.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 66 insertions(+), 1 deletion(-) > >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >index 208c5f5..568bce6 100644 >--- a/lib/netdev-dpdk.c >+++ b/lib/netdev-dpdk.c >@@ -115,6 +115,18 @@ static char *vhost_sock_dir = NULL; /* Location of vhost-user sockets >*/ > */ > #define VHOST_ENQ_RETRY_USECS 100 > >+/* For DPDK ETH interfaces use ifindex values starting with 1000 >+ * to avoid overlapping with kernel-space interfaces. >+ * Also starting with 0 would cause sFlow to ignore 'dpdk0' interface. >+ */ >+#define DPDK_PORT_ID_TO_IFINDEX(port_id) ((port_id) + 1000) >+ >+/* For DPDK vhost-user interfaces use ifindexes starting with 2000. >+ */ >+#define VHOST_ID_TO_IFINDEX(port_id) ((port_id) + 2000) >+ >+#define VHOST_IDS_MAX_LEN 1024 >+ > static const struct rte_eth_conf port_conf = { > .rxmode = { > .mq_mode = ETH_MQ_RX_RSS, >@@ -149,6 +161,7 @@ enum dpdk_dev_type { > static int rte_eal_init_ret = ENODEV; > > static struct ovs_mutex dpdk_mutex = OVS_MUTEX_INITIALIZER; >+static struct ovs_mutex vhost_mutex = OVS_MUTEX_INITIALIZER; > > /* Quality of Service */ > >@@ -790,6 +803,51 @@ netdev_dpdk_vhost_cuse_construct(struct netdev *netdev) > return err; > } > >+/* Counter for VHOST interfaces as DPDK library doesn't provide mechanism >+ * similar to rte_eth_dev_count for vhost-user sockets. >+ */ >+static int vhost_counter OVS_GUARDED_BY(vhost_mutex) = 0; >+ >+/* Array storing vhost_ids, so their ifindex can be reused after socket >+ * recreation. >+ */ >+static char vhost_ids[VHOST_IDS_MAX_LEN][PATH_MAX] OVS_GUARDED_BY(vhost_mutex); >+ >+/* Simple lookup in vhost_ids array. >+ * On success returns id of vhost_id stored in the array, >+ * otherwise returns -1. >+ */ >+static int >+netdev_dpdk_lookup_vhost_id(struct netdev_dpdk *dev) OVS_REQUIRES(vhost_mutex) >+{ >+ for (int i = 0; i < vhost_counter; i++) { >+ if (strncmp(vhost_ids[i], dev->vhost_id, strlen(dev->vhost_id)) == 0) { >+ return i; >+ } >+ } >+ return -1; >+} >+ >+/* Inserts vhost_id at the first free position in the vhost_ids array. >+ */ >+static void >+netdev_dpdk_push_vhost_id(struct netdev_dpdk *dev) >+{ >+ ovs_mutex_lock(&vhost_mutex); >+ if (netdev_dpdk_lookup_vhost_id(dev) < 0) { >+ if (vhost_counter < VHOST_IDS_MAX_LEN) { >+ strncpy(vhost_ids[vhost_counter++], dev->vhost_id, >+ strlen(dev->vhost_id)); >+ } >+ else { >+ VLOG_WARN("Could not assign ifindex to \"%s\" port. " >+ "List of vhost IDs list is full.", >+ dev->vhost_id); >+ } >+ } >+ ovs_mutex_unlock(&vhost_mutex); >+} >+ > static int > netdev_dpdk_vhost_user_construct(struct netdev *netdev) > { >@@ -825,6 +883,8 @@ netdev_dpdk_vhost_user_construct(struct netdev *netdev) > err = vhost_construct_helper(netdev); > } > >+ netdev_dpdk_push_vhost_id(dev); >+ > ovs_mutex_unlock(&dpdk_mutex); > return err; > } >@@ -1775,7 +1835,12 @@ netdev_dpdk_get_ifindex(const struct netdev *netdev) > int ifindex; > > ovs_mutex_lock(&dev->mutex); >- ifindex = dev->port_id; >+ if (dev->type == DPDK_DEV_ETH) { >+ ifindex = DPDK_PORT_ID_TO_IFINDEX(dev->port_id); >+ } <micro-nit> use BSD-style braces for the 'else' case </micro-nit> >+ else { This case applies to both vHost User and vHost Cuse ports - is there an issue in the latter case? >+ ifindex = VHOST_ID_TO_IFINDEX(netdev_dpdk_lookup_vhost_id(dev)); Potential bug here if netdev_dpdk_lookup_vhost_id returns -1. >+ } > ovs_mutex_unlock(&dev->mutex); > > return ifindex; >-- >2.1.0
Hi Mark, Good points, I corrected all of them (hopefully) and resubmitted the patch. Thanks, Przemek -----Original Message----- From: Kavanagh, Mark B Sent: Wednesday, May 4, 2016 4:43 PM To: Lal, PrzemyslawX <przemyslawx.lal@intel.com>; dev@openvswitch.org Subject: RE: [PATCH] netdev-dpdk: add sflow support for vhost-user ports Hi Przemek, Apologies for the churn, but some additional points occurred to me - please refer to comments in netdev_dpdk_get_ifindex. Thanks, Mark > >This patch adds sFlow support for DPDK vHost-user interfaces by >assigning ifindex value. Values of ifindexes for vHost-user interfaces >start with 2000 to avoid overlapping with kernel datapath interfaces. > >Patch also fixes issue with 'dpdk0' interface being ignored by sFlow >agent, because of ifindex 0. Ifindexes values for physical DPDK >interfaces start with 1000 to avoid overlapping with kernel datapath interfaces. > >Signed-off-by: Przemyslaw Lal <przemyslawx.lal@intel.com> >--- > lib/netdev-dpdk.c | 67 >++++++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 66 insertions(+), 1 deletion(-) > >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index >208c5f5..568bce6 100644 >--- a/lib/netdev-dpdk.c >+++ b/lib/netdev-dpdk.c >@@ -115,6 +115,18 @@ static char *vhost_sock_dir = NULL; /* Location of vhost-user sockets >*/ > */ > #define VHOST_ENQ_RETRY_USECS 100 > >+/* For DPDK ETH interfaces use ifindex values starting with 1000 >+ * to avoid overlapping with kernel-space interfaces. >+ * Also starting with 0 would cause sFlow to ignore 'dpdk0' interface. >+ */ >+#define DPDK_PORT_ID_TO_IFINDEX(port_id) ((port_id) + 1000) >+ >+/* For DPDK vhost-user interfaces use ifindexes starting with 2000. >+ */ >+#define VHOST_ID_TO_IFINDEX(port_id) ((port_id) + 2000) >+ >+#define VHOST_IDS_MAX_LEN 1024 >+ > static const struct rte_eth_conf port_conf = { > .rxmode = { > .mq_mode = ETH_MQ_RX_RSS, >@@ -149,6 +161,7 @@ enum dpdk_dev_type { static int rte_eal_init_ret = >ENODEV; > > static struct ovs_mutex dpdk_mutex = OVS_MUTEX_INITIALIZER; >+static struct ovs_mutex vhost_mutex = OVS_MUTEX_INITIALIZER; > > /* Quality of Service */ > >@@ -790,6 +803,51 @@ netdev_dpdk_vhost_cuse_construct(struct netdev *netdev) > return err; > } > >+/* Counter for VHOST interfaces as DPDK library doesn't provide >+mechanism >+ * similar to rte_eth_dev_count for vhost-user sockets. >+ */ >+static int vhost_counter OVS_GUARDED_BY(vhost_mutex) = 0; >+ >+/* Array storing vhost_ids, so their ifindex can be reused after >+socket >+ * recreation. >+ */ >+static char vhost_ids[VHOST_IDS_MAX_LEN][PATH_MAX] >+OVS_GUARDED_BY(vhost_mutex); >+ >+/* Simple lookup in vhost_ids array. >+ * On success returns id of vhost_id stored in the array, >+ * otherwise returns -1. >+ */ >+static int >+netdev_dpdk_lookup_vhost_id(struct netdev_dpdk *dev) >+OVS_REQUIRES(vhost_mutex) { >+ for (int i = 0; i < vhost_counter; i++) { >+ if (strncmp(vhost_ids[i], dev->vhost_id, strlen(dev->vhost_id)) == 0) { >+ return i; >+ } >+ } >+ return -1; >+} >+ >+/* Inserts vhost_id at the first free position in the vhost_ids array. >+ */ >+static void >+netdev_dpdk_push_vhost_id(struct netdev_dpdk *dev) { >+ ovs_mutex_lock(&vhost_mutex); >+ if (netdev_dpdk_lookup_vhost_id(dev) < 0) { >+ if (vhost_counter < VHOST_IDS_MAX_LEN) { >+ strncpy(vhost_ids[vhost_counter++], dev->vhost_id, >+ strlen(dev->vhost_id)); >+ } >+ else { >+ VLOG_WARN("Could not assign ifindex to \"%s\" port. " >+ "List of vhost IDs list is full.", >+ dev->vhost_id); >+ } >+ } >+ ovs_mutex_unlock(&vhost_mutex); >+} >+ > static int > netdev_dpdk_vhost_user_construct(struct netdev *netdev) { @@ -825,6 >+883,8 @@ netdev_dpdk_vhost_user_construct(struct netdev *netdev) > err = vhost_construct_helper(netdev); > } > >+ netdev_dpdk_push_vhost_id(dev); >+ > ovs_mutex_unlock(&dpdk_mutex); > return err; > } >@@ -1775,7 +1835,12 @@ netdev_dpdk_get_ifindex(const struct netdev *netdev) > int ifindex; > > ovs_mutex_lock(&dev->mutex); >- ifindex = dev->port_id; >+ if (dev->type == DPDK_DEV_ETH) { >+ ifindex = DPDK_PORT_ID_TO_IFINDEX(dev->port_id); >+ } <micro-nit> use BSD-style braces for the 'else' case </micro-nit> >+ else { This case applies to both vHost User and vHost Cuse ports - is there an issue in the latter case? >+ ifindex = >+ VHOST_ID_TO_IFINDEX(netdev_dpdk_lookup_vhost_id(dev)); Potential bug here if netdev_dpdk_lookup_vhost_id returns -1. >+ } > ovs_mutex_unlock(&dev->mutex); > > return ifindex; >-- >2.1.0
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 208c5f5..568bce6 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -115,6 +115,18 @@ static char *vhost_sock_dir = NULL; /* Location of vhost-user sockets */ */ #define VHOST_ENQ_RETRY_USECS 100 +/* For DPDK ETH interfaces use ifindex values starting with 1000 + * to avoid overlapping with kernel-space interfaces. + * Also starting with 0 would cause sFlow to ignore 'dpdk0' interface. + */ +#define DPDK_PORT_ID_TO_IFINDEX(port_id) ((port_id) + 1000) + +/* For DPDK vhost-user interfaces use ifindexes starting with 2000. + */ +#define VHOST_ID_TO_IFINDEX(port_id) ((port_id) + 2000) + +#define VHOST_IDS_MAX_LEN 1024 + static const struct rte_eth_conf port_conf = { .rxmode = { .mq_mode = ETH_MQ_RX_RSS, @@ -149,6 +161,7 @@ enum dpdk_dev_type { static int rte_eal_init_ret = ENODEV; static struct ovs_mutex dpdk_mutex = OVS_MUTEX_INITIALIZER; +static struct ovs_mutex vhost_mutex = OVS_MUTEX_INITIALIZER; /* Quality of Service */ @@ -790,6 +803,51 @@ netdev_dpdk_vhost_cuse_construct(struct netdev *netdev) return err; } +/* Counter for VHOST interfaces as DPDK library doesn't provide mechanism + * similar to rte_eth_dev_count for vhost-user sockets. + */ +static int vhost_counter OVS_GUARDED_BY(vhost_mutex) = 0; + +/* Array storing vhost_ids, so their ifindex can be reused after socket + * recreation. + */ +static char vhost_ids[VHOST_IDS_MAX_LEN][PATH_MAX] OVS_GUARDED_BY(vhost_mutex); + +/* Simple lookup in vhost_ids array. + * On success returns id of vhost_id stored in the array, + * otherwise returns -1. + */ +static int +netdev_dpdk_lookup_vhost_id(struct netdev_dpdk *dev) OVS_REQUIRES(vhost_mutex) +{ + for (int i = 0; i < vhost_counter; i++) { + if (strncmp(vhost_ids[i], dev->vhost_id, strlen(dev->vhost_id)) == 0) { + return i; + } + } + return -1; +} + +/* Inserts vhost_id at the first free position in the vhost_ids array. + */ +static void +netdev_dpdk_push_vhost_id(struct netdev_dpdk *dev) +{ + ovs_mutex_lock(&vhost_mutex); + if (netdev_dpdk_lookup_vhost_id(dev) < 0) { + if (vhost_counter < VHOST_IDS_MAX_LEN) { + strncpy(vhost_ids[vhost_counter++], dev->vhost_id, + strlen(dev->vhost_id)); + } + else { + VLOG_WARN("Could not assign ifindex to \"%s\" port. " + "List of vhost IDs list is full.", + dev->vhost_id); + } + } + ovs_mutex_unlock(&vhost_mutex); +} + static int netdev_dpdk_vhost_user_construct(struct netdev *netdev) { @@ -825,6 +883,8 @@ netdev_dpdk_vhost_user_construct(struct netdev *netdev) err = vhost_construct_helper(netdev); } + netdev_dpdk_push_vhost_id(dev); + ovs_mutex_unlock(&dpdk_mutex); return err; } @@ -1775,7 +1835,12 @@ netdev_dpdk_get_ifindex(const struct netdev *netdev) int ifindex; ovs_mutex_lock(&dev->mutex); - ifindex = dev->port_id; + if (dev->type == DPDK_DEV_ETH) { + ifindex = DPDK_PORT_ID_TO_IFINDEX(dev->port_id); + } + else { + ifindex = VHOST_ID_TO_IFINDEX(netdev_dpdk_lookup_vhost_id(dev)); + } ovs_mutex_unlock(&dev->mutex); return ifindex;
This patch adds sFlow support for DPDK vHost-user interfaces by assigning ifindex value. Values of ifindexes for vHost-user interfaces start with 2000 to avoid overlapping with kernel datapath interfaces. Patch also fixes issue with 'dpdk0' interface being ignored by sFlow agent, because of ifindex 0. Ifindexes values for physical DPDK interfaces start with 1000 to avoid overlapping with kernel datapath interfaces. Signed-off-by: Przemyslaw Lal <przemyslawx.lal@intel.com> --- lib/netdev-dpdk.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-)