All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Gaëtan Rivet" <gaetan.rivet@6wind.com>
To: Thomas Monjalon <thomas@monjalon.net>
Cc: Yuanhan Liu <yliu@fridaylinux.org>,
	Ferruh Yigit <ferruh.yigit@intel.com>,
	dev@dpdk.org
Subject: Re: [PATCH] doc: document the new devargs syntax
Date: Tue, 23 Jan 2018 18:37:08 +0100	[thread overview]
Message-ID: <20180123173708.yj6fc2uibmjpxplh@bidouze.vm.6wind.com> (raw)
In-Reply-To: <3336592.DqqRBrUkFp@xps>

On Tue, Jan 23, 2018 at 06:22:53PM +0100, Thomas Monjalon wrote:
> 23/01/2018 17:08, Gaëtan Rivet:
> > Hi Yuanhan, Thomas,
> > 
> > On Tue, Jan 23, 2018 at 03:29:34PM +0100, Thomas Monjalon wrote:
> > > 23/01/2018 13:46, Yuanhan Liu:
> > > > On Thu, Jan 18, 2018 at 10:46:23AM +0100, Gaëtan Rivet wrote:
> > > > > On Thu, Jan 18, 2018 at 09:46:29AM +0100, Thomas Monjalon wrote:
> > > > > > 18/01/2018 08:35, Yuanhan Liu:
> > > > > > > On Wed, Jan 17, 2018 at 12:34:08PM +0000, Ferruh Yigit wrote:
> > > > > > > > So does it make sense to separate them logically? Perhaps as "device identifier"
> > > > > > > > and "device args".
> > > > > > > 
> > > > > > > Then I think it returns back to the old issue: how could we identify a
> > > > > > > port when the bus id (say BDF for PCI bus) is not enough for identifying
> > > > > > > a port? Such case could happen when a single NIC has 2 ports sharing
> > > > > > > the same BDF. It could also happen with the VF representors that will
> > > > > > > be introduced shortly.
> > > > > > 
> > > > > > Yes, the device matching syntax must include bus category, class category
> > > > > > and driver category. So any device can be identified in future.
> > > > > > 
> > > > > > But I think Ferruh is talking about separating device matching
> > > > > > (which is described in this proposal) and device settings
> > > > > > (which are usually mixed in -w and --vdev options).
> > > > > > I agree there are different things and may be separate.
> > > > > > They could share the same syntax (bus/class/driver) but be separate
> > > > > > with a semicolon:
> > > > > > 	matching;settings
> > > > >
> > > > > Can you give an example?
> > > > 
> > > > Let's take port addition in OVS-DPDK as an example. It happens in 2
> > > > steps:
> > > > - port lookup (if port is already probed)
> > > > - dev attachment (if lookup fails)
> > > > 
> > > > And also let's assume we need probe a ConnectX-3 port. Note that for
> > > > ConnectX-3, there are 2 ports sharing the same PCI addr. Thus, PCI
> > > > BDF is not enough. And let's assume we use another extra property
> > > > "port".
> > > > 
> > > > If the proposal described in this patch is being used, the devarg
> > > > would look like following:
> > > > 
> > > >     bus=pci,id=04:00.0/class=eth,port=0/driver=mlx4,mlx4_arg_A=val,...
> > > > 
> > > > Then "bus=pci,id=04:00.0/class=eth,port=0" will be used for lookup,
> > > > It means we are looking for a port with PCI BDF == 04:00.0 AND
> > > > port == 0 (the first port of the 2 ports).
> > > > 
> > > > Note that in my proposal the driver category is not intended for lookup.
> > > > If any properties needed be looked in the driver category, they would
> > > > probably need be elevated to the class category.
> > > 
> > > It is not my thought.
> > > I think we should be able to use bus, class and driver properties for lookup.
> > > We can imagine doing a lookup on a driver specific id, which is not
> > > candidate to elevation to the class category.
> > 
> > This means having a new set of ops for drivers to implement (get / set
> > on specific properties -- configuration items).
> 
> Just new ops to parse the string.
> Then the driver is free to do whatever he wants internally.
> 
> > > > If port not found, then the whole string will be used for dev attachment.
> > > > It means we are attaching a port with PCI BDF == 04.00.0 AND
> > > > port == 0 (the 2nd port will not be attached).
> > > > 
> > > > 
> > > > And here is how the devargs would look like if "matching;settings" is
> > > > being used:
> > > > 
> > > >     bus=pci,id=04:00.0/class=eth,port=0;bus=pci,id=04:00.0/class=eth,port=0/driver=mlx4,mlx4_arg_A=val,...
> > > > 
> > > > The part before ";" will be used for lookup and the later part will be
> > > > used for attachment. It should work. It just looks redundant.
> > > 
> > > It does not have to be redundant.
> > > It can be:
> > > 	bus=pci,id=04:00.0/class=eth,port=0;driver=mlx4,mlx4_arg1=settings1,...
> > > 
> > 
> > Did you mean
> > 
> > > 	bus=pci,id=04:00.0/class=eth,port=0;class=driver,name=mlx4,mlx4_arg1=settings1,...
> 
> No :)
> There are 3 categories: bus, class and driver.
> class is for eth, crypto, event, etc.
> 

ah yes, mixed up.
It makes more sense then.

> > Here? Or is it that you "elevated" driver to be a property of the eth
> > class, and then immediately chained with driver parameters without
> > declaring the new driver class?
> 
> No I think you misunderstand.
> I re-use the same syntax for matching and settings.
> Overview is: bus/class/driver;bus/class/driver
> where first part is for matching, and second part is for settings if any.
> Another overview is: matching;settings
> 

Sure, with the mixup above straightened it's clear.

> > > Another example, setting the MAC address:
> > > 	bus=pci,id=04:00.0/class=eth,port=0;class=eth,mac=00:11:22:33:44:55
> > 
> > So, I guess this ";" syntax is meant for a user to provide once and for
> > all a device string: perhaps on the command line, or programmatically.
> > It would be used first for EAL init, then reused as-is (the entire
> > string) for lookup / port matching afterward.
> > 
> > I think this is forcing the user to keep in mind a logic that should be
> > abstracted away ("Here I am writing for init time, here I am writing for
> > matching -- but I need to put it at the same place for 'reasons'").
> > 
> > I think mashing those two concepts together introduce complexity, and I
> > think keeping them separate is user hostile as the devargs that was used
> > for initializing a device cannot be re-used afterward for matching the
> > device that resulted from this initialization string.
> > 
> > Drivers answers to a specific API (ethdev, cryptodev, ...), to create
> > standardized objects in response to parameters that are given to them
> > for init. I think matching properties should be restricted to higher
> > classes (bus, eth/crypto), while the driver class should be left
> > free-form and to the responsibility of the PMD itself (while having the
> > proper libraries for helping parsing safely, thus driving developpers
> > toward similar syntaxes, while not forcing them in those).
> > 
> > Match could be performed on bus / eth classes only, while init could
> > use elements of the three classes. For simplicity, the same syntax rules
> > could be enforced at all level, or for flexibility some leeway could be
> > left on the most specific (driver).
> 
> I think it is more generic to allow bus/class/driver for matching
> and for settings.

Yes. When you propose

> > > 	bus=pci,id=04:00.0/class=eth,port=0;driver=mlx4,mlx4_arg1=settings1,...

As an example: what I was thinking about was along those lines. "bus"
and "class" used for matching, "driver" used only for init.

So your proposition is more flexible, but I do not see how "driver" will
be used for matching. I guess I'll see.

-- 
Gaëtan Rivet
6WIND

  reply	other threads:[~2018-01-23 17:37 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-16 14:50 [PATCH] doc: document the new devargs syntax Yuanhan Liu
2018-01-16 16:33 ` Mcnamara, John
2018-01-16 23:19 ` Gaëtan Rivet
2018-01-16 23:22   ` Thomas Monjalon
2018-01-16 23:46     ` Gaëtan Rivet
2018-01-17  0:03       ` Thomas Monjalon
2018-01-17  9:37         ` Gaëtan Rivet
2018-01-17  9:43           ` Thomas Monjalon
2018-01-17 10:11 ` Gaëtan Rivet
2018-01-17 10:54   ` Thomas Monjalon
2018-01-17 12:34 ` Ferruh Yigit
2018-01-18  7:35   ` Yuanhan Liu
2018-01-18  8:46     ` Thomas Monjalon
2018-01-18  9:46       ` Gaëtan Rivet
2018-01-23 12:46         ` Yuanhan Liu
2018-01-23 14:29           ` Thomas Monjalon
2018-01-23 16:08             ` Gaëtan Rivet
2018-01-23 17:22               ` Thomas Monjalon
2018-01-23 17:37                 ` Gaëtan Rivet [this message]
2018-01-23 18:12                   ` Thomas Monjalon
2018-01-24 15:24               ` Yuanhan Liu
2018-01-24 16:51                 ` Thomas Monjalon
2018-01-24  6:43             ` Yuanhan Liu
2018-01-24  8:19               ` Thomas Monjalon
2018-01-24  9:28                 ` Yuanhan Liu
2018-01-24 10:21                   ` Thomas Monjalon
2018-01-24 10:36                     ` Yuanhan Liu
2018-01-24 10:37                       ` Thomas Monjalon
2018-01-24 15:04                         ` Yuanhan Liu
2018-01-24 16:57                           ` Thomas Monjalon
2018-01-25 14:41                             ` Yuanhan Liu
2018-01-25 14:58                               ` Thomas Monjalon
2023-06-08 22:51 ` Stephen Hemminger

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=20180123173708.yj6fc2uibmjpxplh@bidouze.vm.6wind.com \
    --to=gaetan.rivet@6wind.com \
    --cc=dev@dpdk.org \
    --cc=ferruh.yigit@intel.com \
    --cc=thomas@monjalon.net \
    --cc=yliu@fridaylinux.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.