On 4/4/17, 5:47 PM, "Darrell Ball" <db...@vmware.com> wrote:
On 4/4/17, 3:09 AM, "Lal, PrzemyslawX" <przemyslawx....@intel.com> wrote:
On 04/04/2017 06:14, Darrell Ball wrote:
>
> On 4/3/17, 5:27 AM, "ovs-dev-boun...@openvswitch.org on behalf of
Przemyslaw Lal" <ovs-dev-boun...@openvswitch.org on behalf of
przemyslawx....@intel.com> wrote:
>
> 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 ifindexes 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 RFC 2863 specification regarding
re-using
> ifindex values by the same virtual interfaces and maximum
ifindex value.
>
> Signed-off-by: Przemyslaw Lal <przemyslawx....@intel.com>
> ---
> lib/netdev-dpdk.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
> index ddc651b..687b0a5 100644
> --- a/lib/netdev-dpdk.c
> +++ b/lib/netdev-dpdk.c
> @@ -2215,7 +2215,11 @@ netdev_dpdk_get_ifindex(const struct
netdev *netdev)
> int ifindex;
>
> ovs_mutex_lock(&dev->mutex);
> - ifindex = dev->port_id;
> + /* Calculate hash from the netdev name. Ensure that ifindex
is a 24-bit
> + * postive integer to meet RFC 2863 recommendations.
> + */
> + uint32_t h = hash_string(netdev->name, 0);
> + ifindex = (int)(h % 0xfffffe + 1);
>
>
> If user configuration was supported, enforcing uniqueness would be the
> responsibility of the user.
This was already discussed on this mailing list and outcome was that
while hash is not perfect, it is the best solution for now.
Also, please keep in mind that names of the physical DPDK devices and
dpdkvhostuser interfaces are configurable, so user can still enforce
uniqueness.
I know uniqueness could be enforced by trial and error of name selection.
I saw the comment
“At some point, with vhost-pmd we will have port_ids also for vhost
interfaces.
Maybe we can revisit this approach when that becomes available.”
If others are fine, then so am I.
The uniqueness issue is understood and there is a workaround capability to
address it.
Let us just fold the patch in, since the patch has been out for long enough to
receive feedback.
Acked by: Darrell Ball <dlu...@gmail.com>
>
> One minor question:
> I know other ifindex implementations do not limit to 24 bits, so I
checked RFC 2863.
> What section is the 24 bit limit recommendation mentioned in RFC
2863; I missed it.
Recommendation of RFC 2863 is to use "small integers". Main purpose of
this patch is to enable dpdkvhostuser interfaces in sFlow metrics collecting
tools.
After posting previous version I've received feedback from the
community that not all interfaces are visible in sFlow collectors and maximum
supported ifindex is 2^24.
Here's the sFlow v5 specification:
https://urldefense.proofpoint.com/v2/url?u=http-3A__sflow.org_sflow-5Fversion-5F5.txt&d=DwICaQ&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=BU2KdxyVIZVbS8qF48IH8n4FNs8DBkSQf7Bp0zdQvXE&s=ZgBDm95VkGw_R6_ULUFT5mh0OtbGPzcnjpWXze9DIGw&e=
2^24 is an sflow recommendation (rather than Interfaces MIB recommendation)
– that’s fine; I was just curious where it came from.
>
>
>
> ovs_mutex_unlock(&dev->mutex);
>
> return ifindex;
> --
> 1.9.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
>
https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=Rcpenle5jqQ1mu3STwFMODvsTFjKL9iMBrwMfu9J8FM&s=FJtoKpLo8NxpzHwFKSz6xsT3aGqZCiR507-MDVxjFeE&e=
>
>
>
>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
