Thread (2 messages) 2 messages, 2 authors, 2023-08-31

RE: [PATCH ethtool] rxclass: fix a bug in rmgr when searching for empty slot

From: Alexander Duyck <hidden>
Date: 2023-08-31 15:19:13

quoted hunk ↗ jump to hunk
-----Original Message-----
From: Yinjun Zhang <redacted>
Sent: Wednesday, August 30, 2023 7:28 PM
To: mkubecek@suse.cz
Cc: oss-drivers@corigine.com; netdev@vger.kernel.org; Yinjun Zhang
[off-list ref]; Alexander Duyck
[off-list ref]; Niklas Söderlund
[off-list ref]
Subject: [PATCH ethtool] rxclass: fix a bug in rmgr when searching for empty
slot

When reverse searching the list in rmgr for a free location the last slot (first
slot searched) in the list needs special care as it might not span the full word
length. This is done by building a bit-mask covering the not-active parts of the
last word and using that to judge if there is a free location in the last word or
not. Once that is known searching in the last slot, or to skip it, can be done by
the same algorithm as for the other slots in the list.

There is a bug in creating the bit-mask for the non-active parts of the last slot
where the 0-indexed nature of bit addressing is not taken into account when
shifting. This leads to a one-off bug, fix it.

Fixes: 8d63f72ccdcb ("Add RX packet classification interface")
Signed-off-by: Yinjun Zhang <redacted>
Cc: Alexander Duyck <alexanderduyck@fb.com>
Reviewed-by: Niklas Söderlund <redacted>
---
 rxclass.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/rxclass.c b/rxclass.c
index 66cf00ba7728..b2471f807d5d 100644
--- a/rxclass.c
+++ b/rxclass.c
@@ -446,7 +446,7 @@ static int rmgr_find_empty_slot(struct rmgr_ctrl
*rmgr,
 	 * If loc rolls over it should be greater than or equal to rmgr->size
 	 * and as such we know we have reached the end of the list.
 	 */
-	if (!~(rmgr->slot[slot_num] | (~1UL << rmgr->size %
BITS_PER_LONG))) {
+	if (!~(rmgr->slot[slot_num] | (~1UL << (rmgr->size - 1) %
+BITS_PER_LONG))) {
 		loc -= 1 + (loc % BITS_PER_LONG);
 		slot_num--;
 	}
Rather than use "rmgr->size - 1" you might just use "loc" there since it is the same value. It would also help to make it a bit more obvious that "loc" here represents number of bits from 0 in this slot and that we are assuming 1 bit will always be in play since the mask is ~1UL and not ~0UL.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help