| Message ID | 1461959837-8164-1-git-send-email-przemyslawx.lal@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
Hi Przemek, A few review comments inline. Also, the patch/commit name suggests that sFlow support is only added for vhost-user ports, but it's also added for dpdk_eth ports, right? Cheers, 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 | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 58 insertions(+), 1 deletion(-) > >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c >index 208c5f5..707e1d2 100644 >--- a/lib/netdev-dpdk.c >+++ b/lib/netdev-dpdk.c >@@ -115,6 +115,16 @@ 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) >+ > static const struct rte_eth_conf port_conf = { > .rxmode = { > .mq_mode = ETH_MQ_RX_RSS, >@@ -149,6 +159,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 +801,45 @@ 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[1024][PATH_MAX] OVS_GUARDED_BY(vhost_mutex); Consider using a macro for the length of 'vhost_ids', e.g. VHOST_IDS_MAX_LEN - see a later comment on how this could be used. >+ >+/* 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) { You should perform a bounds check on 'vhost_counter' to ensure that it doesn't stray past the end of 'vhost_ids'. e.g. if vhost_counter < VHOST_IDS_MAX_LEN, then perform the strncpy, otherwise warn the user that the list is full. >+ strncpy(vhost_ids[vhost_counter++], dev->vhost_id, >+ strlen(dev->vhost_id)); >+ } >+ ovs_mutex_unlock(&vhost_mutex); >+} >+ >+ Remove the extraneous blank line, above. > static int > netdev_dpdk_vhost_user_construct(struct netdev *netdev) > { >@@ -825,6 +875,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 +1827,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; >-- >2.1.0 > >_______________________________________________ >dev mailing list >dev@openvswitch.org >http://openvswitch.org/mailman/listinfo/dev
Hi Mark, It worked for dpdk_eth ports before the patch, only "dpdk0" interface was missing from the stats because it had ifindex 0 assigned. Should I change the subject before resubmitting the patch with changes suggested by you to a more generic one, for instance "netdev-dpdk: add sflow support for dpdk ports"? Thanks, Przemek -----Original Message----- From: Kavanagh, Mark B Sent: Wednesday, May 4, 2016 12:23 PM To: Lal, PrzemyslawX <przemyslawx.lal@intel.com>; dev@openvswitch.org Subject: RE: [ovs-dev] [PATCH] netdev-dpdk: add sflow support for vhost-user ports Hi Przemek, A few review comments inline. Also, the patch/commit name suggests that sFlow support is only added for vhost-user ports, but it's also added for dpdk_eth ports, right? Cheers, 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 | 59 >++++++++++++++++++++++++++++++++++++++++++++++++++++++- > 1 file changed, 58 insertions(+), 1 deletion(-) > >diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index >208c5f5..707e1d2 100644 >--- a/lib/netdev-dpdk.c >+++ b/lib/netdev-dpdk.c >@@ -115,6 +115,16 @@ 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) >+ > static const struct rte_eth_conf port_conf = { > .rxmode = { > .mq_mode = ETH_MQ_RX_RSS, >@@ -149,6 +159,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 +801,45 @@ 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[1024][PATH_MAX] OVS_GUARDED_BY(vhost_mutex); Consider using a macro for the length of 'vhost_ids', e.g. VHOST_IDS_MAX_LEN - see a later comment on how this could be used. >+ >+/* 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) { You should perform a bounds check on 'vhost_counter' to ensure that it doesn't stray past the end of 'vhost_ids'. e.g. if vhost_counter < VHOST_IDS_MAX_LEN, then perform the strncpy, otherwise warn the user that the list is full. >+ strncpy(vhost_ids[vhost_counter++], dev->vhost_id, >+ strlen(dev->vhost_id)); >+ } >+ ovs_mutex_unlock(&vhost_mutex); >+} >+ >+ Remove the extraneous blank line, above. > static int > netdev_dpdk_vhost_user_construct(struct netdev *netdev) { @@ -825,6 >+875,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 +1827,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; >-- >2.1.0 > >_______________________________________________ >dev mailing list >dev@openvswitch.org >http://openvswitch.org/mailman/listinfo/dev
Hi Przemek, You're right, dpdk eth ports were supported, but there was a possibility that their indexes could overlap with kernel interfaces, right? Either way, on reflection, this isn't a major issue; I wouldn't change the wording to 'add support for dpdk ports', as vhost cuse ports aren't supported - if I've missed something though, please let me know. Thanks, Mark > >Hi Mark, > >It worked for dpdk_eth ports before the patch, only "dpdk0" interface was missing from the >stats because it had ifindex 0 assigned. Should I change the subject before resubmitting the >patch with changes suggested by you to a more generic one, for instance "netdev-dpdk: add >sflow support for dpdk ports"? > >Thanks, >Przemek > >-----Original Message----- >From: Kavanagh, Mark B >Sent: Wednesday, May 4, 2016 12:23 PM >To: Lal, PrzemyslawX <przemyslawx.lal@intel.com>; dev@openvswitch.org >Subject: RE: [ovs-dev] [PATCH] netdev-dpdk: add sflow support for vhost-user ports > >Hi Przemek, > >A few review comments inline. > >Also, the patch/commit name suggests that sFlow support is only added for vhost-user ports, >but it's also added for dpdk_eth ports, right? > >Cheers, >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 | 59 >>++++++++++++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 58 insertions(+), 1 deletion(-) >> >>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index >>208c5f5..707e1d2 100644 >>--- a/lib/netdev-dpdk.c >>+++ b/lib/netdev-dpdk.c >>@@ -115,6 +115,16 @@ 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) >>+ >> static const struct rte_eth_conf port_conf = { >> .rxmode = { >> .mq_mode = ETH_MQ_RX_RSS, >>@@ -149,6 +159,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 +801,45 @@ 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[1024][PATH_MAX] OVS_GUARDED_BY(vhost_mutex); > >Consider using a macro for the length of 'vhost_ids', e.g. VHOST_IDS_MAX_LEN - see a later >comment on how this could be used. > >>+ >>+/* 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) { > >You should perform a bounds check on 'vhost_counter' to ensure that it doesn't stray past the >end of 'vhost_ids'. >e.g. if vhost_counter < VHOST_IDS_MAX_LEN, then perform the strncpy, otherwise warn the user >that the list is full. > >>+ strncpy(vhost_ids[vhost_counter++], dev->vhost_id, >>+ strlen(dev->vhost_id)); >>+ } >>+ ovs_mutex_unlock(&vhost_mutex); >>+} >>+ >>+ > >Remove the extraneous blank line, above. > >> static int >> netdev_dpdk_vhost_user_construct(struct netdev *netdev) { @@ -825,6 >>+875,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 +1827,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; >>-- >>2.1.0 >> >>_______________________________________________ >>dev mailing list >>dev@openvswitch.org >>http://openvswitch.org/mailman/listinfo/dev
Thanks Mark, Ok, so it looks like we are on the same page now. I'll resubmit the patch with minor fixes to issues mentioned by you without changing the subject. Best Regards, Przemek -----Original Message----- From: Kavanagh, Mark B Sent: Wednesday, May 4, 2016 2:15 PM To: Lal, PrzemyslawX <przemyslawx.lal@intel.com> Cc: dev@openvswitch.org Subject: RE: [ovs-dev] [PATCH] netdev-dpdk: add sflow support for vhost-user ports Hi Przemek, You're right, dpdk eth ports were supported, but there was a possibility that their indexes could overlap with kernel interfaces, right? Either way, on reflection, this isn't a major issue; I wouldn't change the wording to 'add support for dpdk ports', as vhost cuse ports aren't supported - if I've missed something though, please let me know. Thanks, Mark > >Hi Mark, > >It worked for dpdk_eth ports before the patch, only "dpdk0" interface >was missing from the stats because it had ifindex 0 assigned. Should I >change the subject before resubmitting the patch with changes suggested >by you to a more generic one, for instance "netdev-dpdk: add sflow support for dpdk ports"? > >Thanks, >Przemek > >-----Original Message----- >From: Kavanagh, Mark B >Sent: Wednesday, May 4, 2016 12:23 PM >To: Lal, PrzemyslawX <przemyslawx.lal@intel.com>; dev@openvswitch.org >Subject: RE: [ovs-dev] [PATCH] netdev-dpdk: add sflow support for >vhost-user ports > >Hi Przemek, > >A few review comments inline. > >Also, the patch/commit name suggests that sFlow support is only added >for vhost-user ports, but it's also added for dpdk_eth ports, right? > >Cheers, >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 | 59 >>++++++++++++++++++++++++++++++++++++++++++++++++++++++- >> 1 file changed, 58 insertions(+), 1 deletion(-) >> >>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index >>208c5f5..707e1d2 100644 >>--- a/lib/netdev-dpdk.c >>+++ b/lib/netdev-dpdk.c >>@@ -115,6 +115,16 @@ 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) >>+ >> static const struct rte_eth_conf port_conf = { >> .rxmode = { >> .mq_mode = ETH_MQ_RX_RSS, >>@@ -149,6 +159,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 +801,45 @@ 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[1024][PATH_MAX] OVS_GUARDED_BY(vhost_mutex); > >Consider using a macro for the length of 'vhost_ids', e.g. >VHOST_IDS_MAX_LEN - see a later comment on how this could be used. > >>+ >>+/* 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) { > >You should perform a bounds check on 'vhost_counter' to ensure that it >doesn't stray past the end of 'vhost_ids'. >e.g. if vhost_counter < VHOST_IDS_MAX_LEN, then perform the strncpy, >otherwise warn the user that the list is full. > >>+ strncpy(vhost_ids[vhost_counter++], dev->vhost_id, >>+ strlen(dev->vhost_id)); >>+ } >>+ ovs_mutex_unlock(&vhost_mutex); >>+} >>+ >>+ > >Remove the extraneous blank line, above. > >> static int >> netdev_dpdk_vhost_user_construct(struct netdev *netdev) { @@ -825,6 >>+875,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 +1827,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; >>-- >>2.1.0 >> >>_______________________________________________ >>dev mailing list >>dev@openvswitch.org >>http://openvswitch.org/mailman/listinfo/dev
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 208c5f5..707e1d2 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -115,6 +115,16 @@ 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) + static const struct rte_eth_conf port_conf = { .rxmode = { .mq_mode = ETH_MQ_RX_RSS, @@ -149,6 +159,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 +801,45 @@ 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[1024][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) { + strncpy(vhost_ids[vhost_counter++], dev->vhost_id, + strlen(dev->vhost_id)); + } + ovs_mutex_unlock(&vhost_mutex); +} + + static int netdev_dpdk_vhost_user_construct(struct netdev *netdev) { @@ -825,6 +875,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 +1827,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 | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-)