[v2,3/4] ocelot_ace: fix action of trap
diff mbox series

Message ID 20190813025214.18601-4-yangbo.lu@nxp.com
State Changes Requested
Delegated to: David Miller
Headers show
Series
  • ocelot: support PTP Ethernet frames trapping
Related show

Commit Message

Yangbo Lu Aug. 13, 2019, 2:52 a.m. UTC
The trap action should be copying the frame to CPU and
dropping it for forwarding, but current setting was just
copying frame to CPU.

Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
---
Changes for v2:
	- None.
---
 drivers/net/ethernet/mscc/ocelot_ace.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

allan.nielsen@microchip.com Aug. 13, 2019, 6:16 a.m. UTC | #1
The 08/13/2019 10:52, Yangbo Lu wrote:
> The trap action should be copying the frame to CPU and
> dropping it for forwarding, but current setting was just
> copying frame to CPU.
> 
> Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> ---
> Changes for v2:
> 	- None.
> ---
>  drivers/net/ethernet/mscc/ocelot_ace.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mscc/ocelot_ace.c b/drivers/net/ethernet/mscc/ocelot_ace.c
> index 91250f3..59ad590 100644
> --- a/drivers/net/ethernet/mscc/ocelot_ace.c
> +++ b/drivers/net/ethernet/mscc/ocelot_ace.c
> @@ -317,9 +317,9 @@ static void is2_action_set(struct vcap_data *data,
>  		break;
>  	case OCELOT_ACL_ACTION_TRAP:
>  		VCAP_ACT_SET(PORT_MASK, 0x0);
> -		VCAP_ACT_SET(MASK_MODE, 0x0);
> -		VCAP_ACT_SET(POLICE_ENA, 0x0);
> -		VCAP_ACT_SET(POLICE_IDX, 0x0);
> +		VCAP_ACT_SET(MASK_MODE, 0x1);
> +		VCAP_ACT_SET(POLICE_ENA, 0x1);
> +		VCAP_ACT_SET(POLICE_IDX, OCELOT_POLICER_DISCARD);
>  		VCAP_ACT_SET(CPU_QU_NUM, 0x0);
>  		VCAP_ACT_SET(CPU_COPY_ENA, 0x1);
>  		break;

This is still wrong, please see the comments provided the first time you
submitted this.

/Allan
allan.nielsen@microchip.com Aug. 13, 2019, 6:30 a.m. UTC | #2
The 08/13/2019 08:16, Allan W . Nielsen wrote:
> The 08/13/2019 10:52, Yangbo Lu wrote:
> > The trap action should be copying the frame to CPU and
> > dropping it for forwarding, but current setting was just
> > copying frame to CPU.
> > 
> > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> > ---
> > Changes for v2:
> > 	- None.
> > ---
> >  drivers/net/ethernet/mscc/ocelot_ace.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/mscc/ocelot_ace.c b/drivers/net/ethernet/mscc/ocelot_ace.c
> > index 91250f3..59ad590 100644
> > --- a/drivers/net/ethernet/mscc/ocelot_ace.c
> > +++ b/drivers/net/ethernet/mscc/ocelot_ace.c
> > @@ -317,9 +317,9 @@ static void is2_action_set(struct vcap_data *data,
> >  		break;
> >  	case OCELOT_ACL_ACTION_TRAP:
> >  		VCAP_ACT_SET(PORT_MASK, 0x0);
> > -		VCAP_ACT_SET(MASK_MODE, 0x0);
> > -		VCAP_ACT_SET(POLICE_ENA, 0x0);
> > -		VCAP_ACT_SET(POLICE_IDX, 0x0);
> > +		VCAP_ACT_SET(MASK_MODE, 0x1);
> > +		VCAP_ACT_SET(POLICE_ENA, 0x1);
> > +		VCAP_ACT_SET(POLICE_IDX, OCELOT_POLICER_DISCARD);
> >  		VCAP_ACT_SET(CPU_QU_NUM, 0x0);
> >  		VCAP_ACT_SET(CPU_COPY_ENA, 0x1);
> >  		break;
> 
> This is still wrong, please see the comments provided the first time you
> submitted this.
> 
> /Allan

I believe this will make it work - but I have not tested it:

 	case OCELOT_ACL_ACTION_TRAP:
 		VCAP_ACT_SET(PORT_MASK, 0x0);
-		VCAP_ACT_SET(MASK_MODE, 0x0);
+		VCAP_ACT_SET(MASK_MODE, 0x1);
 		VCAP_ACT_SET(CPU_QU_NUM, 0x0);
 		VCAP_ACT_SET(CPU_COPY_ENA, 0x1);
 		break;
Yangbo Lu Aug. 14, 2019, 4:35 a.m. UTC | #3
Hi Allan,

> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of Allan W . Nielsen
> Sent: Tuesday, August 13, 2019 2:30 PM
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: netdev@vger.kernel.org; David S . Miller <davem@davemloft.net>;
> Alexandre Belloni <alexandre.belloni@bootlin.com>; Microchip Linux Driver
> Support <UNGLinuxDriver@microchip.com>
> Subject: Re: [v2, 3/4] ocelot_ace: fix action of trap
> 
> The 08/13/2019 08:16, Allan W . Nielsen wrote:
> > The 08/13/2019 10:52, Yangbo Lu wrote:
> > > The trap action should be copying the frame to CPU and dropping it
> > > for forwarding, but current setting was just copying frame to CPU.
> > >
> > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> > > ---
> > > Changes for v2:
> > > 	- None.
> > > ---
> > >  drivers/net/ethernet/mscc/ocelot_ace.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/mscc/ocelot_ace.c
> > > b/drivers/net/ethernet/mscc/ocelot_ace.c
> > > index 91250f3..59ad590 100644
> > > --- a/drivers/net/ethernet/mscc/ocelot_ace.c
> > > +++ b/drivers/net/ethernet/mscc/ocelot_ace.c
> > > @@ -317,9 +317,9 @@ static void is2_action_set(struct vcap_data *data,
> > >  		break;
> > >  	case OCELOT_ACL_ACTION_TRAP:
> > >  		VCAP_ACT_SET(PORT_MASK, 0x0);
> > > -		VCAP_ACT_SET(MASK_MODE, 0x0);
> > > -		VCAP_ACT_SET(POLICE_ENA, 0x0);
> > > -		VCAP_ACT_SET(POLICE_IDX, 0x0);
> > > +		VCAP_ACT_SET(MASK_MODE, 0x1);
> > > +		VCAP_ACT_SET(POLICE_ENA, 0x1);
> > > +		VCAP_ACT_SET(POLICE_IDX, OCELOT_POLICER_DISCARD);
> > >  		VCAP_ACT_SET(CPU_QU_NUM, 0x0);
> > >  		VCAP_ACT_SET(CPU_COPY_ENA, 0x1);
> > >  		break;
> >
> > This is still wrong, please see the comments provided the first time
> > you submitted this.
> >
> > /Allan
> 
> I believe this will make it work - but I have not tested it:
> 
>  	case OCELOT_ACL_ACTION_TRAP:
>  		VCAP_ACT_SET(PORT_MASK, 0x0);
> -		VCAP_ACT_SET(MASK_MODE, 0x0);
> +		VCAP_ACT_SET(MASK_MODE, 0x1);
>  		VCAP_ACT_SET(CPU_QU_NUM, 0x0);
>  		VCAP_ACT_SET(CPU_COPY_ENA, 0x1);
>  		break;
> 

[Y.b. Lu] I will have a try. It seems more proper.
Thanks.

> --
> /Allan
Yangbo Lu Aug. 20, 2019, 4:23 a.m. UTC | #4
Hi,

> -----Original Message-----
> From: netdev-owner@vger.kernel.org <netdev-owner@vger.kernel.org> On
> Behalf Of Allan W . Nielsen
> Sent: Tuesday, August 13, 2019 2:30 PM
> To: Y.b. Lu <yangbo.lu@nxp.com>
> Cc: netdev@vger.kernel.org; David S . Miller <davem@davemloft.net>;
> Alexandre Belloni <alexandre.belloni@bootlin.com>; Microchip Linux Driver
> Support <UNGLinuxDriver@microchip.com>
> Subject: Re: [v2, 3/4] ocelot_ace: fix action of trap
> 
> The 08/13/2019 08:16, Allan W . Nielsen wrote:
> > The 08/13/2019 10:52, Yangbo Lu wrote:
> > > The trap action should be copying the frame to CPU and dropping it
> > > for forwarding, but current setting was just copying frame to CPU.
> > >
> > > Signed-off-by: Yangbo Lu <yangbo.lu@nxp.com>
> > > ---
> > > Changes for v2:
> > > 	- None.
> > > ---
> > >  drivers/net/ethernet/mscc/ocelot_ace.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/net/ethernet/mscc/ocelot_ace.c
> > > b/drivers/net/ethernet/mscc/ocelot_ace.c
> > > index 91250f3..59ad590 100644
> > > --- a/drivers/net/ethernet/mscc/ocelot_ace.c
> > > +++ b/drivers/net/ethernet/mscc/ocelot_ace.c
> > > @@ -317,9 +317,9 @@ static void is2_action_set(struct vcap_data *data,
> > >  		break;
> > >  	case OCELOT_ACL_ACTION_TRAP:
> > >  		VCAP_ACT_SET(PORT_MASK, 0x0);
> > > -		VCAP_ACT_SET(MASK_MODE, 0x0);
> > > -		VCAP_ACT_SET(POLICE_ENA, 0x0);
> > > -		VCAP_ACT_SET(POLICE_IDX, 0x0);
> > > +		VCAP_ACT_SET(MASK_MODE, 0x1);
> > > +		VCAP_ACT_SET(POLICE_ENA, 0x1);
> > > +		VCAP_ACT_SET(POLICE_IDX, OCELOT_POLICER_DISCARD);
> > >  		VCAP_ACT_SET(CPU_QU_NUM, 0x0);
> > >  		VCAP_ACT_SET(CPU_COPY_ENA, 0x1);
> > >  		break;
> >
> > This is still wrong, please see the comments provided the first time
> > you submitted this.
> >
> > /Allan
> 
> I believe this will make it work - but I have not tested it:
> 
>  	case OCELOT_ACL_ACTION_TRAP:
>  		VCAP_ACT_SET(PORT_MASK, 0x0);
> -		VCAP_ACT_SET(MASK_MODE, 0x0);
> +		VCAP_ACT_SET(MASK_MODE, 0x1);
>  		VCAP_ACT_SET(CPU_QU_NUM, 0x0);
>  		VCAP_ACT_SET(CPU_COPY_ENA, 0x1);
>  		break;
> 

[Y.b. Lu] It makes sense. And it worked.
I have sent out v3 which only included this one patch. I'd like to send the other patches once Felix driver is accepted, but I'd like to collect the suggestions :)
Thanks.

Sorry, I missed to add change logs for v3 patch...
https://patchwork.ozlabs.org/patch/1149770/


> --
> /Allan

Patch
diff mbox series

diff --git a/drivers/net/ethernet/mscc/ocelot_ace.c b/drivers/net/ethernet/mscc/ocelot_ace.c
index 91250f3..59ad590 100644
--- a/drivers/net/ethernet/mscc/ocelot_ace.c
+++ b/drivers/net/ethernet/mscc/ocelot_ace.c
@@ -317,9 +317,9 @@  static void is2_action_set(struct vcap_data *data,
 		break;
 	case OCELOT_ACL_ACTION_TRAP:
 		VCAP_ACT_SET(PORT_MASK, 0x0);
-		VCAP_ACT_SET(MASK_MODE, 0x0);
-		VCAP_ACT_SET(POLICE_ENA, 0x0);
-		VCAP_ACT_SET(POLICE_IDX, 0x0);
+		VCAP_ACT_SET(MASK_MODE, 0x1);
+		VCAP_ACT_SET(POLICE_ENA, 0x1);
+		VCAP_ACT_SET(POLICE_IDX, OCELOT_POLICER_DISCARD);
 		VCAP_ACT_SET(CPU_QU_NUM, 0x0);
 		VCAP_ACT_SET(CPU_COPY_ENA, 0x1);
 		break;