On 04/13/2016 11:38 AM, Panu Matilainen wrote:
On 04/13/2016 11:33 AM, Panu Matilainen wrote:
On 04/13/2016 11:16 AM, Weglicki, MichalX wrote:
-----Original Message-----
From: Panu Matilainen [mailto:pmati...@redhat.com]
Sent: Wednesday, April 13, 2016 8:50 AM
To: Weglicki, MichalX <michalx.wegli...@intel.com>; dev@openvswitch.org
Subject: Re: [ovs-dev] [PATCH] Update relevant artifacts to add
support for DPDK 16.04.
On 04/12/2016 05:05 PM, mweglicx wrote:
[...]
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index e09b471..2295e53 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1740,31 +1740,31 @@ netdev_dpdk_get_features(const struct netdev
*netdev_,
link = dev->link;
ovs_mutex_unlock(&dev->mutex);
- if (link.link_duplex == ETH_LINK_AUTONEG_DUPLEX) {
+ if (link.link_duplex == ETH_LINK_AUTONEG) {
This isn't right, link_duplex is either ETH_LINK_HALF_DUPLEX or
ETH_LINK_FULL_DUPLEX. It sort of happens to work because both
ETH_LINK_AUTONEG and ETH_LINK_FULL_DUPLEX are defined to 1 and having
autonegotiation end up with half-duplex these days isn't that likely.
[MW] Thank you, your're right, just to be sure, you mean that I should
check autoneg through
link_autoneg flag, like this:
if (link.link_autoneg) {
*current = NETDEV_F_AUTONEG;
} else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
if (link.link_speed == ETH_SPEED_NUM_10M) {
*current = NETDEV_F_10MB_HD;
}
if (link.link_speed == ETH_SPEED_NUM_100M) {
*current = NETDEV_F_100MB_HD;
}
if (link.link_speed == ETH_SPEED_NUM_1G) {
*current = NETDEV_F_1GB_HD;
}
} else if (link.link_duplex == ETH_LINK_FULL_DUPLEX) {
if (link.link_speed == ETH_SPEED_NUM_10M) {
*current = NETDEV_F_10MB_FD;
}
if (link.link_speed == ETH_SPEED_NUM_100M) {
*current = NETDEV_F_100MB_FD;
}
if (link.link_speed == ETH_SPEED_NUM_1G) {
*current = NETDEV_F_1GB_FD;
}
if (link.link_speed == ETH_SPEED_NUM_10G) {
*current = NETDEV_F_10GB_FD;
}
}
Based on the comments in header files this should work. Thanks in
advance.
Yup, that's what I meant. However turns out that isn't right either.
Looking at lib/netdev-linux.c, *current is a bitfield with current speed
set, AND if its autonegotiated then NETDEV_F_AUTONEG is *also* set. I
think this code was never quite correct to begin with.
To clarify, I think the DPDK code for this was never quite correct to
begin with.
So I had a quick look whether 2.5 and earlier need some kind of fix in
this area. AFAICS the deal is that the autoneg case is a "can't happen"
situation here: when initializing a port, value of zero indicates
"autonegotiate this, please" but the driver then sets the fields to the
negotiated value and there's no way to tell whether it was
autonegotiated or not after the fact.
In other words, this could be applied to OVS 2.5 (and older) and nothing
would actually change:
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -1736,11 +1736,7 @@ netdev_dpdk_get_features(const struct netdev *netdev,
link = dev->link;
ovs_mutex_unlock(&dev->mutex);
- if (link.link_duplex == ETH_LINK_AUTONEG_DUPLEX) {
- if (link.link_speed == ETH_LINK_SPEED_AUTONEG) {
- *current = NETDEV_F_AUTONEG;
- }
- } else if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
+ if (link.link_duplex == ETH_LINK_HALF_DUPLEX) {
if (link.link_speed == ETH_LINK_SPEED_10) {
*current = NETDEV_F_10MB_HD;
}
But since its just a NOP and now fixed for newer versions... probably
not worth the trouble.
- Panu -
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
