In current implementation port_id is used as an ifindex for all netdev-
dpdk interfaces.
For physical DPDK interfaces using port_id as ifindex causes that '0' is
set as ifindex for 'dpdk0' interface, '1' for 'dpdk1' and so on. For the
DPDK vHost interfaces ifindexes are not even assigned (0 is used by
default) due to the fact that vHost ports don't use port_id field from the
DPDK library.
This causes multiple negative side-effects. First of all 0 is an invalid
ifindex value. The other issue is possible overlapping of 'dpdkX'
interfaces ifindex values with the infindexes of kernel space interfaces
which may cause problems in any external tools that use those values.
Neither 'dpdk0', nor any DPDK vHost interfaces are visible in sFlow
collector tools, as all interfaces with ifindexes smaller than 1 are
ignored.
Proposed solution to these issues is to calculate a hash of interface's
name and use calculated value as an ifindex. This way interfaces keep
their ifindexes during OVS-DPDK restarts, ports re-initialization events,
etc., show up in sFlow collectors and meet RFC2863 specification regarding
re-using ifindex values by the same virtual interfaces.
Signed-off-by: Przemyslaw Lal <[email protected]>
---
Thanks for the patch.
I'm not the most experienced with sflow but after looking into it its clear the
need for a change to ifindex is required for DPDK devices.
lib/netdev-dpdk.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index de78ddd..ef99eb3
100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -2075,7 +2075,13 @@ netdev_dpdk_get_ifindex(const struct netdev
*netdev)
int ifindex;
ovs_mutex_lock(&dev->mutex);
- ifindex = dev->port_id;
Using a hash for the ifindex of dpdk devices will essentially make them non
sequential.
More of a question to the community, but is it ok if ifindexes are
non-sequential? Wasn't sure of this myself.
There's no requirement for ifIndexes to be sequential, even more -
calculating hash using devices' names ensures that the same devices will
have the same ifIndex after event of port re-initialization and OVS
restart. Such behavior is also recommended by IETF RFC 2863 standard.
Using hash was also suggested by the community during patch V1 review.
+ /* Calculate hash from the netdev name using hash_bytes() function.
+ * Because ifindex is declared as signed int in the kernel sources
and
+ * OVS follows this implementation right shift is needed to set sign
bit
+ * to 0 and then XOR to slightly improve collision rate.
+ */
+ uint32_t h = hash_bytes(netdev->name, strlen(netdev->name), 0);
+ ifindex = (int)((h >> 1) ^ (h & 0x0FFFFFFF));
In the (unlikely) event of a hash collision, what would be the outcome?
From what I can see the main use is for dpif_sflow_add_port().
I'm just wondering is there a need to ensure that no 2 DPDK ports have the same
ifindex?
From What I can see dpif_sflow_add_port() would still execute for the port.
Only one of them will be accessible in the sFlow. However, as you
mentioned, this is very unlikely - during my tests I've generated random
100 000 vHost-user port names using"vhuXXXXXXXX-XX" naming scheme where
Xs are the first characters of UUID - I used Openstack port naming
convention as an inspiration - and maximum number of collisions I
observed was 5, but in the most of runs it was 0.
Anyway, at the end of the day it shouldn't be a big issue since DPDK
vHost and DPDK physical interfaces names can be changed by user.
ovs_mutex_unlock(&dev->mutex);
return ifindex;
--
1.9.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
