On Tue, Feb 09, 2021 at 08:11:15PM -0800, Florian Fainelli wrote:
>
>
> On 2/9/2021 5:02 PM, George McCollister wrote:
> > Add offloading for HSR/PRP (IEC 62439-3) tag insertion, tag removal
> > forwarding and duplication supported by the xrs7000 series switches.
> >
> > Only HSR v1 and PRP v1 are supported by the xrs7000 series switches (HSR
> > v0 is not).
> >
> > Signed-off-by: George McCollister <george.mccollis...@gmail.com>
> > ---
> [snip]
> > + val &= ~BIT(dsa_upstream_port(ds, port));
> > + regmap_write(priv->regmap, XRS_PORT_FWD_MASK(partner->index), val);
> > + regmap_write(priv->regmap, XRS_PORT_FWD_MASK(port), val);
> > +
> > + regmap_fields_write(priv->ps_forward, partner->index,
> > + XRS_PORT_FORWARDING);
> > + regmap_fields_write(priv->ps_forward, port, XRS_PORT_FORWARDING);
> > +
> > + hsr_pair[0] = port;
> > + hsr_pair[1] = partner->index;
> > + for (i = 0; i < ARRAY_SIZE(hsr_pair); i++) {
> > + slave = dsa_to_port(ds, hsr_pair[i])->slave;
> > + slave->features |= XRS7000X_SUPPORTED_HSR_FEATURES;
>
> It's a bit weird to change the supported features while joining, usually
> you set them ahead of time to indicate what you are capable of doing and
> those can get toggled by user-space to enable/disable said feature, I
> suppose the goal here is to influence the HSR data path's decisions to
> insert or not tags so this may be okay. This does beg several questions:
>
> - should slave->vlan_features also include that feature set somehow (can
> I have a VLAN upper?)
hsr_check_dev_ok:
if (is_vlan_dev(dev)) {
NL_SET_ERR_MSG_MOD(extack, "HSR on top of VLAN is not yet
supported in this driver.");
return -EINVAL;
}
> - should there be a notifier running to advertise NETDEV_FEAT_CHANGE?
I felt it's a bit weird too to toggle the netdev flags just like that
instead of just enabling them at probe time or something (or have DSA
set them in dsa_slave_create(), just as it currently checks
ds->ops->port_vlan_add), but since there's no need for anyone to process
that notification, and there don't appear to be any strict guidelines, I
didn't say anything. I guess the current code is just fine for what is
needed at the moment.
Reviewed-by: Vladimir Oltean <olte...@gmail.com>
