DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Morten Brørup" <mb@smartsharesystems.com>
To: "Thomas Monjalon" <thomas.monjalon@6wind.com>,
	"Marc Sune" <marcdevel@gmail.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v4 0/2] ethdev: add port speed capability bitmap
Date: Mon, 14 Sep 2015 12:52:03 +0200	[thread overview]
Message-ID: <98CBD80474FA8B44BF855DF32C47DC358AF3B7@smartserver.smartshare.dk> (raw)
In-Reply-To: <2046894.c3eJ0QZGuc@xps13>

It is important to consider that a multipath link (bonding etc.) is not a physical link, but a logical link (built on top of multiple physical links). Regardless whether it is a Layer2 link aggregate (IEEE 802.1ad, Ethernet bonding, EtherChannel, DSL pair bonding, etc.) or a Layer3 multipath link (e.g. simultaneously using Wi-Fi and mobile networks). So it doesn't make sense trying to impose physical link properties on a purely logical link. Likewise, it doesn't make sense to impose logical link properties on physical links. In other words: Don't consider bonding or any other logical link types when designing the PHY API.

I think there is consensus that 1/ (PHY capabilities) and 2/ (PHY advertisements) should use the same definitions, specifically a bitmap field. And when you disregard bonding, I don't see any reason to use different definitions for 3/ (PHY negotiation result). This makes it one unified API for all three purposes.

Nelio suggested adding a support function to convert the bitmap field to a speed value as an integer. I strongly support this, because you cannot expect the bitmap to be ordered by speed. This support function will be able to determine which speed is higher when exotic speeds are added to the bitmap. Please extend this conversion function to give three output parameters: speed, full/half duplex, auto negotiation/non-auto negotiation, or add two separate functions to get the duplex and auto-negotiation.

I haven't read the suggested code, but there should be some means in 2/ (advertisements) to disable auto negotiation, e.g. a single bit in the bitmap to indicate if the speed/duplex-indicating bits in the bitmap means forced speed/duplex (in which case only a single speed/duplex-bit should be set) or auto negotiation advertised speed/duplex (in which case multiple speed/duplex-bits can be set). And some means in 3/ (result) and maybe 2/ (advertisements) to select and/or indicate physical interface in dual-personality ports (e.g. ports where the PHY has both an SFP and a RJ45 connector, but only one of the two can be used at any time).


Med venlig hilsen / kind regards
- Morten Brørup

-----Original Message-----
From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] 
Sent: 13. september 2015 23:19
To: Marc Sune
Cc: Nélio Laranjeiro; dev@dpdk.org; Olga Shern; Adrien Mazarguil; Morten Brørup
Subject: Re: [dpdk-dev] [PATCH v4 0/2] ethdev: add port speed capability bitmap

2015-09-13 21:14, Marc Sune:
> 2015-09-09 15:33 GMT+02:00 Thomas Monjalon <thomas.monjalon@6wind.com>:
> > 2015-09-09 15:10, Nélio Laranjeiro:
> > > I think V2 is better, maybe you can add a function to convert a 
> > > single bitmap value to the equivalent integer and get rid of 
> > > ETH_SPEED_XXX
> > macros.
> > >
> > > Thomas what is your opinion?
> >
> > Your proposal looks good Nelio.
> 
> I am confused, specially since you were the one advocating for having 
> a unified set of constants for speeds (discussion in v2).

Yes, my first thought was advocating an unification between capabilities and negotiated link properties.
After I was convinced by Nelio's arguments: bitmap is good for capabilities (especially to describe every capabilities in one field) but integer is better for negotiated speed (especially for aggregated links).
Converting bitmap speed into integer should be easy to implement in a function.

> In any case, as I see it, if we want to address the comments of  M. Brorup:
> 
> http://comments.gmane.org/gmane.comp.networking.dpdk.devel/19664
> 
> we need bitmaps for rte_eth_conf link_speed to set the advertised speeds.

Yes I forgot this interesting comment. It is saying we need
	1/ capabilities
	2/ advertised modes (for auto-negotiation or fixed config)
	3/ negotiated mode
Previously we were focused only on 1/ and 3/.
2/ was only limited to a mode configured without negotiation and was using the same field as 3/.
Maybe we should really have 3 different fields. 1/ and 2/ would use a bitmap.

> Let me know if you really want to come back to v2 or not.

It needs more discussion. What do you think of the above proposal?
What is the opinion of Nelio? Morten?

  parent reply	other threads:[~2015-09-14 10:52 UTC|newest]

Thread overview: 61+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-11 23:45 [dpdk-dev] [RFC PATCH " Marc Sune
2015-05-11 23:45 ` [dpdk-dev] [RFC PATCH 1/2] Added ETH_SPEED_CAP bitmap in rte_eth_dev_info Marc Sune
2015-05-25 17:46   ` Stephen Hemminger
2015-05-26  8:41     ` Marc Sune
2015-05-26 15:03   ` Stephen Hemminger
2015-05-26 15:09     ` Marc Sune
2015-05-11 23:45 ` [dpdk-dev] [RFC PATCH 2/2] Filling speed capability bitmaps in the PMDs Marc Sune
2015-05-25 16:32 ` [dpdk-dev] [RFC PATCH 0/2] ethdev: add port speed capability bitmap Marc Sune
2015-05-26 19:50 ` [dpdk-dev] [PATCH v2 " Marc Sune
2015-05-26 19:50   ` [dpdk-dev] [PATCH v2 1/2] Added ETH_SPEED_CAP bitmap in rte_eth_dev_info Marc Sune
2015-05-27  4:02     ` Thomas Monjalon
2015-05-27  9:15       ` Marc Sune
2015-05-29 18:23         ` Thomas Monjalon
2015-06-08  8:50           ` Marc Sune
2015-06-11  9:08             ` Thomas Monjalon
2015-06-11 14:35               ` Marc Sune
2015-05-26 19:50   ` [dpdk-dev] [PATCH v2 2/2] Filling speed capability bitmaps in the PMDs Marc Sune
2015-05-26 21:20     ` Igor Ryzhov
2015-05-27  8:59       ` Marc Sune
2015-08-28 23:20   ` [dpdk-dev] [PATCH v3 0/2] ethdev: add port speed capability bitmap Marc Sune
2015-08-28 23:20     ` [dpdk-dev] [PATCH v3 1/2] Added ETH_SPEED_ bitmap in rte_eth_dev_info Marc Sune
2015-08-28 23:20     ` [dpdk-dev] [PATCH v3 2/2] Filling speed capability bitmaps in the PMDs Marc Sune
2015-08-29  0:16     ` [dpdk-dev] [PATCH v4 0/2] ethdev: add port speed capability bitmap Marc Sune
2015-08-29  0:16       ` [dpdk-dev] [PATCH v4 1/2] Added ETH_SPEED_ bitmap in rte_eth_dev_info Marc Sune
2015-08-29  0:16       ` [dpdk-dev] [PATCH v4 2/2] Filling speed capability bitmaps in the PMDs Marc Sune
2015-09-07 20:52       ` [dpdk-dev] [PATCH v4 0/2] ethdev: add port speed capability bitmap Marc Sune
2015-09-08 10:03         ` Nélio Laranjeiro
2015-09-08 20:26           ` Marc Sune
2015-09-09 13:10           ` Nélio Laranjeiro
2015-09-09 13:33             ` Thomas Monjalon
2015-09-13 19:14               ` Marc Sune
2015-09-13 21:18                 ` Thomas Monjalon
2015-09-14 10:02                   ` Nélio Laranjeiro
2015-09-14 10:52                   ` Morten Brørup [this message]
2015-09-14 21:33                     ` Marc Sune
2015-09-14 22:50                       ` Morten Brørup
2015-09-15  8:25                         ` Nélio Laranjeiro
2015-09-15  8:48                           ` Marc Sune
2015-09-15  9:39                             ` Morten Brørup
2015-09-15 10:04                             ` Adrien Mazarguil
2015-09-15 10:24                               ` Morten Brørup
2015-09-15 21:20                               ` Marc Sune
2015-09-16 14:41                                 ` Adrien Mazarguil
2015-09-15  7:05                       ` Thomas Monjalon
2015-09-15  7:33                         ` Morten Brørup
2015-09-15  9:06                       ` Morten Brørup
2015-10-04 21:12       ` [dpdk-dev] [PATCH v5 0/4] ethdev: add speed capabilities and refactor link API Marc Sune
2015-10-04 21:12         ` [dpdk-dev] [PATCH v5 1/4] ethdev: Added ETH_SPEED_CAP bitmap for ports Marc Sune
2015-10-04 21:12         ` [dpdk-dev] [PATCH v5 2/4] ethdev: Fill speed capability bitmaps in the PMDs Marc Sune
2015-10-04 21:12         ` [dpdk-dev] [PATCH v5 3/4] ethdev: redesign link speed config API Marc Sune
2015-10-05 10:59           ` Neil Horman
2015-10-07 13:31             ` Marc Sune
2015-10-06 13:48           ` Nélio Laranjeiro
2015-10-07 13:37             ` Marc Sune
2016-01-28 17:33           ` Harish Patil
2016-02-02  2:20             ` Stephen Hemminger
2016-02-02 22:30               ` Marc
2016-02-11 15:27                 ` Nélio Laranjeiro
2016-02-11 23:23                   ` Marc
2015-10-04 21:12         ` [dpdk-dev] [PATCH v5 4/4] doc: update with link changes Marc Sune
2015-10-04 21:21         ` [dpdk-dev] [PATCH v5 0/4] ethdev: add speed capabilities and refactor link API Marc Sune

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=98CBD80474FA8B44BF855DF32C47DC358AF3B7@smartserver.smartshare.dk \
    --to=mb@smartsharesystems.com \
    --cc=dev@dpdk.org \
    --cc=marcdevel@gmail.com \
    --cc=thomas.monjalon@6wind.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).