From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?iso-8859-1?Q?N=E9lio?= Laranjeiro Subject: Re: [PATCH v9 3/4] ethdev: redesign link speed config API Date: Wed, 9 Mar 2016 11:20:40 +0100 Message-ID: <20160309102040.GQ27714@autoinstall.dev.6wind.com> References: <1455488259-1000-1-git-send-email-marcdevel@gmail.com> <1456793151-1475-1-git-send-email-marcdevel@gmail.com> <1456793151-1475-4-git-send-email-marcdevel@gmail.com> <20160309084547.GM27714@autoinstall.dev.6wind.com> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: quoted-printable Cc: "dev@dpdk.org" To: Marc Return-path: Received: from mail-wm0-f41.google.com (mail-wm0-f41.google.com [74.125.82.41]) by dpdk.org (Postfix) with ESMTP id EACC22BAA for ; Wed, 9 Mar 2016 11:20:54 +0100 (CET) Received: by mail-wm0-f41.google.com with SMTP id p65so185578470wmp.1 for ; Wed, 09 Mar 2016 02:20:54 -0800 (PST) Content-Disposition: inline In-Reply-To: List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Wed, Mar 09, 2016 at 11:09:55AM +0100, Marc wrote: > On 9 March 2016 at 09:45, N=E9lio Laranjeiro <[1]nelio.laranjeiro@6w= ind.com> > wrote: >=20 > Hi Marc, >=20 > A small remark bellow. > On Tue, Mar 01, 2016 at 01:45:50AM +0100, Marc Sune wrote: > > This patch redesigns the API to set the link speed/s configure > > for an ethernet port. Specifically: > > > > - it allows to define a set of advertised speeds for > >=A0 =A0auto-negociation. > > - it allows to disable link auto-negociation (single fixed speed= ). > > - default: auto-negociate all supported speeds. > > > > Other changes: > > > > * Added utility MACROs ETH_SPEED_NUM_XXX with the numeric > >=A0 =A0values of all supported link speeds, in Mbps. > > * Converted link_speed to uint32_t to accomodate 100G speeds > >=A0 =A0and beyond (bug). > > * Added autoneg flag in struct rte_eth_link to indicate if > >=A0 =A0link speed was a result of auto-negociation or was fixed > >=A0 =A0by configuration. > > * Added utility function to convert numeric speeds to bitmap > >=A0 =A0fields. > > * Added rte_eth_speed_to_bm_flag() to version map. > > > > Signed-off-by: Marc Sune <[2]marcdevel@gmail.com> > > --- > >=A0 app/test-pipeline/init.c=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 |= =A0 =A02 +- > >=A0 app/test-pmd/cmdline.c=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= | 124 > +++++++++++++++--------------- > >=A0 app/test-pmd/config.c=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0|=A0 =A04 +- > >=A0 app/test/virtual_pmd.c=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= |=A0 =A04 +- > >=A0 drivers/net/af_packet/rte_eth_af_packet.c |=A0 =A05 +- > >=A0 drivers/net/bnx2x/bnx2x_ethdev.c=A0 =A0 =A0 =A0 =A0 |=A0 =A08= +- > >=A0 drivers/net/bonding/rte_eth_bond_8023ad.c |=A0 14 ++-- > >=A0 drivers/net/cxgbe/base/t4_hw.c=A0 =A0 =A0 =A0 =A0 =A0 |=A0 =A0= 8 +- > >=A0 drivers/net/cxgbe/cxgbe_ethdev.c=A0 =A0 =A0 =A0 =A0 |=A0 =A02= +- > >=A0 drivers/net/e1000/em_ethdev.c=A0 =A0 =A0 =A0 =A0 =A0 =A0| 116 > ++++++++++++++-------------- > >=A0 drivers/net/e1000/igb_ethdev.c=A0 =A0 =A0 =A0 =A0 =A0 | 111 > +++++++++++++------------- > >=A0 drivers/net/fm10k/fm10k_ethdev.c=A0 =A0 =A0 =A0 =A0 |=A0 =A08= +- > >=A0 drivers/net/i40e/i40e_ethdev.c=A0 =A0 =A0 =A0 =A0 =A0 |=A0 73= +++++++++--------- > >=A0 drivers/net/i40e/i40e_ethdev_vf.c=A0 =A0 =A0 =A0 =A0|=A0 11 += -- > >=A0 drivers/net/ixgbe/ixgbe_ethdev.c=A0 =A0 =A0 =A0 =A0 |=A0 78 += +++++++----------- > >=A0 drivers/net/mlx4/mlx4.c=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0= |=A0 =A06 +- > >=A0 drivers/net/mlx5/mlx5_ethdev.c=A0 =A0 =A0 =A0 =A0 =A0 |=A0 10= +-- > >=A0 drivers/net/mpipe/mpipe_tilegx.c=A0 =A0 =A0 =A0 =A0 |=A0 =A06= +- > >=A0 drivers/net/nfp/nfp_net.c=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0|= =A0 =A04 +- > >=A0 drivers/net/null/rte_eth_null.c=A0 =A0 =A0 =A0 =A0 =A0|=A0 =A0= 5 +- > >=A0 drivers/net/pcap/rte_eth_pcap.c=A0 =A0 =A0 =A0 =A0 =A0|=A0 =A0= 9 ++- > >=A0 drivers/net/ring/rte_eth_ring.c=A0 =A0 =A0 =A0 =A0 =A0|=A0 =A0= 5 +- > >=A0 drivers/net/virtio/virtio_ethdev.c=A0 =A0 =A0 =A0 |=A0 =A02 += - > >=A0 drivers/net/virtio/virtio_ethdev.h=A0 =A0 =A0 =A0 |=A0 =A02 - > >=A0 drivers/net/vmxnet3/vmxnet3_ethdev.c=A0 =A0 =A0 |=A0 =A05 +- > >=A0 drivers/net/xenvirt/rte_eth_xenvirt.c=A0 =A0 =A0|=A0 =A05 +- > >=A0 examples/ip_pipeline/config_parse.c=A0 =A0 =A0 =A0|=A0 =A03 += - > >=A0 lib/librte_ether/rte_ethdev.c=A0 =A0 =A0 =A0 =A0 =A0 =A0|=A0 = 49 ++++++++++++ > >=A0 lib/librte_ether/rte_ethdev.h=A0 =A0 =A0 =A0 =A0 =A0 =A0| 113 > +++++++++++++++++---------- > >=A0 lib/librte_ether/rte_ether_version.map=A0 =A0 |=A0 =A06 ++ > >=A0 30 files changed, 448 insertions(+), 350 deletions(-) > > > > diff --git a/app/test-pipeline/init.c b/app/test-pipeline/init.c > > index db2196b..6a69fe2 100644 > > --- a/app/test-pipeline/init.c > > +++ b/app/test-pipeline/init.c > > @@ -200,7 +200,7 @@ app_ports_check_link(void) > >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0port =3D (uint8_t) app.ports[i]; > >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0memset(&link, 0, sizeof(link)); > >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rte_eth_link_get_nowait(port, &lin= k); > > -=A0 =A0 =A0 =A0 =A0 =A0 =A0RTE_LOG(INFO, USER1, "Port %u (%u Gb= ps) %s\n", > > +=A0 =A0 =A0 =A0 =A0 =A0 =A0RTE_LOG(INFO, USER1, "Port %u (%d Gb= ps) %s\n", > >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0port, > >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0link.link_speed / = 1000, > >=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0link.link_status ?= "UP" : "DOWN"); > > diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c > > index 52e9f5f..57ad25f 100644 > > --- a/app/test-pmd/cmdline.c > > +++ b/app/test-pmd/cmdline.c > > @@ -956,14 +956,65 @@ struct cmd_config_speed_all { > >=A0 =A0 =A0 =A0cmdline_fixed_string_t value2; > >=A0 }; > > > > +static int > > +parse_and_check_speed_duplex(char *value1, char *value2, uint32= _t > *link_speed) >=20 > "value" variables should have the more friendly name, we need to r= ead > the > code to understand what is value1 and value2. >=20 > Hello, > Ok, but note that the entire source code of cmdline.c uses value1 an= d > value2 to reference speed and duplex mode. This is not something I > introduced in this patch. > Marc Ok, I did not look in the whole file only in your patch. You kept the logic of the file, it is good for me. > >[...] > -- > N=E9lio Laranjeiro > 6WIND >=20 > References >=20 > Visible links > 1. mailto:nelio.laranjeiro@6wind.com > 2. mailto:marcdevel@gmail.com --=20 N=E9lio Laranjeiro 6WIND