Re: [PATCH net-next v2] bonding: Remove unnecessary check

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 8/24/22 14:07, Jay Vosburgh wrote:
On 8/24/22, Sun Shouxin <sunshouxin@xxxxxxxxxxxxxxx> wrote:
This code is intended to support bond alb interface added to
Linux bridge by modifying MAC, however, it doesn't work for
one bond alb interface with vlan added to bridge.
Since commit d5410ac7b0ba("net:bonding:support balance-alb
interface with vlan to bridge"), new logic is adapted to handle
bond alb with or without vlan id, and then the code is deprecated.

I think this could still be clearer; the actual changes relate to the stack of
interfaces (e.g., eth0 -> bond0 -> vlan123 -> bridge0), not what VLAN tags
incoming traffic contains.

The code being removed here is specifically for the case of
eth0 -> bond0 -> bridge0, without an intermediate VLAN interface
in the stack (because, if memory serves, netif_is_bridge_port doesn't
transfer through to the bond if there's a VLAN interface in between).

Also, this code is for incoming traffic, assigning the bond's MAC to
traffic arriving on interfaces other than the active interface (which bears
the bond's MAC in alb mode; the other interfaces have different MACs).
Commit d5410ac7b0ba affects the balance assignments for outgoing ARP
traffic.  I'm not sure that d5410 is an exact replacement for the code this
patch removes.

I would be more comfortable with a change like this if it can be demonstrated that an example test case functions as expected before and after the change. Could a selftests test be written with veths to demonstrate this code is indeed redundant?

-Jon



Suggested-by: Hu Yadi <huyd12@xxxxxxxxxxxxxxx>
Signed-off-by: Sun Shouxin <sunshouxin@xxxxxxxxxxxxxxx>
---
  drivers/net/bonding/bond_main.c | 13 -------------
  1 file changed, 13 deletions(-)

diff --git a/drivers/net/bonding/bond_main.c
b/drivers/net/bonding/bond_main.c
index 50e60843020c..6b0f0ce9b9a1 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -1578,19 +1578,6 @@ static rx_handler_result_t bond_handle_frame(struct
sk_buff **pskb)

  	skb->dev = bond->dev;

-	if (BOND_MODE(bond) == BOND_MODE_ALB &&
-	    netif_is_bridge_port(bond->dev) &&
-	    skb->pkt_type == PACKET_HOST) {
-
-		if (unlikely(skb_cow_head(skb,
-					  skb->data - skb_mac_header(skb)))) {
-			kfree_skb(skb);
-			return RX_HANDLER_CONSUMED;
-		}
-		bond_hw_addr_copy(eth_hdr(skb)->h_dest, bond->dev->dev_addr,
-				  bond->dev->addr_len);
-	}
-
  	return ret;
  }

--
2.27.0







[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux