From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: <marcdevel@gmail.com> Received: from mail-io0-f181.google.com (mail-io0-f181.google.com [209.85.223.181]) by dpdk.org (Postfix) with ESMTP id A71508D94 for <dev@dpdk.org>; Mon, 14 Sep 2015 23:33:39 +0200 (CEST) Received: by iofh134 with SMTP id h134so181719940iof.0 for <dev@dpdk.org>; Mon, 14 Sep 2015 14:33:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=oQndQ1D5N/Rc3IWBqA7yLCjfiTezCWDtVwDINJ+ZExg=; b=S6NA5q3d4rTytG8fJneY8XtVZCWKUkcyaTfJCQa2PuQsTBIjT6jOVQeNREkEadq1g5 eEjNMX4i5SAlDp2n3q/fFqgzN77NkJ2eGwP2dGUtSRTLIGEcrPtHhj1vwTvQBS3nuWN0 5XmHqWd92CJis8qoBVAPPA8TaglKzl3ZnjTKI3et+3lnReSdQwFtIjq+260hoNEjd7Kb 90r3HjBmQXeLkEmGmWPCwAyhVJOSl+PmEGD+KWglWe3cCpkbN8vD4FE3xM3FL6n+Ab16 2t7oZzC+NVqKigskAXucXRQL4ReiOQ2791IMMKcPZg6jRnckru/tMFjf+mPzaXRsyoyC mxvg== MIME-Version: 1.0 X-Received: by 10.107.137.162 with SMTP id t34mr31696620ioi.103.1442266415876; Mon, 14 Sep 2015 14:33:35 -0700 (PDT) Received: by 10.79.109.22 with HTTP; Mon, 14 Sep 2015 14:33:35 -0700 (PDT) In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC358AF3B7@smartserver.smartshare.dk> References: <20150909131037.GA25122@autoinstall.dev.6wind.com> <2699193.9riTyGPe1z@xps13> <CA+3n-TorPf4Kp3YsA3TpDpJ2pyqwQ_x3OVjA_2jm9_-q_8+uFA@mail.gmail.com> <2046894.c3eJ0QZGuc@xps13> <98CBD80474FA8B44BF855DF32C47DC358AF3B7@smartserver.smartshare.dk> Date: Mon, 14 Sep 2015 23:33:35 +0200 Message-ID: <CA+3n-TqTraUpWpRXN1RfZjijwDXJR0ovXkH8ZiH8PWOAzuYbEQ@mail.gmail.com> From: Marc Sune <marcdevel@gmail.com> To: =?UTF-8?Q?Morten_Br=C3=B8rup?= <mb@smartsharesystems.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Cc: dev@dpdk.org Subject: Re: [dpdk-dev] [PATCH v4 0/2] ethdev: add port speed capability bitmap X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> X-List-Received-Date: Mon, 14 Sep 2015 21:33:40 -0000 2015-09-14 12:52 GMT+02:00 Morten Br=C3=B8rup <mb@smartsharesystems.com>: > 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 a= ny > other logical link types when designing the PHY API. > +1 > > 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. > Agree. > > 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. Agree with Nelio&you. This is useful. > This support function will be able to determine which speed is higher whe= n > 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 th= e > duplex and auto-negotiation. > Since, Full/Half duplex is for legacy 10/100Mbps only (afaik), I have my doubts on using a bit for all speeds. I would suggest to define (unroll) 100M (or 100M_FD) and 100M_HD, and the same 10Mbps/1gbps, as Thomas was suggesting some mails ago. This was done in v4 (implicitely 100M =3D=3D 100M_FD). See below. > > 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 mean= s > 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). Agree. v3/4 of this patch adds the bitmap in the advertised, as per discussed, to select a group of speeds This is not implemented by drivers yet (!). So, as of v4 of this patch, there could be: a) autoneg any supported speed (=3D> bitmap =3D=3D 0) b) autoneg over group of speeds (=3D> more than one = bit set in the bitmap) c) forced speed (one and only one set in the bitmap). I think this is precisely what you meant + b) as a bonus > 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 t= wo > can be used at any time). > > For rte_eth_link_get() I don't have such a strong opinion. You either * encode the link speed and duplex as of now, separating duplex and numeric speed. I would suggest to add the encoded speed+duplex bitmap flag for consistency (although redundant). * or you return a single value, the bitmap with a single flag set of the unrolled speeds, and then have the helpers int rte_eth_speed_from_bm(int val_bm) and bool rte_eth_duplex_from_bm(int val_bm). Marc > > Med venlig hilsen / kind regards > - Morten Br=C3=B8rup > > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: 13. september 2015 23:19 > To: Marc Sune > Cc: N=C3=A9lio Laranjeiro; dev@dpdk.org; Olga Shern; Adrien Mazarguil; Mo= rten > Br=C3=B8rup > 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=C3=A9lio 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 speed= s. > > 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 usin= g > 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? > > >