[PATCH net v2] net: mvpp2: prs: Don't override the sign bit in SRAM parser shift

[PATCH net v2] net: mvpp2: prs: Don't override the sign bit in SRAM parser shift

[Maxime Chevallier]

The Header Parser allows identifying various fields in the packet
headers, used for various kind of filtering and classification
steps.

This is a re-entrant process, where the offset in the packet header
depends on the previous lookup results. This offset is represented in
the SRAM results of the TCAM, as a shift to be operated.

This shift can be negative in some cases, such as in IPv6 parsing.

This commit prevents overriding the sign bit when setting the shift
value, which could cause instabilities when parsing IPv6 flows.

Fixes: 3f518509dedc ("ethernet: Add new driver for Marvell Armada 375 network unit")
Suggested-by: Alan Winkowski <walan@marvell.com>
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
---
V2 : Fix a typo in the commit log, reported by Sergei.

 drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c
index ae2240074d8e..5692c6087bbb 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_prs.c
@@ -312,7 +312,8 @@ static void mvpp2_prs_sram_shift_set(struct mvpp2_prs_entry *pe, int shift,
 	}
 
 	/* Set value */
-	pe->sram[MVPP2_BIT_TO_WORD(MVPP2_PRS_SRAM_SHIFT_OFFS)] = shift & MVPP2_PRS_SRAM_SHIFT_MASK;
+	pe->sram[MVPP2_BIT_TO_WORD(MVPP2_PRS_SRAM_SHIFT_OFFS)] |=
+		shift & MVPP2_PRS_SRAM_SHIFT_MASK;
 
 	/* Reset and set operation */
 	mvpp2_prs_sram_bits_clear(pe, MVPP2_PRS_SRAM_OP_SEL_SHIFT_OFFS,
-- 
2.20.1

Re: [PATCH net v2] net: mvpp2: prs: Don't override the sign bit in SRAM parser shift

[Maxime Chevallier]

Hello David,

On Thu, 20 Jun 2019 11:42:45 +0200
Maxime Chevallier <maxime.chevallier@bootlin.com> wrote:

>The Header Parser allows identifying various fields in the packet
>headers, used for various kind of filtering and classification
>steps.
>
>This is a re-entrant process, where the offset in the packet header
>depends on the previous lookup results. This offset is represented in
>the SRAM results of the TCAM, as a shift to be operated.
>
>This shift can be negative in some cases, such as in IPv6 parsing.
>
>This commit prevents overriding the sign bit when setting the shift
>value, which could cause instabilities when parsing IPv6 flows.
>
>Fixes: 3f518509dedc ("ethernet: Add new driver for Marvell Armada 375 network unit")
>Suggested-by: Alan Winkowski <walan@marvell.com>
>Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
>---
>V2 : Fix a typo in the commit log, reported by Sergei.

I see that this patch was set as "Accepted" on patchwork, but hasn't
made it to -net, I was wondering if this patch slipped through the
cracks :)

https://patchwork.ozlabs.org/patch/1119311/

Thanks,

Maxime

Re: [PATCH net v2] net: mvpp2: prs: Don't override the sign bit in SRAM parser shift

[David Miller]

From: Maxime Chevallier <maxime.chevallier@bootlin.com>
Date: Tue, 25 Jun 2019 14:04:12 +0200

> I see that this patch was set as "Accepted" on patchwork, but hasn't
> made it to -net, I was wondering if this patch slipped through the
> cracks :)
> 
> https://patchwork.ozlabs.org/patch/1119311/

It should really be there now.

I don't know how that happened, honestly ;)