netdev - Re: [RFC] switchdev: clarify ndo_get_phys_port

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170726131146.45fe5b3e@cakuba.netronome.com>
Date:   Wed, 26 Jul 2017 13:11:46 -0700
From:   Jakub Kicinski <kubakici@...pl>
To:     Or Gerlitz <gerlitz.or@...il.com>
Cc:     Jiri Pirko <jiri@...nulli.us>,
        Linux Netdev List <netdev@...r.kernel.org>,
        Or Gerlitz <ogerlitz@...lanox.com>,
        Michael Chan <michael.chan@...adcom.com>,
        Sathya Perla <sathya.perla@...adcom.com>,
        Simon Horman <simon.horman@...ronome.com>,
        David Miller <davem@...emloft.net>
Subject: Re: [RFC] switchdev: clarify ndo_get_phys_port_name() formats

On Wed, 26 Jul 2017 12:23:17 +0300, Or Gerlitz wrote:
> On Wed, Jul 26, 2017 at 11:13 AM, Jakub Kicinski <kubakici@...pl> wrote:
> > On Wed, 26 Jul 2017 07:48:40 +0200, Jiri Pirko wrote:  
> 
> >> I think it would make sense if the driver would just fill-up a struct in
> >> the ndo call and core would generate the string.  
> 
> > I do like the idea of core generating the string.  I have a temptation
> > to try to involve devlink in this somehow, since port id and split info
> > is already available there.  Perhaps that would only overcomplicate
> > things.  
> 
> > The other question is: can we remove the option to generate an arbitrary
> > string completely?  I think Or and mlx5 folks may object since it would
> > mean mlx5 VF repr names would change.  
> 
> What we have today is the representor driver instance setting the VF index as
> the of phys port name and we're telling users to have this sort of udev rule:
> 
> SUBSYSTEM=="net", ACTION=="add",
> ATTR{phys_switch_id}=="<phys_switch_id>", \ ATTR{phys_port_name}!="",
> NAME="$PF_NIC$attr{phys_port_name}"

Example names generated by this rule would be pfnic_0, pfnic_1 for vf
representors 0 and 1?

> that has affiliation to  the PF where this VF belongs. AFAIK this
> model/assumption is now under push to
> higher level (open stack), so lets see if/what we want to change here
> w.r.t to sriov offloading drivers.
> 
> I would opt for an approach where the value returned by the kernel is
> the minimal possible and
> user-space has flexibility to further orchestrate that with udev
> rules. I wasn't fully clear on Jakub's suggestion
> which parts must come from the kernel. Do we have any length
> limitation for the phys port name?

Looks like the limit today is IFNAMSIZ.  I'm in favor of leaving the
flexibility to the userspace, why I suggested adding the pf%d or
pf%dvf%d to the name is that I don't think we have any other way today
of telling whether representor is for a physical port, VF or PF.

If I understand mlx5 code, you're not reporting port ids for physical
ports so presence of the name already implies it's a VF but in case you
want to add port splitting support, for example, reporting the name on
physical ports will become more of a necessity.

If we adopt Jiri's suggestion of returning structured data it will be
very easy to give user space type and indexes separately, but we should
probably still return the string for backwards compatibility.