netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next v2 0/5] net: xilinx: axienet: Multicast fixes and improvements
@ 2024-08-15 19:36 Sean Anderson
  2024-08-15 19:36 ` [PATCH net-next v2 1/5] net: xilinx: axienet: Always disable promiscuous mode Sean Anderson
                   ` (4 more replies)
  0 siblings, 5 replies; 14+ messages in thread
From: Sean Anderson @ 2024-08-15 19:36 UTC (permalink / raw)
  To: Radhey Shyam Pandey, netdev
  Cc: David S . Miller, Andrew Lunn, linux-arm-kernel, Michal Simek,
	Daniel Borkmann, linux-kernel, Paolo Abeni, Jakub Kicinski,
	Eric Dumazet, Sean Anderson

This series has a few small patches improving the handling of multicast
addresses. In particular, it makes the driver a whole lot less spammy,
and adjusts things so we aren't in promiscuous mode when we have more
than four multicast addresses (a common occurance on modern systems).

As the hardware has a 4-entry CAM, the ideal method would be to "pack"
multiple addresses into one CAM entry. Something like:

entry.address = address[0] | address[1];
entry.mask = ~(address[0] ^ address[1]);

Which would make the entry match both addresses (along with some others
that would need to be filtered in software).

Mapping addresses to entries in an efficient way is a bit tricky. If
anyone knows of an in-tree example of something like this, I'd be glad
to hear about it.

Changes in v2:
- Split off IFF_PROMISC change from printing changes

Sean Anderson (5):
  net: xilinx: axienet: Always disable promiscuous mode
  net: xilinx: axienet: Fix dangling multicast addresses
  net: xilinx: axienet: Don't print if we go into promiscuous mode
  net: xilinx: axienet: Don't set IFF_PROMISC in ndev->flags
  net: xilinx: axienet: Support IFF_ALLMULTI

 drivers/net/ethernet/xilinx/xilinx_axienet.h  |  3 ++
 .../net/ethernet/xilinx/xilinx_axienet_main.c | 52 +++++++++----------
 2 files changed, 29 insertions(+), 26 deletions(-)

-- 
2.35.1.1320.gc452695387.dirty


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH net-next v2 1/5] net: xilinx: axienet: Always disable promiscuous mode
  2024-08-15 19:36 [PATCH net-next v2 0/5] net: xilinx: axienet: Multicast fixes and improvements Sean Anderson
@ 2024-08-15 19:36 ` Sean Anderson
  2024-08-20  1:30   ` Jakub Kicinski
  2024-08-15 19:36 ` [PATCH net-next v2 2/5] net: xilinx: axienet: Fix dangling multicast addresses Sean Anderson
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Sean Anderson @ 2024-08-15 19:36 UTC (permalink / raw)
  To: Radhey Shyam Pandey, netdev
  Cc: David S . Miller, Andrew Lunn, linux-arm-kernel, Michal Simek,
	Daniel Borkmann, linux-kernel, Paolo Abeni, Jakub Kicinski,
	Eric Dumazet, Sean Anderson, Simon Horman

If promiscuous mode is disabled when there are fewer than four multicast
addresses, then it will to be reflected in the hardware. Fix this by
always clearing the promiscuous mode flag even when we program multicast
addresses.

Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")
Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
Reviewed-by: Simon Horman <horms@kernel.org>
---

(no changes since v1)

 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index ca04c298daa2..e664611c29cf 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -451,6 +451,10 @@ static void axienet_set_multicast_list(struct net_device *ndev)
 	} else if (!netdev_mc_empty(ndev)) {
 		struct netdev_hw_addr *ha;
 
+		reg = axienet_ior(lp, XAE_FMI_OFFSET);
+		reg &= ~XAE_FMI_PM_MASK;
+		axienet_iow(lp, XAE_FMI_OFFSET, reg);
+
 		i = 0;
 		netdev_for_each_mc_addr(ha, ndev) {
 			if (i >= XAE_MULTICAST_CAM_TABLE_NUM)
-- 
2.35.1.1320.gc452695387.dirty


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next v2 2/5] net: xilinx: axienet: Fix dangling multicast addresses
  2024-08-15 19:36 [PATCH net-next v2 0/5] net: xilinx: axienet: Multicast fixes and improvements Sean Anderson
  2024-08-15 19:36 ` [PATCH net-next v2 1/5] net: xilinx: axienet: Always disable promiscuous mode Sean Anderson
@ 2024-08-15 19:36 ` Sean Anderson
  2024-08-20  1:33   ` Jakub Kicinski
  2024-08-15 19:36 ` [PATCH net-next v2 3/5] net: xilinx: axienet: Don't print if we go into promiscuous mode Sean Anderson
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 14+ messages in thread
From: Sean Anderson @ 2024-08-15 19:36 UTC (permalink / raw)
  To: Radhey Shyam Pandey, netdev
  Cc: David S . Miller, Andrew Lunn, linux-arm-kernel, Michal Simek,
	Daniel Borkmann, linux-kernel, Paolo Abeni, Jakub Kicinski,
	Eric Dumazet, Sean Anderson, Simon Horman

If a multicast address is removed but there are still some multicast
addresses, that address would remain programmed into the frame filter.
Fix this by explicitly setting the enable bit for each filter.

Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")
Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
Reviewed-by: Simon Horman <horms@kernel.org>
---

(no changes since v1)

 drivers/net/ethernet/xilinx/xilinx_axienet.h  |  1 +
 .../net/ethernet/xilinx/xilinx_axienet_main.c | 21 ++++++++-----------
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h
index 0d5b300107e0..03fef656478e 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet.h
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h
@@ -170,6 +170,7 @@
 #define XAE_UAW0_OFFSET		0x00000700 /* Unicast address word 0 */
 #define XAE_UAW1_OFFSET		0x00000704 /* Unicast address word 1 */
 #define XAE_FMI_OFFSET		0x00000708 /* Filter Mask Index */
+#define XAE_FFE_OFFSET		0x0000070C /* Frame Filter Enable */
 #define XAE_AF0_OFFSET		0x00000710 /* Address Filter 0 */
 #define XAE_AF1_OFFSET		0x00000714 /* Address Filter 1 */
 
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index e664611c29cf..1bcabb016ca9 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -433,7 +433,7 @@ static int netdev_set_mac_address(struct net_device *ndev, void *p)
  */
 static void axienet_set_multicast_list(struct net_device *ndev)
 {
-	int i;
+	int i = 0;
 	u32 reg, af0reg, af1reg;
 	struct axienet_local *lp = netdev_priv(ndev);
 
@@ -455,7 +455,6 @@ static void axienet_set_multicast_list(struct net_device *ndev)
 		reg &= ~XAE_FMI_PM_MASK;
 		axienet_iow(lp, XAE_FMI_OFFSET, reg);
 
-		i = 0;
 		netdev_for_each_mc_addr(ha, ndev) {
 			if (i >= XAE_MULTICAST_CAM_TABLE_NUM)
 				break;
@@ -474,6 +473,7 @@ static void axienet_set_multicast_list(struct net_device *ndev)
 			axienet_iow(lp, XAE_FMI_OFFSET, reg);
 			axienet_iow(lp, XAE_AF0_OFFSET, af0reg);
 			axienet_iow(lp, XAE_AF1_OFFSET, af1reg);
+			axienet_iow(lp, XAE_FFE_OFFSET, 1);
 			i++;
 		}
 	} else {
@@ -481,18 +481,15 @@ static void axienet_set_multicast_list(struct net_device *ndev)
 		reg &= ~XAE_FMI_PM_MASK;
 
 		axienet_iow(lp, XAE_FMI_OFFSET, reg);
-
-		for (i = 0; i < XAE_MULTICAST_CAM_TABLE_NUM; i++) {
-			reg = axienet_ior(lp, XAE_FMI_OFFSET) & 0xFFFFFF00;
-			reg |= i;
-
-			axienet_iow(lp, XAE_FMI_OFFSET, reg);
-			axienet_iow(lp, XAE_AF0_OFFSET, 0);
-			axienet_iow(lp, XAE_AF1_OFFSET, 0);
-		}
-
 		dev_info(&ndev->dev, "Promiscuous mode disabled.\n");
 	}
+
+	for (; i < XAE_MULTICAST_CAM_TABLE_NUM; i++) {
+		reg = axienet_ior(lp, XAE_FMI_OFFSET) & 0xFFFFFF00;
+		reg |= i;
+		axienet_iow(lp, XAE_FMI_OFFSET, reg);
+		axienet_iow(lp, XAE_FFE_OFFSET, 0);
+	}
 }
 
 /**
-- 
2.35.1.1320.gc452695387.dirty


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next v2 3/5] net: xilinx: axienet: Don't print if we go into promiscuous mode
  2024-08-15 19:36 [PATCH net-next v2 0/5] net: xilinx: axienet: Multicast fixes and improvements Sean Anderson
  2024-08-15 19:36 ` [PATCH net-next v2 1/5] net: xilinx: axienet: Always disable promiscuous mode Sean Anderson
  2024-08-15 19:36 ` [PATCH net-next v2 2/5] net: xilinx: axienet: Fix dangling multicast addresses Sean Anderson
@ 2024-08-15 19:36 ` Sean Anderson
  2024-08-16 10:53   ` Simon Horman
  2024-08-15 19:36 ` [PATCH net-next v2 4/5] net: xilinx: axienet: Don't set IFF_PROMISC in ndev->flags Sean Anderson
  2024-08-15 19:36 ` [PATCH net-next v2 5/5] net: xilinx: axienet: Support IFF_ALLMULTI Sean Anderson
  4 siblings, 1 reply; 14+ messages in thread
From: Sean Anderson @ 2024-08-15 19:36 UTC (permalink / raw)
  To: Radhey Shyam Pandey, netdev
  Cc: David S . Miller, Andrew Lunn, linux-arm-kernel, Michal Simek,
	Daniel Borkmann, linux-kernel, Paolo Abeni, Jakub Kicinski,
	Eric Dumazet, Sean Anderson

A message about being in promiscuous mode is printed every time each
additional multicast address beyond four is added. Suppress this message
like is done in other drivers.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

Changes in v2:
- Split off IFF_PROMISC change

 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 1bcabb016ca9..9382ce50aeb2 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -447,7 +447,6 @@ static void axienet_set_multicast_list(struct net_device *ndev)
 		reg = axienet_ior(lp, XAE_FMI_OFFSET);
 		reg |= XAE_FMI_PM_MASK;
 		axienet_iow(lp, XAE_FMI_OFFSET, reg);
-		dev_info(&ndev->dev, "Promiscuous mode enabled.\n");
 	} else if (!netdev_mc_empty(ndev)) {
 		struct netdev_hw_addr *ha;
 
@@ -481,7 +480,6 @@ static void axienet_set_multicast_list(struct net_device *ndev)
 		reg &= ~XAE_FMI_PM_MASK;
 
 		axienet_iow(lp, XAE_FMI_OFFSET, reg);
-		dev_info(&ndev->dev, "Promiscuous mode disabled.\n");
 	}
 
 	for (; i < XAE_MULTICAST_CAM_TABLE_NUM; i++) {
-- 
2.35.1.1320.gc452695387.dirty


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next v2 4/5] net: xilinx: axienet: Don't set IFF_PROMISC in ndev->flags
  2024-08-15 19:36 [PATCH net-next v2 0/5] net: xilinx: axienet: Multicast fixes and improvements Sean Anderson
                   ` (2 preceding siblings ...)
  2024-08-15 19:36 ` [PATCH net-next v2 3/5] net: xilinx: axienet: Don't print if we go into promiscuous mode Sean Anderson
@ 2024-08-15 19:36 ` Sean Anderson
  2024-08-16 10:54   ` Simon Horman
  2024-08-15 19:36 ` [PATCH net-next v2 5/5] net: xilinx: axienet: Support IFF_ALLMULTI Sean Anderson
  4 siblings, 1 reply; 14+ messages in thread
From: Sean Anderson @ 2024-08-15 19:36 UTC (permalink / raw)
  To: Radhey Shyam Pandey, netdev
  Cc: David S . Miller, Andrew Lunn, linux-arm-kernel, Michal Simek,
	Daniel Borkmann, linux-kernel, Paolo Abeni, Jakub Kicinski,
	Eric Dumazet, Sean Anderson

Contrary to the comment, we don't have to inform the net subsystem.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

Changes in v2:
- Split off from printing changes

 drivers/net/ethernet/xilinx/xilinx_axienet_main.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 9382ce50aeb2..9bcad515f156 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -439,11 +439,6 @@ static void axienet_set_multicast_list(struct net_device *ndev)
 
 	if (ndev->flags & (IFF_ALLMULTI | IFF_PROMISC) ||
 	    netdev_mc_count(ndev) > XAE_MULTICAST_CAM_TABLE_NUM) {
-		/* We must make the kernel realize we had to move into
-		 * promiscuous mode. If it was a promiscuous mode request
-		 * the flag is already set. If not we set it.
-		 */
-		ndev->flags |= IFF_PROMISC;
 		reg = axienet_ior(lp, XAE_FMI_OFFSET);
 		reg |= XAE_FMI_PM_MASK;
 		axienet_iow(lp, XAE_FMI_OFFSET, reg);
-- 
2.35.1.1320.gc452695387.dirty


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH net-next v2 5/5] net: xilinx: axienet: Support IFF_ALLMULTI
  2024-08-15 19:36 [PATCH net-next v2 0/5] net: xilinx: axienet: Multicast fixes and improvements Sean Anderson
                   ` (3 preceding siblings ...)
  2024-08-15 19:36 ` [PATCH net-next v2 4/5] net: xilinx: axienet: Don't set IFF_PROMISC in ndev->flags Sean Anderson
@ 2024-08-15 19:36 ` Sean Anderson
  4 siblings, 0 replies; 14+ messages in thread
From: Sean Anderson @ 2024-08-15 19:36 UTC (permalink / raw)
  To: Radhey Shyam Pandey, netdev
  Cc: David S . Miller, Andrew Lunn, linux-arm-kernel, Michal Simek,
	Daniel Borkmann, linux-kernel, Paolo Abeni, Jakub Kicinski,
	Eric Dumazet, Sean Anderson, Simon Horman

Add support for IFF_ALLMULTI by configuring a single filter to match the
multicast address bit. This allows us to keep promiscuous mode disabled,
even when we have more than four multicast addresses. An even better
solution would be to "pack" addresses into the available CAM registers,
but that can wait for a future series.

Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
Reviewed-by: Simon Horman <horms@kernel.org>
---

(no changes since v1)

 drivers/net/ethernet/xilinx/xilinx_axienet.h  |  2 ++
 .../net/ethernet/xilinx/xilinx_axienet_main.c | 34 +++++++++++--------
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h
index 03fef656478e..d1b68a040f5a 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet.h
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h
@@ -173,6 +173,8 @@
 #define XAE_FFE_OFFSET		0x0000070C /* Frame Filter Enable */
 #define XAE_AF0_OFFSET		0x00000710 /* Address Filter 0 */
 #define XAE_AF1_OFFSET		0x00000714 /* Address Filter 1 */
+#define XAE_AM0_OFFSET		0x00000750 /* Frame Filter Mask Value Bytes 3-0 */
+#define XAE_AM1_OFFSET		0x00000754 /* Frame Filter Mask Value Bytes 7-4 */
 
 #define XAE_TX_VLAN_DATA_OFFSET 0x00004000 /* TX VLAN data table address */
 #define XAE_RX_VLAN_DATA_OFFSET 0x00008000 /* RX VLAN data table address */
diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
index 9bcad515f156..c420bc753750 100644
--- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
+++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
@@ -437,18 +437,27 @@ static void axienet_set_multicast_list(struct net_device *ndev)
 	u32 reg, af0reg, af1reg;
 	struct axienet_local *lp = netdev_priv(ndev);
 
-	if (ndev->flags & (IFF_ALLMULTI | IFF_PROMISC) ||
-	    netdev_mc_count(ndev) > XAE_MULTICAST_CAM_TABLE_NUM) {
-		reg = axienet_ior(lp, XAE_FMI_OFFSET);
+	reg = axienet_ior(lp, XAE_FMI_OFFSET);
+	reg &= ~XAE_FMI_PM_MASK;
+	if (ndev->flags & IFF_PROMISC)
 		reg |= XAE_FMI_PM_MASK;
+	else
+		reg &= ~XAE_FMI_PM_MASK;
+	axienet_iow(lp, XAE_FMI_OFFSET, reg);
+
+	if (ndev->flags & IFF_ALLMULTI ||
+	    netdev_mc_count(ndev) > XAE_MULTICAST_CAM_TABLE_NUM) {
+		reg &= 0xFFFFFF00;
 		axienet_iow(lp, XAE_FMI_OFFSET, reg);
+		axienet_iow(lp, XAE_AF0_OFFSET, 1); /* Multicast bit */
+		axienet_iow(lp, XAE_AF1_OFFSET, 0);
+		axienet_iow(lp, XAE_AM0_OFFSET, 1); /* ditto */
+		axienet_iow(lp, XAE_AM1_OFFSET, 0);
+		axienet_iow(lp, XAE_FFE_OFFSET, 1);
+		i = 1;
 	} else if (!netdev_mc_empty(ndev)) {
 		struct netdev_hw_addr *ha;
 
-		reg = axienet_ior(lp, XAE_FMI_OFFSET);
-		reg &= ~XAE_FMI_PM_MASK;
-		axienet_iow(lp, XAE_FMI_OFFSET, reg);
-
 		netdev_for_each_mc_addr(ha, ndev) {
 			if (i >= XAE_MULTICAST_CAM_TABLE_NUM)
 				break;
@@ -461,24 +470,21 @@ static void axienet_set_multicast_list(struct net_device *ndev)
 			af1reg = (ha->addr[4]);
 			af1reg |= (ha->addr[5] << 8);
 
-			reg = axienet_ior(lp, XAE_FMI_OFFSET) & 0xFFFFFF00;
+			reg &= 0xFFFFFF00;
 			reg |= i;
 
 			axienet_iow(lp, XAE_FMI_OFFSET, reg);
 			axienet_iow(lp, XAE_AF0_OFFSET, af0reg);
 			axienet_iow(lp, XAE_AF1_OFFSET, af1reg);
+			axienet_iow(lp, XAE_AM0_OFFSET, 0xffffffff);
+			axienet_iow(lp, XAE_AM1_OFFSET, 0x0000ffff);
 			axienet_iow(lp, XAE_FFE_OFFSET, 1);
 			i++;
 		}
-	} else {
-		reg = axienet_ior(lp, XAE_FMI_OFFSET);
-		reg &= ~XAE_FMI_PM_MASK;
-
-		axienet_iow(lp, XAE_FMI_OFFSET, reg);
 	}
 
 	for (; i < XAE_MULTICAST_CAM_TABLE_NUM; i++) {
-		reg = axienet_ior(lp, XAE_FMI_OFFSET) & 0xFFFFFF00;
+		reg &= 0xFFFFFF00;
 		reg |= i;
 		axienet_iow(lp, XAE_FMI_OFFSET, reg);
 		axienet_iow(lp, XAE_FFE_OFFSET, 0);
-- 
2.35.1.1320.gc452695387.dirty


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 3/5] net: xilinx: axienet: Don't print if we go into promiscuous mode
  2024-08-15 19:36 ` [PATCH net-next v2 3/5] net: xilinx: axienet: Don't print if we go into promiscuous mode Sean Anderson
@ 2024-08-16 10:53   ` Simon Horman
  0 siblings, 0 replies; 14+ messages in thread
From: Simon Horman @ 2024-08-16 10:53 UTC (permalink / raw)
  To: Sean Anderson
  Cc: Radhey Shyam Pandey, netdev, David S. Miller, Andrew Lunn,
	linux-arm-kernel, Michal Simek, Daniel Borkmann, linux-kernel,
	Paolo Abeni, Jakub Kicinski, Eric Dumazet

On Thu, Aug 15, 2024 at 03:36:12PM -0400, Sean Anderson wrote:
> A message about being in promiscuous mode is printed every time each
> additional multicast address beyond four is added. Suppress this message
> like is done in other drivers.
> 
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
> 
> Changes in v2:
> - Split off IFF_PROMISC change

Reviewed-by: Simon Horman <horms@kernel.org>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 4/5] net: xilinx: axienet: Don't set IFF_PROMISC in ndev->flags
  2024-08-15 19:36 ` [PATCH net-next v2 4/5] net: xilinx: axienet: Don't set IFF_PROMISC in ndev->flags Sean Anderson
@ 2024-08-16 10:54   ` Simon Horman
  0 siblings, 0 replies; 14+ messages in thread
From: Simon Horman @ 2024-08-16 10:54 UTC (permalink / raw)
  To: Sean Anderson
  Cc: Radhey Shyam Pandey, netdev, David S . Miller, Andrew Lunn,
	linux-arm-kernel, Michal Simek, Daniel Borkmann, linux-kernel,
	Paolo Abeni, Jakub Kicinski, Eric Dumazet

On Thu, Aug 15, 2024 at 03:36:13PM -0400, Sean Anderson wrote:
> Contrary to the comment, we don't have to inform the net subsystem.
> 
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
> 
> Changes in v2:
> - Split off from printing changes

Reviewed-by: Simon Horman <horms@kernel.org>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 1/5] net: xilinx: axienet: Always disable promiscuous mode
  2024-08-15 19:36 ` [PATCH net-next v2 1/5] net: xilinx: axienet: Always disable promiscuous mode Sean Anderson
@ 2024-08-20  1:30   ` Jakub Kicinski
  2024-08-20 14:24     ` Sean Anderson
  0 siblings, 1 reply; 14+ messages in thread
From: Jakub Kicinski @ 2024-08-20  1:30 UTC (permalink / raw)
  To: Sean Anderson
  Cc: Radhey Shyam Pandey, netdev, David S . Miller, Andrew Lunn,
	linux-arm-kernel, Michal Simek, Daniel Borkmann, linux-kernel,
	Paolo Abeni, Eric Dumazet, Simon Horman

On Thu, 15 Aug 2024 15:36:10 -0400 Sean Anderson wrote:
> If promiscuous mode is disabled when there are fewer than four multicast
> addresses, then it will to be reflected in the hardware. Fix this by

it will *not* be reflected?
Something is off with this commit messages, or at least I can't parse

> always clearing the promiscuous mode flag even when we program multicast
> addresses.
> 
> Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")

I think we should ship it as a fix to net?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 2/5] net: xilinx: axienet: Fix dangling multicast addresses
  2024-08-15 19:36 ` [PATCH net-next v2 2/5] net: xilinx: axienet: Fix dangling multicast addresses Sean Anderson
@ 2024-08-20  1:33   ` Jakub Kicinski
  2024-08-20 14:43     ` Sean Anderson
  0 siblings, 1 reply; 14+ messages in thread
From: Jakub Kicinski @ 2024-08-20  1:33 UTC (permalink / raw)
  To: Sean Anderson
  Cc: Radhey Shyam Pandey, netdev, David S . Miller, Andrew Lunn,
	linux-arm-kernel, Michal Simek, Daniel Borkmann, linux-kernel,
	Paolo Abeni, Eric Dumazet, Simon Horman

On Thu, 15 Aug 2024 15:36:11 -0400 Sean Anderson wrote:
> If a multicast address is removed but there are still some multicast
> addresses, that address would remain programmed into the frame filter.
> Fix this by explicitly setting the enable bit for each filter.
> 
> Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> Reviewed-by: Simon Horman <horms@kernel.org>

Again, I'd go for net.

> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h
> index 0d5b300107e0..03fef656478e 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h
> @@ -170,6 +170,7 @@
>  #define XAE_UAW0_OFFSET		0x00000700 /* Unicast address word 0 */
>  #define XAE_UAW1_OFFSET		0x00000704 /* Unicast address word 1 */
>  #define XAE_FMI_OFFSET		0x00000708 /* Filter Mask Index */
> +#define XAE_FFE_OFFSET		0x0000070C /* Frame Filter Enable */
>  #define XAE_AF0_OFFSET		0x00000710 /* Address Filter 0 */
>  #define XAE_AF1_OFFSET		0x00000714 /* Address Filter 1 */

There is a conflict with current net / net-next here, because of
9ff2f816e2aa65ca9, you'll need to rebase / repost (which is why 
I'm allowing myself the nit picks ;))

> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> index e664611c29cf..1bcabb016ca9 100644
> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
> @@ -433,7 +433,7 @@ static int netdev_set_mac_address(struct net_device *ndev, void *p)
>   */
>  static void axienet_set_multicast_list(struct net_device *ndev)
>  {
> -	int i;
> +	int i = 0;

Consider renaming i to addr_cnt ? or addr_num ?

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 1/5] net: xilinx: axienet: Always disable promiscuous mode
  2024-08-20  1:30   ` Jakub Kicinski
@ 2024-08-20 14:24     ` Sean Anderson
  2024-08-22 14:25       ` Sean Anderson
  0 siblings, 1 reply; 14+ messages in thread
From: Sean Anderson @ 2024-08-20 14:24 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Radhey Shyam Pandey, netdev, David S . Miller, Andrew Lunn,
	linux-arm-kernel, Michal Simek, Daniel Borkmann, linux-kernel,
	Paolo Abeni, Eric Dumazet, Simon Horman

On 8/19/24 21:30, Jakub Kicinski wrote:
> On Thu, 15 Aug 2024 15:36:10 -0400 Sean Anderson wrote:
>> If promiscuous mode is disabled when there are fewer than four multicast
>> addresses, then it will to be reflected in the hardware. Fix this by
> 
> it will *not* be reflected?
> Something is off with this commit messages, or at least I can't parse
> 
>> always clearing the promiscuous mode flag even when we program multicast
>> addresses.
>> 
>> Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")
> 
> I think we should ship it as a fix to net?

Yes, probably. I put these patches first so they could be easily cherry-picked.

--Sean

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 2/5] net: xilinx: axienet: Fix dangling multicast addresses
  2024-08-20  1:33   ` Jakub Kicinski
@ 2024-08-20 14:43     ` Sean Anderson
  0 siblings, 0 replies; 14+ messages in thread
From: Sean Anderson @ 2024-08-20 14:43 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Radhey Shyam Pandey, netdev, David S . Miller, Andrew Lunn,
	linux-arm-kernel, Michal Simek, Daniel Borkmann, linux-kernel,
	Paolo Abeni, Eric Dumazet, Simon Horman

On 8/19/24 21:33, Jakub Kicinski wrote:
> On Thu, 15 Aug 2024 15:36:11 -0400 Sean Anderson wrote:
>> If a multicast address is removed but there are still some multicast
>> addresses, that address would remain programmed into the frame filter.
>> Fix this by explicitly setting the enable bit for each filter.
>> 
>> Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")
>> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
>> Reviewed-by: Simon Horman <horms@kernel.org>
> 
> Again, I'd go for net.
> 
>> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet.h b/drivers/net/ethernet/xilinx/xilinx_axienet.h
>> index 0d5b300107e0..03fef656478e 100644
>> --- a/drivers/net/ethernet/xilinx/xilinx_axienet.h
>> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet.h
>> @@ -170,6 +170,7 @@
>>  #define XAE_UAW0_OFFSET		0x00000700 /* Unicast address word 0 */
>>  #define XAE_UAW1_OFFSET		0x00000704 /* Unicast address word 1 */
>>  #define XAE_FMI_OFFSET		0x00000708 /* Filter Mask Index */
>> +#define XAE_FFE_OFFSET		0x0000070C /* Frame Filter Enable */
>>  #define XAE_AF0_OFFSET		0x00000710 /* Address Filter 0 */
>>  #define XAE_AF1_OFFSET		0x00000714 /* Address Filter 1 */
> 
> There is a conflict with current net / net-next here, because of
> 9ff2f816e2aa65ca9, you'll need to rebase / repost (which is why 
> I'm allowing myself the nit picks ;))
> 
>> diff --git a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>> index e664611c29cf..1bcabb016ca9 100644
>> --- a/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>> +++ b/drivers/net/ethernet/xilinx/xilinx_axienet_main.c
>> @@ -433,7 +433,7 @@ static int netdev_set_mac_address(struct net_device *ndev, void *p)
>>   */
>>  static void axienet_set_multicast_list(struct net_device *ndev)
>>  {
>> -	int i;
>> +	int i = 0;
> 
> Consider renaming i to addr_cnt ? or addr_num ?

Well, this doesn't really have anything to do with addresses. It selects
the "Filter Index" (as named by the datasheet) so I'd rename it `filter`
if anything.

This hardware is actually really unusual since the CAM acts on the
entire first 64 bytes of the packet. So you could theoretically filter
for the source address too, but I don't know why you'd want to since all
you can do is drop the packet. Seems like a big waste of resources to
me.

--Sean

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 1/5] net: xilinx: axienet: Always disable promiscuous mode
  2024-08-20 14:24     ` Sean Anderson
@ 2024-08-22 14:25       ` Sean Anderson
  2024-08-22 15:24         ` Jakub Kicinski
  0 siblings, 1 reply; 14+ messages in thread
From: Sean Anderson @ 2024-08-22 14:25 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: Radhey Shyam Pandey, netdev, David S . Miller, Andrew Lunn,
	linux-arm-kernel, Michal Simek, Daniel Borkmann, linux-kernel,
	Paolo Abeni, Eric Dumazet, Simon Horman

On 8/20/24 10:24, Sean Anderson wrote:
> On 8/19/24 21:30, Jakub Kicinski wrote:
>> On Thu, 15 Aug 2024 15:36:10 -0400 Sean Anderson wrote:
>>> If promiscuous mode is disabled when there are fewer than four multicast
>>> addresses, then it will to be reflected in the hardware. Fix this by
>> 
>> it will *not* be reflected?
>> Something is off with this commit messages, or at least I can't parse
>> 
>>> always clearing the promiscuous mode flag even when we program multicast
>>> addresses.
>>> 
>>> Fixes: 8a3b7a252dca ("drivers/net/ethernet/xilinx: added Xilinx AXI Ethernet driver")
>> 
>> I think we should ship it as a fix to net?
> 
> Yes, probably. I put these patches first so they could be easily cherry-picked.

OK, so to be clear: how should I send these patches?

--Sean


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH net-next v2 1/5] net: xilinx: axienet: Always disable promiscuous mode
  2024-08-22 14:25       ` Sean Anderson
@ 2024-08-22 15:24         ` Jakub Kicinski
  0 siblings, 0 replies; 14+ messages in thread
From: Jakub Kicinski @ 2024-08-22 15:24 UTC (permalink / raw)
  To: Sean Anderson
  Cc: Radhey Shyam Pandey, netdev, David S . Miller, Andrew Lunn,
	linux-arm-kernel, Michal Simek, Daniel Borkmann, linux-kernel,
	Paolo Abeni, Eric Dumazet, Simon Horman

On Thu, 22 Aug 2024 10:25:06 -0400 Sean Anderson wrote:
> > Yes, probably. I put these patches first so they could be easily cherry-picked.  
> 
> OK, so to be clear: how should I send these patches?

You gotta rebase and repost the first two -- I would have taken them
directly to net from this posting but there is a conflict on patch 2
(as mentioned there).

Could you repost the first two rebased ASAP? (You can leave the rename
of i for the net-next series). I'm literally prepping  the PR
net->Linus right now, if you post soon they will be cross-merged into
net-next by EOD, making it easier to merge the rest.

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2024-08-22 15:24 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-15 19:36 [PATCH net-next v2 0/5] net: xilinx: axienet: Multicast fixes and improvements Sean Anderson
2024-08-15 19:36 ` [PATCH net-next v2 1/5] net: xilinx: axienet: Always disable promiscuous mode Sean Anderson
2024-08-20  1:30   ` Jakub Kicinski
2024-08-20 14:24     ` Sean Anderson
2024-08-22 14:25       ` Sean Anderson
2024-08-22 15:24         ` Jakub Kicinski
2024-08-15 19:36 ` [PATCH net-next v2 2/5] net: xilinx: axienet: Fix dangling multicast addresses Sean Anderson
2024-08-20  1:33   ` Jakub Kicinski
2024-08-20 14:43     ` Sean Anderson
2024-08-15 19:36 ` [PATCH net-next v2 3/5] net: xilinx: axienet: Don't print if we go into promiscuous mode Sean Anderson
2024-08-16 10:53   ` Simon Horman
2024-08-15 19:36 ` [PATCH net-next v2 4/5] net: xilinx: axienet: Don't set IFF_PROMISC in ndev->flags Sean Anderson
2024-08-16 10:54   ` Simon Horman
2024-08-15 19:36 ` [PATCH net-next v2 5/5] net: xilinx: axienet: Support IFF_ALLMULTI Sean Anderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).