On Tue, Jul 25, 2017 at 10:15 AM, Jakub Kicinski <kubak...@wp.pl> wrote:
...
>> +static int bnxt_get_phys_port_name(struct net_device *dev, char *buf,
>> + size_t len)
>> +{
>> + struct bnxt *bp = netdev_priv(dev);
>> + int rc;
>> +
>> + /* The PF and it's VF-reps only support the switchdev framework */
>> + if (!BNXT_PF(bp))
>> + return -EOPNOTSUPP;
>> +
>> + /* The switch-id that the pf belongs to is exported by
>> + * the switchdev ndo. This name is just to distinguish from the
>> + * vf-rep ports.
>> + */
>> + rc = snprintf(buf, len, "pf%d", bp->pf.port_id);
>
> This is worrisome. What do you mean by PF? In my experience, since
> for years PFs had one-to-one association with physical ports people use
> the terms interchangeably. This causes a lot of headaches in proper
> eswitch modelling.
>
> I'm not sure whether this is a HW limitation or engineers trying to
> make things "easy for users" but most VF-representor patchsets we've
> seen on netdev don't include PF representors. NICs have 3 types of
> ports - PFs, VFs and external ports/MACs. For some reason there is a
> tendency to keep pretending PF and physical port is the same entity,
> which among other things makes it impossible to install VF->PF rules
> (as in from VF to the host, not to the network).
>
> Most high performance NIC vendors also have multi-PF NICs, even though
> those are not deployed by wider public. If we keep pretending PF is
> the external port, those will get very awkward to model. This is a bit
> of a pet peeve of mine :)
Jakub, we'll consider implementing a PF-rep as an add-on feature...
>
> If this bnxt_get_phys_port_name() relates to the external port, please
> change your implementation to comply with
> Documentation/networking/switchdev.txt, in particular:
>
>> Suggested naming convention is "swXpYsZ", where X is the switch name or ID, Y
>> is the port name or ID, and Z is the sub-port name or ID. For example,
>> sw1p1s0
>> would be sub-port 0 on port 1 on switch 1.
>
> So for physical ports the convention is p<port_id>, and in case of
> breakout p<port_id>s<subport_id>.
I'm sending a follow-up patch that fixes the naming for both the
external port and the VF-reps as proposed in your RFC ( switchdev:
clarify ndo_get_phys_port_name() formats)
thanks,
-Sathya
