-----Original Message----- From: Panu Matilainen [mailto:pmati...@redhat.com] Sent: Wednesday, April 13, 2016 9:38 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/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. - Panu - Yes, you are right, I double checked netdev-linux and this is exactly how it is implemented there. Ok, I will apply v2 shortly, thank you. Br, Michal. -------------------------------------------------------------- Intel Research and Development Ireland Limited Registered in Ireland Registered Office: Collinstown Industrial Park, Leixlip, County Kildare Registered Number: 308263 This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies. _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev