netdev.vger.kernel.org archive mirror
 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 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).