diff mbox series

net: mlx5: update hw flows when encap source mac changed

Message ID 1546780328-72363-1-git-send-email-xiangxia.m.yue@gmail.com
State Not Applicable
Delegated to: David Miller
Headers show
Series net: mlx5: update hw flows when encap source mac changed | expand

Commit Message

Tonghao Zhang Jan. 6, 2019, 1:12 p.m. UTC 
From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

When we offload tc filters to hardware, hardware flows can
be updated when mac of encap destination ip is changed.
But we ignore one case, that the mac of local encap ip can
be changed too, so we should also update them.

To fix it, add route_dev in mlx5e_encap_entry struct to save
the local encap netdevice, and when mac changed, check it and
update flows.

Fixs: 232c00139 ("net/mlx5e: Add support to neighbour update flow")
Cc: Hadar Hen Zion <hadarh@mellanox.com>
Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
---
 drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c |  5 ++++-
 drivers/net/ethernet/mellanox/mlx5/core/en_rep.c    | 20 +++++++++++++++-----
 drivers/net/ethernet/mellanox/mlx5/core/en_rep.h    |  2 ++
 3 files changed, 21 insertions(+), 6 deletions(-)

Comments

Cong Wang Jan. 11, 2019, 10:50 p.m. UTC | #1 
On Fri, Jan 11, 2019 at 6:19 AM <xiangxia.m.yue@gmail.com> wrote:
> @@ -622,17 +629,20 @@ static void mlx5e_rep_neigh_update(struct work_struct *work)
>         memcpy(ha, n->ha, ETH_ALEN);
>         nud_state = n->nud_state;
>         dead = n->dead;
> +       netdev = n->dev;
>         read_unlock_bh(&n->lock);
>
>         neigh_connected = (nud_state & NUD_VALID) && !dead;
> +       source_mac_changed = (e->h_source == netdev) && !ether_addr_equal(e->h_source, netdev->dev_addr);

I don't know why you want to compare ->h_source, which points to
a mac address, with a net_device pointer here.
Tonghao Zhang Jan. 12, 2019, 3:53 a.m. UTC | #2 
On Sat, Jan 12, 2019 at 6:50 AM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Fri, Jan 11, 2019 at 6:19 AM <xiangxia.m.yue@gmail.com> wrote:
> > @@ -622,17 +629,20 @@ static void mlx5e_rep_neigh_update(struct work_struct *work)
> >         memcpy(ha, n->ha, ETH_ALEN);
> >         nud_state = n->nud_state;
> >         dead = n->dead;
> > +       netdev = n->dev;
> >         read_unlock_bh(&n->lock);
> >
> >         neigh_connected = (nud_state & NUD_VALID) && !dead;
> > +       source_mac_changed = (e->h_source == netdev) && !ether_addr_equal(e->h_source, netdev->dev_addr);
>
> I don't know why you want to compare ->h_source, which points to
> a mac address, with a net_device pointer here.
my fault, but it works, because it also update the source mac. I
simply the implement in v2.
Or Gerlitz Jan. 17, 2019, 1:04 p.m. UTC | #3 
On Fri, Jan 11, 2019 at 4:19 PM <xiangxia.m.yue@gmail.com> wrote:
>
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>

We have common prefix, please use it:

net/mlx5e: Update offloaded flows when encap source mac changed

> When we offload tc filters to hardware, hardware flows can
> be updated when mac of encap destination ip is changed.
> But we ignore one case, that the mac of local encap ip can
> be changed too, so we should also update them.
>
> To fix it, add route_dev in mlx5e_encap_entry struct to save
> the local encap netdevice, and when mac changed, check it and
> update flows.

The problem seems to exist and the solution seems to be proper

I will look into it later, please resend with [PATCH net-next]  and
few nit fixes

> Fixs: 232c00139 ("net/mlx5e: Add support to neighbour update flow")

should be Fixes: and get 12 hex digits (git log --oneline --abbrev=12)
diff mbox series

Patch

diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
index 046948e..c1a5d74 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/tc_tun.c
@@ -192,7 +192,7 @@  static char *gen_eth_tnl_hdr(char *buf, struct net_device *dev,
 	char *ip;
 
 	ether_addr_copy(eth->h_dest, e->h_dest);
-	ether_addr_copy(eth->h_source, dev->dev_addr);
+	ether_addr_copy(eth->h_source, e->h_source);
 	if (is_vlan_dev(dev)) {
 		struct vlan_hdr *vlan = (struct vlan_hdr *)
 					((char *)eth + ETH_HLEN);
@@ -256,6 +256,7 @@  int mlx5e_tc_tun_create_header_ipv4(struct mlx5e_priv *priv,
 	e->m_neigh.family = n->ops->family;
 	memcpy(&e->m_neigh.dst_ip, n->primary_key, n->tbl->key_len);
 	e->out_dev = out_dev;
+	e->route_dev = route_dev;
 
 	/* It's important to add the neigh to the hash table before checking
 	 * the neigh validity state. So if we'll get a notification, in case the
@@ -270,6 +271,8 @@  int mlx5e_tc_tun_create_header_ipv4(struct mlx5e_priv *priv,
 	nud_state = n->nud_state;
 	ether_addr_copy(e->h_dest, n->ha);
 	read_unlock_bh(&n->lock);
+	
+	ether_addr_copy(e->h_source, route_dev->dev_addr);
 
 	/* add ethernet header */
 	ip = (struct iphdr *)gen_eth_tnl_hdr(encap_header, route_dev, e,
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
index 96cc0c6..f0f72e9 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.c
@@ -581,19 +581,24 @@  static void mlx5e_rep_neigh_entry_release(struct mlx5e_neigh_hash_entry *nhe)
 static void mlx5e_rep_update_flows(struct mlx5e_priv *priv,
 				   struct mlx5e_encap_entry *e,
 				   bool neigh_connected,
-				   unsigned char ha[ETH_ALEN])
+				   unsigned char ha[ETH_ALEN],
+				   bool source_mac_changed)
 {
 	struct ethhdr *eth = (struct ethhdr *)e->encap_header;
 
 	ASSERT_RTNL();
 
 	if ((e->flags & MLX5_ENCAP_ENTRY_VALID) &&
-	    (!neigh_connected || !ether_addr_equal(e->h_dest, ha)))
+	    (!neigh_connected ||
+	     !ether_addr_equal(e->h_dest, ha) ||
+	     source_mac_changed))
 		mlx5e_tc_encap_flows_del(priv, e);
 
-	if (neigh_connected && !(e->flags & MLX5_ENCAP_ENTRY_VALID)) {
+	if (!(e->flags & MLX5_ENCAP_ENTRY_VALID) &&
+	    (neigh_connected || source_mac_changed)) {
 		ether_addr_copy(e->h_dest, ha);
 		ether_addr_copy(eth->h_dest, ha);
+		ether_addr_copy(eth->h_source, e->route_dev->dev_addr);
 
 		mlx5e_tc_encap_flows_add(priv, e);
 	}
@@ -609,7 +614,9 @@  static void mlx5e_rep_neigh_update(struct work_struct *work)
 	struct mlx5e_priv *priv;
 	bool neigh_connected;
 	bool encap_connected;
+	bool source_mac_changed;
 	u8 nud_state, dead;
+	struct net_device *netdev;
 
 	rtnl_lock();
 
@@ -622,17 +629,20 @@  static void mlx5e_rep_neigh_update(struct work_struct *work)
 	memcpy(ha, n->ha, ETH_ALEN);
 	nud_state = n->nud_state;
 	dead = n->dead;
+	netdev = n->dev;
 	read_unlock_bh(&n->lock);
 
 	neigh_connected = (nud_state & NUD_VALID) && !dead;
+	source_mac_changed = (e->h_source == netdev) && !ether_addr_equal(e->h_source, netdev->dev_addr);
 
 	list_for_each_entry(e, &nhe->encap_list, encap_list) {
 		encap_connected = !!(e->flags & MLX5_ENCAP_ENTRY_VALID);
 		priv = netdev_priv(e->out_dev);
 
 		if (encap_connected != neigh_connected ||
-		    !ether_addr_equal(e->h_dest, ha))
-			mlx5e_rep_update_flows(priv, e, neigh_connected, ha);
+		    !ether_addr_equal(e->h_dest, ha) ||
+		    source_mac_changed)
+			mlx5e_rep_update_flows(priv, e, neigh_connected, ha, source_mac_changed);
 	}
 	mlx5e_rep_neigh_entry_release(nhe);
 	rtnl_unlock();
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
index edd7228..96fc7c6 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rep.h
@@ -146,8 +146,10 @@  struct mlx5e_encap_entry {
 	u32 encap_id;
 	struct ip_tunnel_info tun_info;
 	unsigned char h_dest[ETH_ALEN];	/* destination eth addr	*/
+	unsigned char h_source[ETH_ALEN];
 
 	struct net_device *out_dev;
+	struct net_device *route_dev;
 	int tunnel_type;
 	int tunnel_hlen;
 	int reformat_type;