On Tue, 2 Jul 2019 04:26:47 +0000, Parav Pandit wrote:
> > On Mon, 1 Jul 2019 07:27:32 -0500, Parav Pandit wrote:
> > > In an eswitch, PCI PF may have port which is normally represented
> > > using a representor netdevice.
> > > To have better visibility of eswitch port, its association with PF, a
> > > representor netdevice and port number, introduce a PCI PF port flavour
> > > and port attriute.
> > >
> > > When devlink port flavour is PCI PF, fill up PCI PF attributes of the
> > > port.
> > >
> > > Extend port name creation using PCI PF number on best effort basis.
> > > So that vendor drivers can skip defining their own scheme.
> > >
> > > $ devlink port show
> > > pci/0000:05:00.0/0: type eth netdev eth0 flavour pcipf pfnum 0
> > >
> > > Acked-by: Jiri Pirko <j...@mellanox.com>
> > > Signed-off-by: Parav Pandit <pa...@mellanox.com>
> > > diff --git a/include/net/devlink.h b/include/net/devlink.h index
> > > 6625ea068d5e..8db9c0e83fb5 100644
> > > --- a/include/net/devlink.h
> > > +++ b/include/net/devlink.h
> > > @@ -38,6 +38,10 @@ struct devlink {
> > > char priv[0] __aligned(NETDEV_ALIGN); };
> > >
> > > +struct devlink_port_pci_pf_attrs {
> >
> > Why the named structure? Anonymous one should be just fine?
> >
> No specific reason for this patch. But named structure allows to
> extend it more easily with code readability.
I'd argue the readability - I hove to scroll up/look up the structure
just to see it has a single member. But no big deal :)
> Such as subsequently we want to add the peer_mac etc port attributes.
> Named structure to store those attributes are helpful.
It remains to be seen if peer attributes are flavour specific 🤔
I'd imagine most port types would have some form of a peer (other
than a network port, perhaps). But perhaps different peer attributes.
> > > diff --git a/net/core/devlink.c b/net/core/devlink.c index
> > > 89c533778135..001f9e2c96f0 100644
> > > --- a/net/core/devlink.c
> > > +++ b/net/core/devlink.c
> > > @@ -517,6 +517,11 @@ static int devlink_nl_port_attrs_put(struct sk_buff
> > > *msg,
> > > return -EMSGSIZE;
> > > if (nla_put_u32(msg, DEVLINK_ATTR_PORT_NUMBER, attrs->port_number))
> > > return -EMSGSIZE;
> >
> > Why would we report network port information for PF and VF port
> > flavours?
>
> I didn't see any immediate need to report, at the same time didn't
> find any reason to treat such port flavours differently than existing
> one. It just gives a clear view of the device's eswitch. Might find
> it useful during debugging while inspecting device internal tables..
PFs and VFs ports are not tied to network ports in switchdev mode.
You have only one network port under a devlink instance AFAIR, anyway.
> > > + if (devlink_port->attrs.flavour == DEVLINK_PORT_FLAVOUR_PCI_PF) {
> > > + if (nla_put_u16(msg, DEVLINK_ATTR_PORT_PCI_PF_NUMBER,
> > > + attrs->pci_pf.pf))
> > > + return -EMSGSIZE;
> > > + }
> > > if (!attrs->split)
> > > return 0;
> > > if (nla_put_u32(msg, DEVLINK_ATTR_PORT_SPLIT_GROUP,
> > > attrs->port_number))
