On Wed, Mar 09, 2016 at 11:09:55AM +0100, Marc wrote:
> On 9 March 2016 at 09:45, N?lio Laranjeiro <[1]nelio.laranjeiro at
> 6wind.com>
> wrote:
>
> Hi Marc,
>
> 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
> >? ?auto-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
> >? ?values of all supported link speeds, in Mbps.
> > * Converted link_speed to uint32_t to accomodate 100G speeds
> >? ?and beyond (bug).
> > * Added autoneg flag in struct rte_eth_link to indicate if
> >? ?link speed was a result of auto-negociation or was fixed
> >? ?by configuration.
> > * Added utility function to convert numeric speeds to bitmap
> >? ?fields.
> > * Added rte_eth_speed_to_bm_flag() to version map.
> >
> > Signed-off-by: Marc Sune <[2]marcdevel at gmail.com>
> > ---
> >? app/test-pipeline/init.c? ? ? ? ? ? ? ? ? |? ?2 +-
> >? app/test-pmd/cmdline.c? ? ? ? ? ? ? ? ? ? | 124
> +++++++++++++++---------------
> >? app/test-pmd/config.c? ? ? ? ? ? ? ? ? ? ?|? ?4 +-
> >? app/test/virtual_pmd.c? ? ? ? ? ? ? ? ? ? |? ?4 +-
> >? drivers/net/af_packet/rte_eth_af_packet.c |? ?5 +-
> >? drivers/net/bnx2x/bnx2x_ethdev.c? ? ? ? ? |? ?8 +-
> >? drivers/net/bonding/rte_eth_bond_8023ad.c |? 14 ++--
> >? drivers/net/cxgbe/base/t4_hw.c? ? ? ? ? ? |? ?8 +-
> >? drivers/net/cxgbe/cxgbe_ethdev.c? ? ? ? ? |? ?2 +-
> >? drivers/net/e1000/em_ethdev.c? ? ? ? ? ? ?| 116
> ++++++++++++++--------------
> >? drivers/net/e1000/igb_ethdev.c? ? ? ? ? ? | 111
> +++++++++++++-------------
> >? drivers/net/fm10k/fm10k_ethdev.c? ? ? ? ? |? ?8 +-
> >? drivers/net/i40e/i40e_ethdev.c? ? ? ? ? ? |? 73 +++++++++---------
> >? drivers/net/i40e/i40e_ethdev_vf.c? ? ? ? ?|? 11 +--
> >? drivers/net/ixgbe/ixgbe_ethdev.c? ? ? ? ? |? 78 ++++++++-----------
> >? drivers/net/mlx4/mlx4.c? ? ? ? ? ? ? ? ? ?|? ?6 +-
> >? drivers/net/mlx5/mlx5_ethdev.c? ? ? ? ? ? |? 10 +--
> >? drivers/net/mpipe/mpipe_tilegx.c? ? ? ? ? |? ?6 +-
> >? drivers/net/nfp/nfp_net.c? ? ? ? ? ? ? ? ?|? ?4 +-
> >? drivers/net/null/rte_eth_null.c? ? ? ? ? ?|? ?5 +-
> >? drivers/net/pcap/rte_eth_pcap.c? ? ? ? ? ?|? ?9 ++-
> >? drivers/net/ring/rte_eth_ring.c? ? ? ? ? ?|? ?5 +-
> >? drivers/net/virtio/virtio_ethdev.c? ? ? ? |? ?2 +-
> >? drivers/net/virtio/virtio_ethdev.h? ? ? ? |? ?2 -
> >? drivers/net/vmxnet3/vmxnet3_ethdev.c? ? ? |? ?5 +-
> >? drivers/net/xenvirt/rte_eth_xenvirt.c? ? ?|? ?5 +-
> >? examples/ip_pipeline/config_parse.c? ? ? ?|? ?3 +-
> >? lib/librte_ether/rte_ethdev.c? ? ? ? ? ? ?|? 49 ++++++++++++
> >? lib/librte_ether/rte_ethdev.h? ? ? ? ? ? ?| 113
> +++++++++++++++++----------
> >? lib/librte_ether/rte_ether_version.map? ? |? ?6 ++
> >? 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)
> >? ? ? ? ? ? ? ?port = (uint8_t) app.ports[i];
> >? ? ? ? ? ? ? ?memset(&link, 0, sizeof(link));
> >? ? ? ? ? ? ? ?rte_eth_link_get_nowait(port, &link);
> > -? ? ? ? ? ? ?RTE_LOG(INFO, USER1, "Port %u (%u Gbps) %s\n",
> > +? ? ? ? ? ? ?RTE_LOG(INFO, USER1, "Port %u (%d Gbps) %s\n",
> >? ? ? ? ? ? ? ? ? ? ? ?port,
> >? ? ? ? ? ? ? ? ? ? ? ?link.link_speed / 1000,
> >? ? ? ? ? ? ? ? ? ? ? ?link.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 {
> >? ? ? ?cmdline_fixed_string_t value2;
> >? };
> >
> > +static int
> > +parse_and_check_speed_duplex(char *value1, char *value2, uint32_t
> *link_speed)
>
> "value" variables should have the more friendly name, we need to read
> the
> code to understand what is value1 and value2.
>
> Hello,
> Ok, but note that the entire source code of cmdline.c uses value1 and
> 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?lio Laranjeiro
> 6WIND
>
> References
>
> Visible links
> 1. mailto:nelio.laranjeiro at 6wind.com
> 2. mailto:marcdevel at gmail.com
--
N?lio Laranjeiro
6WIND
