From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id F162F2B9C for ; Wed, 23 Mar 2016 03:44:42 +0100 (CET) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga103.fm.intel.com with ESMTP; 22 Mar 2016 19:44:41 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.24,380,1455004800"; d="scan'208";a="939631723" Received: from fmsmsx103.amr.corp.intel.com ([10.18.124.201]) by orsmga002.jf.intel.com with ESMTP; 22 Mar 2016 19:44:41 -0700 Received: from fmsmsx115.amr.corp.intel.com (10.18.116.19) by FMSMSX103.amr.corp.intel.com (10.18.124.201) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 22 Mar 2016 19:44:41 -0700 Received: from shsmsx102.ccr.corp.intel.com (10.239.4.154) by fmsmsx115.amr.corp.intel.com (10.18.116.19) with Microsoft SMTP Server (TLS) id 14.3.248.2; Tue, 22 Mar 2016 19:44:40 -0700 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.132]) by shsmsx102.ccr.corp.intel.com ([169.254.2.232]) with mapi id 14.03.0248.002; Wed, 23 Mar 2016 10:44:37 +0800 From: "Zhang, Helin" To: Thomas Monjalon , "marcdevel@gmail.com" , "Richardson, Bruce" , "Doherty, Declan" , "Ananyev, Konstantin" , "Lu, Wenzhuo" , "Chen, Jing D" , "harish.patil@qlogic.com" , "rahul.lakkireddy@chelsio.com" , "johndale@cisco.com" , "vido@cesnet.cz" , "adrien.mazarguil@6wind.com" , "alejandro.lucero@netronome.com" CC: "dev@dpdk.org" Thread-Topic: [PATCH v11 2/8] ethdev: use constants for link duplex Thread-Index: AQHRgHhZsI+iQf5wLUyKFF1v4/RAlp9mWbVQ Date: Wed, 23 Mar 2016 02:44:37 +0000 Message-ID: References: <1457992546-32230-1-git-send-email-thomas.monjalon@6wind.com> <1458238145-7496-1-git-send-email-thomas.monjalon@6wind.com> <1458238145-7496-3-git-send-email-thomas.monjalon@6wind.com> In-Reply-To: <1458238145-7496-3-git-send-email-thomas.monjalon@6wind.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v11 2/8] ethdev: use constants for link duplex X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 23 Mar 2016 02:44:43 -0000 > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Friday, March 18, 2016 2:09 AM > To: marcdevel@gmail.com; Richardson, Bruce; Doherty, Declan; Ananyev, > Konstantin; Lu, Wenzhuo; Zhang, Helin; Chen, Jing D; > harish.patil@qlogic.com; rahul.lakkireddy@chelsio.com; > johndale@cisco.com; vido@cesnet.cz; adrien.mazarguil@6wind.com; > alejandro.lucero@netronome.com > Cc: dev@dpdk.org > Subject: [PATCH v11 2/8] ethdev: use constants for link duplex >=20 > From: Marc Sune >=20 > Some duplex values are replaced from 0 to half-duplex when link is down. >=20 > Some drivers are still using their own constants for duplex modes. >=20 > Signed-off-by: Marc Sune > --- > drivers/net/e1000/em_ethdev.c | 2 +- > drivers/net/e1000/igb_ethdev.c | 2 +- > drivers/net/ixgbe/ixgbe_ethdev.c | 2 +- > drivers/net/virtio/virtio_ethdev.c | 2 +- drivers/net/virtio/virtio_eth= dev.h | > 2 -- > lib/librte_ether/rte_ethdev.h | 2 +- > 6 files changed, 5 insertions(+), 7 deletions(-) >=20 > diff --git a/drivers/net/e1000/em_ethdev.c > b/drivers/net/e1000/em_ethdev.c index 58093c6..943a270 100644 > --- a/drivers/net/e1000/em_ethdev.c > +++ b/drivers/net/e1000/em_ethdev.c > @@ -1108,7 +1108,7 @@ eth_em_link_update(struct rte_eth_dev *dev, int > wait_to_complete) > link.link_status =3D ETH_LINK_UP; > } else if (!link_check && (link.link_status =3D=3D ETH_LINK_UP)) { > link.link_speed =3D 0; > - link.link_duplex =3D 0; > + link.link_duplex =3D ETH_LINK_HALF_DUPLEX; > link.link_status =3D ETH_LINK_DOWN; > } > rte_em_dev_atomic_write_link_status(dev, &link); diff --git > a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c index > 311f866..ea156ce 100644 > --- a/drivers/net/e1000/igb_ethdev.c > +++ b/drivers/net/e1000/igb_ethdev.c > @@ -2033,7 +2033,7 @@ eth_igb_link_update(struct rte_eth_dev *dev, int > wait_to_complete) > link.link_status =3D ETH_LINK_UP; > } else if (!link_check) { > link.link_speed =3D 0; > - link.link_duplex =3D 0; > + link.link_duplex =3D ETH_LINK_HALF_DUPLEX; > link.link_status =3D ETH_LINK_DOWN; > } > rte_igb_dev_atomic_write_link_status(dev, &link); diff --git > a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c > index be28f7e..35dac49 100644 > --- a/drivers/net/ixgbe/ixgbe_ethdev.c > +++ b/drivers/net/ixgbe/ixgbe_ethdev.c > @@ -2997,7 +2997,7 @@ ixgbe_dev_link_update(struct rte_eth_dev *dev, > int wait_to_complete) >=20 > link.link_status =3D ETH_LINK_DOWN; > link.link_speed =3D 0; > - link.link_duplex =3D 0; > + link.link_duplex =3D ETH_LINK_HALF_DUPLEX; > memset(&old, 0, sizeof(old)); > rte_ixgbe_dev_atomic_read_link_status(dev, &old); >=20 > diff --git a/drivers/net/virtio/virtio_ethdev.c > b/drivers/net/virtio/virtio_ethdev.c > index 3ebc221..63a368a 100644 > --- a/drivers/net/virtio/virtio_ethdev.c > +++ b/drivers/net/virtio/virtio_ethdev.c > @@ -1401,7 +1401,7 @@ virtio_dev_link_update(struct rte_eth_dev *dev, > __rte_unused int wait_to_complet > memset(&link, 0, sizeof(link)); > virtio_dev_atomic_read_link_status(dev, &link); > old =3D link; > - link.link_duplex =3D FULL_DUPLEX; > + link.link_duplex =3D ETH_LINK_FULL_DUPLEX; > link.link_speed =3D SPEED_10G; >=20 > if (vtpci_with_feature(hw, VIRTIO_NET_F_STATUS)) { diff --git > a/drivers/net/virtio/virtio_ethdev.h b/drivers/net/virtio/virtio_ethdev.h > index fed9571..66423a0 100644 > --- a/drivers/net/virtio/virtio_ethdev.h > +++ b/drivers/net/virtio/virtio_ethdev.h > @@ -42,8 +42,6 @@ > #define SPEED_100 100 > #define SPEED_1000 1000 > #define SPEED_10G 10000 > -#define HALF_DUPLEX 1 > -#define FULL_DUPLEX 2 >=20 > #ifndef PAGE_SIZE > #define PAGE_SIZE 4096 > diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.= h > index ec8d6b1..5379bee 100644 > --- a/lib/librte_ether/rte_ethdev.h > +++ b/lib/librte_ether/rte_ethdev.h > @@ -246,7 +246,7 @@ struct rte_eth_stats { > */ > struct rte_eth_link { > uint16_t link_speed; /**< ETH_LINK_SPEED_[10, 100, 1000, 10000] > */ > - uint16_t link_duplex; /**< ETH_LINK_[HALF_DUPLEX, > FULL_DUPLEX] */ > + uint16_t link_duplex; /**< ETH_LINK_[HALF/FULL]_DUPLEX */ > uint8_t link_status : 1; /**< ETH_LINK_[DOWN/UP] */ > }__attribute__((aligned(8))); /**< aligned for atomic64 read/write *= / For link speed and link duplex, I'd suggest to add one more status of 'UNKN= OWN'. Because, sometimes it cannot get all the information from hardware. For link stauts, assume it in DOWN state is acceptable, while for other two= , I don't think so. Currently it can be seen that a default link speed and duplex will be set i= f it cannot get the accurate info from hardware. That's not good, and I think UNKNOWN c= ould be better. What do you think? Thanks, Helin >=20 > -- > 2.7.0