RE: [EXT] Re: [PATCH v1 net-next 1/3] net: dsa: felix: qos classified based on pcp

[Xiaoliang Yang]

Hi Vladimir,

On Mon, 11 May 2020 at 08:19, Vladimir Oltean <vladimir.oltean@xxxxxxx> wrote:
> 
> The new skbedit priority offload action looks interesting to me.
> But it also raises the question of what to do in the default case where such rules are not installed. I think it is ok to support a
> 1-to-1 VLAN PCP to TC mapping by default? This should also be needed for features such as Priority Flow Control.

skbedit priority offload seems only support port based prority set now, I haven't found how to set a priority for each port and QoS. So I set a 1-to-1 VLAN PCP to TC mapping by default.

> Xiaoliang, just a small comment in case you need to resend.
> The felix->info structure is intended to hold SoC-specific data that 
> is likely to differ between chips (like for example if vsc7511 support 
> ever appears in felix). But I see ANA:PORT:QOS_CFG and 
> ANA:PORT:QOS_PCP_DEI_MAP_CFG are common registers, so are there any 
> specific reasons why you put this in felix_vsc9959 and not in the 
> common ocelot code?

All right, I have checked they are common registers, I will move port_qos_map_init() function to felix.c.

> > +       for (i = 0; i < FELIX_NUM_TC * 2; i++) {
> > +               ocelot_rmw_ix(ocelot,
> > +                             (ANA_PORT_PCP_DEI_MAP_DP_PCP_DEI_VAL & 
> > + i) |
> > +								 ANA_PORT_PCP_DEI_MAP_QOS_PCP_DEI_VAL(i),
> > +                             ANA_PORT_PCP_DEI_MAP_DP_PCP_DEI_VAL |
> > +                             ANA_PORT_PCP_DEI_MAP_QOS_PCP_DEI_VAL_M,
> > +                             ANA_PORT_PCP_DEI_MAP,
> > +                             port, i);
> 
> ANA_PORT_PCP_DEI_MAP_DP_PCP_DEI_VAL is 1 bit. Are you sure this should be % i and not % 2?

Because in QOS_PCP_DEI_MAP_CFG register, BIT(3) is DP value, BIT(2, 0) is QOS value. QoS class=QOS_PCP_DEI_MAP_CFG[i].QOS_PC
P_DEI_VAL, i=8*DEI + PCP, so DP value need to be set BIT(3)&i.

Regards,
Xiaoliang Yang