Re: [ovs-dev] [PATCH] netdev-dpdk: add sflow support for vhost-user ports
>
>Hi Mark,
>
>RFC 2863 standard describes in "Interface Numbering" chapter
>(https://tools.ietf.org/html/rfc2863#page-10) that if there's possibility to
>reuse ifindex
>after reinitialization then it should be reused.
[MK] Very interesting - thanks for the link.
And in our case it's definitely possible. I
>also agree with you that limitation to 1024 vhostuser interfaces is a major
>problem here and
>I should avoid such arbitrary values. I think the solution might be to use
>hashmap or OVS
>list implementation or maybe even dynamic allocation of new vhost ifindex
>entries.
[MK] Do you think that the issues relating to ifIndex/ifNumber are only
applicable to DPDK-based vhost and ring ports?
i.e. does OVS handle kernel-based ifIndexes gracefully currently, or is
this a larger issue that requires a more general solution?
>
>I have also analyzed current vhost-cuse initialization and adding support for
>ifindex
>assignment to this type of port should be a trivial task and it could be
>easily introduced in
>V2 of this patch.
[MK] Okay great - I look forward to it.
>
>Please share your thoughts.
>
>Thanks,
>Przemek
>
>-Original Message-
>From: Kavanagh, Mark B
>Sent: Wednesday, May 11, 2016 11:56 AM
>To: Lal, PrzemyslawX ; dev@openvswitch.org
>Subject: RE: [PATCH] netdev-dpdk: add sflow support for vhost-user ports
>
>>
>>Hi Mark,
>>
>>Replies inline prefixed with [PL].
>>
>>Thanks,
>>Przemek
>>
>>-Original Message-
>>From: Kavanagh, Mark B
>>Sent: Thursday, May 5, 2016 2:19 PM
>>To: Lal, PrzemyslawX ; dev@openvswitch.org
>>Subject: RE: [PATCH] netdev-dpdk: add sflow support for vhost-user
>>ports
>>
>>Hi Przemek,
>>
>>Some additional comments/queries inline.
>>
>>Thanks again,
>>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
>>>---
>>> lib/netdev-dpdk.c | 70
>>>++-
>>> 1 file changed, 69 insertions(+), 1 deletion(-)
>>>
>>>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
>>>208c5f5..1b60c5a 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,50 @@ 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\"
Re: [ovs-dev] [PATCH] netdev-dpdk: add sflow support for vhost-user ports
Reusing an ifIndex from time to time is no problem from the sFlow standard
point of view either. In fact we used a hash approach in the host-sflow
project to assign index numbers to containers (by open-hashing the
container UUID into a range of possible index numbers):
https://github.com/sflow/host-sflow/blob/master/src/Linux/hsflowd.c#L373
--
Neil McKee
InMon Corp.
http://www.inmon.com
On Thu, Jun 16, 2016 at 4:16 AM, Lal, PrzemyslawX wrote:
> Hi Mark,
>
> RFC 2863 standard describes in "Interface Numbering" chapter (
> https://tools.ietf.org/html/rfc2863#page-10) that if there's possibility
> to reuse ifindex after reinitialization then it should be reused. And in
> our case it's definitely possible. I also agree with you that limitation to
> 1024 vhostuser interfaces is a major problem here and I should avoid such
> arbitrary values. I think the solution might be to use hashmap or OVS list
> implementation or maybe even dynamic allocation of new vhost ifindex
> entries.
>
> I have also analyzed current vhost-cuse initialization and adding support
> for ifindex assignment to this type of port should be a trivial task and it
> could be easily introduced in V2 of this patch.
>
> Please share your thoughts.
>
> Thanks,
> Przemek
>
> -Original Message-
> From: Kavanagh, Mark B
> Sent: Wednesday, May 11, 2016 11:56 AM
> To: Lal, PrzemyslawX ; dev@openvswitch.org
> Subject: RE: [PATCH] netdev-dpdk: add sflow support for vhost-user ports
>
> >
> >Hi Mark,
> >
> >Replies inline prefixed with [PL].
> >
> >Thanks,
> >Przemek
> >
> >-Original Message-
> >From: Kavanagh, Mark B
> >Sent: Thursday, May 5, 2016 2:19 PM
> >To: Lal, PrzemyslawX ; dev@openvswitch.org
> >Subject: RE: [PATCH] netdev-dpdk: add sflow support for vhost-user
> >ports
> >
> >Hi Przemek,
> >
> >Some additional comments/queries inline.
> >
> >Thanks again,
> >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
> >>---
> >> lib/netdev-dpdk.c | 70
> >>++-
> >> 1 file changed, 69 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
> >>208c5f5..1b60c5a 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,50 @@ 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 < V
Re: [ovs-dev] [PATCH] netdev-dpdk: add sflow support for vhost-user ports
Hi Mark,
RFC 2863 standard describes in "Interface Numbering" chapter
(https://tools.ietf.org/html/rfc2863#page-10) that if there's possibility to
reuse ifindex after reinitialization then it should be reused. And in our case
it's definitely possible. I also agree with you that limitation to 1024
vhostuser interfaces is a major problem here and I should avoid such arbitrary
values. I think the solution might be to use hashmap or OVS list implementation
or maybe even dynamic allocation of new vhost ifindex entries.
I have also analyzed current vhost-cuse initialization and adding support for
ifindex assignment to this type of port should be a trivial task and it could
be easily introduced in V2 of this patch.
Please share your thoughts.
Thanks,
Przemek
-Original Message-
From: Kavanagh, Mark B
Sent: Wednesday, May 11, 2016 11:56 AM
To: Lal, PrzemyslawX ; dev@openvswitch.org
Subject: RE: [PATCH] netdev-dpdk: add sflow support for vhost-user ports
>
>Hi Mark,
>
>Replies inline prefixed with [PL].
>
>Thanks,
>Przemek
>
>-Original Message-
>From: Kavanagh, Mark B
>Sent: Thursday, May 5, 2016 2:19 PM
>To: Lal, PrzemyslawX ; dev@openvswitch.org
>Subject: RE: [PATCH] netdev-dpdk: add sflow support for vhost-user
>ports
>
>Hi Przemek,
>
>Some additional comments/queries inline.
>
>Thanks again,
>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
>>---
>> lib/netdev-dpdk.c | 70
>>++-
>> 1 file changed, 69 insertions(+), 1 deletion(-)
>>
>>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
>>208c5f5..1b60c5a 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,50 @@ 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
>>+882,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
Re: [ovs-dev] [PATCH] netdev-dpdk: add sflow support for vhost-user ports
>
>Hi Mark,
>
>Replies inline prefixed with [PL].
>
>Thanks,
>Przemek
>
>-Original Message-
>From: Kavanagh, Mark B
>Sent: Thursday, May 5, 2016 2:19 PM
>To: Lal, PrzemyslawX ; dev@openvswitch.org
>Subject: RE: [PATCH] netdev-dpdk: add sflow support for vhost-user ports
>
>Hi Przemek,
>
>Some additional comments/queries inline.
>
>Thanks again,
>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
>>---
>> lib/netdev-dpdk.c | 70
>>++-
>> 1 file changed, 69 insertions(+), 1 deletion(-)
>>
>>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
>>208c5f5..1b60c5a 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,50 @@ 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
>>+882,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;
>> }
>>@@ -1773,9 +1832,18 @@ netdev_dpdk_get_ifindex(const struct netdev
>>*netdev) {
>> struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
>> int ifindex;
>>+int ret;
>>
>> 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 {
>
>
>This 'else' statement is executed in both the case of vhost user devices and
>also vhost cuse
>devices.
>In the latter case, do the port numbers returned by VHOST_ID_IFINDEX make
>sense? (i.e. can
>sFlow 'talk' to vhost-cuse ports? If not, then this code has a bug).
>
>[PL] At this point for vhost cuse there's always '-1' returned from lookup
>function, so in
>the nested if statement you'll always hit 'else' and 0 will be returned as
>ifindex for vhost
>cuse devices.
Agreed. But then all vhost-cuse devices share index of 0 - this is surely
problematic?
>Anyway setting ifindexes for vhost cuse device
Re: [ovs-dev] [PATCH] netdev-dpdk: add sflow support for vhost-user ports
Hi Mark,
Replies inline prefixed with [PL].
Thanks,
Przemek
-Original Message-
From: Kavanagh, Mark B
Sent: Thursday, May 5, 2016 2:19 PM
To: Lal, PrzemyslawX ; dev@openvswitch.org
Subject: RE: [PATCH] netdev-dpdk: add sflow support for vhost-user ports
Hi Przemek,
Some additional comments/queries inline.
Thanks again,
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
>---
> lib/netdev-dpdk.c | 70
>++-
> 1 file changed, 69 insertions(+), 1 deletion(-)
>
>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index
>208c5f5..1b60c5a 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,50 @@ 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
>+882,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;
> }
>@@ -1773,9 +1832,18 @@ netdev_dpdk_get_ifindex(const struct netdev
>*netdev) {
> struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> int ifindex;
>+int ret;
>
> 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 {
This 'else' statement is executed in both the case of vhost user devices and
also vhost cuse devices.
In the latter case, do the port numbers returned by VHOST_ID_IFINDEX make
sense? (i.e. can sFlow 'talk' to vhost-cuse ports? If not, then this code has a
bug).
[PL] At this point for vhost cuse there's always '-1' returned from lookup
function, so in the nested if statement you'll always hit 'else' and 0 will be
returned as ifindex for vhost cuse devices.
Anyway setting ifindexes for vhost cuse devices should be a trivial task and if
vhost cuse is supported by sflow I can handle it - but then it will require
changing patch subject and message.
>+if ((ret = netdev_dpdk_lookup_vhost_id(dev)) >= 0) {
>+ifindex
Re: [ovs-dev] [PATCH] netdev-dpdk: add sflow support for vhost-user ports
>
>On Thu, May 05, 2016 at 12:18:52PM +, Kavanagh, Mark B wrote:
>> >+if ((ret = netdev_dpdk_lookup_vhost_id(dev)) >= 0) {
>> >+ifindex = VHOST_ID_TO_IFINDEX(ret);
>> >+} else {
>> >+ifindex = 0;
>>
>> I don't think that 0 is an appropriate value to return here, since it
>> overlaps with a kernel interface's index.
>
>0 is never a valid ifindex value, at least on a POSIX system, since it
>is the error return value from if_nametoindex(), see:
>
> http://pubs.opengroup.org/onlinepubs/009695399/functions/if_nametoindex.html
Good to know Ben - thanks for the link!
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] netdev-dpdk: add sflow support for vhost-user ports
On Thu, May 05, 2016 at 12:18:52PM +, Kavanagh, Mark B wrote:
> >+if ((ret = netdev_dpdk_lookup_vhost_id(dev)) >= 0) {
> >+ifindex = VHOST_ID_TO_IFINDEX(ret);
> >+} else {
> >+ifindex = 0;
>
> I don't think that 0 is an appropriate value to return here, since it
> overlaps with a kernel interface's index.
0 is never a valid ifindex value, at least on a POSIX system, since it
is the error return value from if_nametoindex(), see:
http://pubs.opengroup.org/onlinepubs/009695399/functions/if_nametoindex.html
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] netdev-dpdk: add sflow support for vhost-user ports
Hi Przemek,
Some additional comments/queries inline.
Thanks again,
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
>---
> lib/netdev-dpdk.c | 70 ++-
> 1 file changed, 69 insertions(+), 1 deletion(-)
>
>diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
>index 208c5f5..1b60c5a 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,50 @@ 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 +882,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;
> }
>@@ -1773,9 +1832,18 @@ netdev_dpdk_get_ifindex(const struct netdev *netdev)
> {
> struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
> int ifindex;
>+int ret;
>
> 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 {
This 'else' statement is executed in both the case of vhost user devices and
also vhost cuse devices.
In the latter case, do the port numbers returned by VHOST_ID_IFINDEX make
sense? (i.e. can sFlow 'talk' to vhost-cuse ports? If not, then this code has a
bug).
>+if ((ret = netdev_dpdk_lookup_vhost_id(dev)) >= 0) {
>+ifindex = VHOST_ID_TO_IFINDEX(ret);
>+} else {
>+ifindex = 0;
I don't think that 0 is an appropriate value to return here, since it overlaps
with a kernel interface's index.
>+}
>+}
> ovs_mutex_unlock(&dev->mutex);
>
> return ifindex;
>--
>2.1.0
[MK] There's a gap in this code - when the vhost-user device is destroyed, its
entry should be removed from 'vhost_ids'.
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] netdev-dpdk: add sflow support for vhost-user ports
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
---
lib/netdev-dpdk.c | 70 ++-
1 file changed, 69 insertions(+), 1 deletion(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index 208c5f5..1b60c5a 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,50 @@ 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 +882,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;
}
@@ -1773,9 +1832,18 @@ netdev_dpdk_get_ifindex(const struct netdev *netdev)
{
struct netdev_dpdk *dev = netdev_dpdk_cast(netdev);
int ifindex;
+int ret;
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 {
+if ((ret = netdev_dpdk_lookup_vhost_id(dev)) >= 0) {
+ifindex = VHOST_ID_TO_IFINDEX(ret);
+} else {
+ifindex = 0;
+}
+}
ovs_mutex_unlock(&dev->mutex);
return ifindex;
--
2.1.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [PATCH] netdev-dpdk: add sflow support for vhost-user ports
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 ; 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
>---
> 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);
>+}
use BSD-style braces for the 'else' case
>+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
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
Re: [ovs-dev] [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
>---
> 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);
>+}
use BSD-style braces for the 'else' case
>+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
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] netdev-dpdk: add sflow support for vhost-user ports
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
---
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);
+}
+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
Re: [ovs-dev] [PATCH] netdev-dpdk: add sflow support for vhost-user ports
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
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 ; 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
>>---
>> 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
Re: [ovs-dev] [PATCH] netdev-dpdk: add sflow support for vhost-user ports
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 ; 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
>---
> 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->
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 ; 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
>>---
>> 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
>st
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
>---
> 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
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
[ovs-dev] [PATCH] netdev-dpdk: add sflow support for vhost-user ports
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
---
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);
+
+/* 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;
--
2.1.0
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
