All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Fastabend <john.fastabend@gmail.com>
To: Narendra_K@Dell.com
Cc: Ben Hutchings <bhutchings@solarflare.com>,
	netdev@vger.kernel.org, john.r.fastabend@intel.com
Subject: Re: [PATCH net-next] net: Add phys_port identifier to struct net_device and export it to sysfs
Date: Fri, 21 Jun 2013 10:11:07 -0700	[thread overview]
Message-ID: <51C4892B.8000806@gmail.com> (raw)
In-Reply-To: <1371670481.1956.105.camel@bwh-desktop.uk.level5networks.com>

On 06/19/2013 12:34 PM, Ben Hutchings wrote:
> On Thu, 2013-06-20 at 00:23 +0530, Narendra_K@Dell.com wrote:
>>> -----Original Message-----
>>> From: Ben Hutchings [mailto:bhutchings@solarflare.com]
>>> Sent: Wednesday, June 19, 2013 9:07 PM
>>> To: K, Narendra
>>> Cc: john.fastabend@gmail.com; netdev@vger.kernel.org;
>>> john.r.fastabend@intel.com
>>> Subject: Re: [PATCH net-next] net: Add phys_port identifier to struct
>>> net_device and export it to sysfs
>>>
>>> On Wed, 2013-06-19 at 07:29 -0700, Narendra_K@Dell.com wrote:
>>> [...]
>>>> 2. show_phys_port  function sees a consistent value of
>>>> 'netdev->phys_port.port_id and netdev->phys_port.port_id_len '  if
>>>> another execution path changes the value of 'netdev->phys_port.port_id
>>>> and netdev->phys_port.port_id_len '  with write_lock(&dev_base_lock)
>>>> held (similar to how dev->operstate is being changed).
>>> [...]
>>>
>>> If the physical port ID can change dynamically (I hadn't thought of that, but an
>>> embedded switch could support such reconfiguration) then any such change
>>> also needs to be announced through rtnetlink.  Actually, I think the value
>>> needs to be included in rtnetlink information anyway.
>>>
>>
>> Ok. Thank you Ben. I had not thought about this scenario.  I was
>> thinking about the reason to hold the dev_base_lock.  Do you think
>> points 1 and 2 are correct reason to hold the dev_base_lock ?
>
> I think so.
>
>> If correct,  I think the 'show_broadcast' function also needs to be
>> fixed as it is not holding the lock.
>
> I think the broadcast address should never change during the lifetime of
> a device, so it doesn't need the lock.  That might not be true for all
> layer 2 protocols though.
>
> Ben.
>

Also, do you think this will be primarily useful for partitioning
devices that expose multiple physical functions? Or do you see
a use case for SR-IOV with virtual functions as well. The pyhs_port
attribute provides a common interface for both cases which is good I
suppose in the VF case however the host can already learn this. I
gather from your original post here that you are aware of all this.

quoting:
> I was thinking about the scenario of VF0 and VF1 coming from PF0 in the host
> Network Controller 1 being direct assigned to a KVM guest via VTD and netdevices
> from VF0 and VF1 being bonded in the guest. Assuming that physical port number used
> by VF0 and VF1 is 1, additional information is needed to identify if port number 1
> is on Network controller 1 or Network controller 2. (In the host we could use
> PCI b/d/f to differentiate two Network Controllers). I think it is similar to
> hybrid guest acceleration on the VF assignment aspect.

I'm curious though why would the host/libvirt assign two VFs from the
same PF to a guest like this? Is this really a host mis-configuration
that you want a way to detect in the guest?

Thanks,
John

-- 
John Fastabend         Intel Corporation

  parent reply	other threads:[~2013-06-21 17:11 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-06-17 18:10 [PATCH net-next] net: Add phys_port identifier to struct net_device and export it to sysfs Narendra_K
2013-06-17 18:47 ` John Fastabend
2013-06-19 14:29   ` Narendra_K
2013-06-19 15:36     ` Ben Hutchings
2013-06-19 18:53       ` Narendra_K
2013-06-19 19:34         ` Ben Hutchings
2013-06-19 21:37           ` Praveen_Paladugu
2013-06-21 17:11           ` John Fastabend [this message]
2013-06-25 17:33             ` Narendra_K
2013-06-28 16:33               ` John Fastabend
2013-06-28 17:09                 ` Ben Hutchings
2013-07-02 14:40                   ` Narendra_K
2013-07-11 20:39 ` Jiri Pirko
2013-07-15 15:34   ` Narendra_K
2013-07-21  5:55     ` Or Gerlitz
2013-07-21  7:24       ` Jiri Pirko
2013-07-21 11:14         ` Or Gerlitz
2013-07-21 14:48           ` Ben Hutchings
2013-07-21 20:29             ` Or Gerlitz
2013-07-21 20:50               ` Ben Hutchings
2013-07-22 11:46             ` Narendra_K
2013-07-22 11:49               ` Jiri Pirko
2013-07-22 15:48                 ` Or Gerlitz

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=51C4892B.8000806@gmail.com \
    --to=john.fastabend@gmail.com \
    --cc=Narendra_K@Dell.com \
    --cc=bhutchings@solarflare.com \
    --cc=john.r.fastabend@intel.com \
    --cc=netdev@vger.kernel.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.