All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] vxlan fdb nexthop misc fixes
@ 2020-05-29  5:12 Roopa Prabhu
  2020-05-29  5:12 ` [PATCH net-next 1/2] vxlan: add check to prevent use of remote ip attributes with NDA_NH_ID Roopa Prabhu
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Roopa Prabhu @ 2020-05-29  5:12 UTC (permalink / raw)
  To: davem, kuba; +Cc: netdev, dsahern, nikolay, jiri, idosch, petrm

From: Roopa Prabhu <roopa@cumulusnetworks.com>

Roopa Prabhu (2):
  vxlan: add check to prevent use of remote ip attributes with NDA_NH_ID
  vxlan: few locking fixes in nexthop event handler

 drivers/net/vxlan.c | 36 +++++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

-- 
2.1.4


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

* [PATCH net-next 1/2] vxlan: add check to prevent use of remote ip attributes with NDA_NH_ID
  2020-05-29  5:12 [PATCH net-next 0/2] vxlan fdb nexthop misc fixes Roopa Prabhu
@ 2020-05-29  5:12 ` Roopa Prabhu
  2020-05-29  5:12 ` [PATCH net-next 2/2] vxlan: few locking fixes in nexthop event handler Roopa Prabhu
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Roopa Prabhu @ 2020-05-29  5:12 UTC (permalink / raw)
  To: davem, kuba; +Cc: netdev, dsahern, nikolay, jiri, idosch, petrm

From: Roopa Prabhu <roopa@cumulusnetworks.com>

NDA_NH_ID represents a remote ip or a group of remote ips.
It allows use of nexthop groups in lieu of a remote ip or a
list of remote ips supported by the fdb api.

Current code ignores the other remote ip attrs when NDA_NH_ID is
specified. In the spirit of strict checking, This commit adds a
check to explicitly return an error on incorrect usage.

Fixes: 1274e1cc4226 ("vxlan: ecmp support for mac fdb entries")
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
Note: extack support for this is a bit intrusive to be included
in this patch. The function already does not support extack for the
other errors.  ndo_fdb_add and ndo_fdb_del handlers use this function
and ndo_fdb_del does not support extack. I can send a separate patch
covering extack for these add/del paths.

 drivers/net/vxlan.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index a0015cd..fe606c6 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -1196,6 +1196,10 @@ static int vxlan_fdb_parse(struct nlattr *tb[], struct vxlan_dev *vxlan,
 	struct net *net = dev_net(vxlan->dev);
 	int err;
 
+	if (tb[NDA_NH_ID] && (tb[NDA_DST] || tb[NDA_VNI] || tb[NDA_IFINDEX] ||
+	    tb[NDA_PORT]))
+		return -EINVAL;
+
 	if (tb[NDA_DST]) {
 		err = vxlan_nla_get_addr(ip, tb[NDA_DST]);
 		if (err)
-- 
2.1.4


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

* [PATCH net-next 2/2] vxlan: few locking fixes in nexthop event handler
  2020-05-29  5:12 [PATCH net-next 0/2] vxlan fdb nexthop misc fixes Roopa Prabhu
  2020-05-29  5:12 ` [PATCH net-next 1/2] vxlan: add check to prevent use of remote ip attributes with NDA_NH_ID Roopa Prabhu
@ 2020-05-29  5:12 ` Roopa Prabhu
  2020-05-30 13:34 ` [PATCH net-next 0/2] vxlan fdb nexthop misc fixes Ido Schimmel
  2020-05-31  4:47 ` David Miller
  3 siblings, 0 replies; 6+ messages in thread
From: Roopa Prabhu @ 2020-05-29  5:12 UTC (permalink / raw)
  To: davem, kuba; +Cc: netdev, dsahern, nikolay, jiri, idosch, petrm

From: Roopa Prabhu <roopa@cumulusnetworks.com>

- remove fdb from nh_list before the rcu grace period
- protect fdb->vdev with rcu
- hold spin lock before destroying fdb

Fixes: c7cdbe2efc40 ("vxlan: support for nexthop notifiers")
Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
 drivers/net/vxlan.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index fe606c6..39bc10a 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -81,7 +81,7 @@ struct vxlan_fdb {
 	u16		  flags;	/* see ndm_flags and below */
 	struct list_head  nh_list;
 	struct nexthop __rcu *nh;
-	struct vxlan_dev  *vdev;
+	struct vxlan_dev  __rcu *vdev;
 };
 
 #define NTF_VXLAN_ADDED_BY_USER 0x100
@@ -837,7 +837,7 @@ static struct vxlan_fdb *vxlan_fdb_alloc(struct vxlan_dev *vxlan, const u8 *mac,
 	f->updated = f->used = jiffies;
 	f->vni = src_vni;
 	f->nh = NULL;
-	f->vdev = vxlan;
+	RCU_INIT_POINTER(f->vdev, vxlan);
 	INIT_LIST_HEAD(&f->nh_list);
 	INIT_LIST_HEAD(&f->remotes);
 	memcpy(f->eth_addr, mac, ETH_ALEN);
@@ -963,7 +963,7 @@ static void __vxlan_fdb_free(struct vxlan_fdb *f)
 	nh = rcu_dereference_raw(f->nh);
 	if (nh) {
 		rcu_assign_pointer(f->nh, NULL);
-		list_del_rcu(&f->nh_list);
+		rcu_assign_pointer(f->vdev, NULL);
 		nexthop_put(nh);
 	}
 
@@ -1000,7 +1000,7 @@ static void vxlan_fdb_destroy(struct vxlan_dev *vxlan, struct vxlan_fdb *f,
 	}
 
 	hlist_del_rcu(&f->hlist);
-	f->vdev = NULL;
+	list_del_rcu(&f->nh_list);
 	call_rcu(&f->rcu, vxlan_fdb_free);
 }
 
@@ -4615,17 +4615,35 @@ static struct notifier_block vxlan_switchdev_notifier_block __read_mostly = {
 	.notifier_call = vxlan_switchdev_event,
 };
 
+static void vxlan_fdb_nh_flush(struct nexthop *nh)
+{
+	struct vxlan_fdb *fdb;
+	struct vxlan_dev *vxlan;
+	u32 hash_index;
+
+	rcu_read_lock();
+	list_for_each_entry_rcu(fdb, &nh->fdb_list, nh_list) {
+		vxlan = rcu_dereference(fdb->vdev);
+		WARN_ON(!vxlan);
+		hash_index = fdb_head_index(vxlan, fdb->eth_addr,
+					    vxlan->default_dst.remote_vni);
+		spin_lock_bh(&vxlan->hash_lock[hash_index]);
+		if (!hlist_unhashed(&fdb->hlist))
+			vxlan_fdb_destroy(vxlan, fdb, false, false);
+		spin_unlock_bh(&vxlan->hash_lock[hash_index]);
+	}
+	rcu_read_unlock();
+}
+
 static int vxlan_nexthop_event(struct notifier_block *nb,
 			       unsigned long event, void *ptr)
 {
 	struct nexthop *nh = ptr;
-	struct vxlan_fdb *fdb, *tmp;
 
 	if (!nh || event != NEXTHOP_EVENT_DEL)
 		return NOTIFY_DONE;
 
-	list_for_each_entry_safe(fdb, tmp, &nh->fdb_list, nh_list)
-		vxlan_fdb_destroy(fdb->vdev, fdb, false, false);
+	vxlan_fdb_nh_flush(nh);
 
 	return NOTIFY_DONE;
 }
-- 
2.1.4


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

* Re: [PATCH net-next 0/2] vxlan fdb nexthop misc fixes
  2020-05-29  5:12 [PATCH net-next 0/2] vxlan fdb nexthop misc fixes Roopa Prabhu
  2020-05-29  5:12 ` [PATCH net-next 1/2] vxlan: add check to prevent use of remote ip attributes with NDA_NH_ID Roopa Prabhu
  2020-05-29  5:12 ` [PATCH net-next 2/2] vxlan: few locking fixes in nexthop event handler Roopa Prabhu
@ 2020-05-30 13:34 ` Ido Schimmel
  2020-05-30 19:07   ` Roopa Prabhu
  2020-05-31  4:47 ` David Miller
  3 siblings, 1 reply; 6+ messages in thread
From: Ido Schimmel @ 2020-05-30 13:34 UTC (permalink / raw)
  To: Roopa Prabhu; +Cc: davem, kuba, netdev, dsahern, nikolay, jiri, idosch, petrm

On Thu, May 28, 2020 at 10:12:34PM -0700, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>

Hi Roopa,

I noticed that sparse complains about the following problem in
the original submission (not handled by current set):

drivers/net/vxlan.c:884:41: warning: dereference of noderef expression

Seems to be fixed by:

diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
index 39bc10a7fd2e..ea7af03e0957 100644
--- a/drivers/net/vxlan.c
+++ b/drivers/net/vxlan.c
@@ -881,13 +881,13 @@ static int vxlan_fdb_nh_update(struct vxlan_dev *vxlan, struct vxlan_fdb *fdb,
                        goto err_inval;
                }
 
-               if (!nh->is_group || !nh->nh_grp->mpath) {
+               nhg = rtnl_dereference(nh->nh_grp);
+               if (!nh->is_group || !nhg->mpath) {
                        NL_SET_ERR_MSG(extack, "Nexthop is not a multipath group");
                        goto err_inval;
                }
 
                /* check nexthop group family */
-               nhg = rtnl_dereference(nh->nh_grp);
                switch (vxlan->default_dst.remote_ip.sa.sa_family) {
                case AF_INET:
                        if (!nhg->has_v4) {

Assuming it's correct, can you please fold it into v2?

> 
> Roopa Prabhu (2):
>   vxlan: add check to prevent use of remote ip attributes with NDA_NH_ID
>   vxlan: few locking fixes in nexthop event handler
> 
>  drivers/net/vxlan.c | 36 +++++++++++++++++++++++++++++-------
>  1 file changed, 29 insertions(+), 7 deletions(-)
> 
> -- 
> 2.1.4
> 

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

* Re: [PATCH net-next 0/2] vxlan fdb nexthop misc fixes
  2020-05-30 13:34 ` [PATCH net-next 0/2] vxlan fdb nexthop misc fixes Ido Schimmel
@ 2020-05-30 19:07   ` Roopa Prabhu
  0 siblings, 0 replies; 6+ messages in thread
From: Roopa Prabhu @ 2020-05-30 19:07 UTC (permalink / raw)
  To: Ido Schimmel
  Cc: David Miller, kuba, netdev, David Ahern, Nikolay Aleksandrov,
	Jiri Pirko, Ido Schimmel, Petr Machata

On Sat, May 30, 2020 at 6:34 AM Ido Schimmel <idosch@idosch.org> wrote:
>
> On Thu, May 28, 2020 at 10:12:34PM -0700, Roopa Prabhu wrote:
> > From: Roopa Prabhu <roopa@cumulusnetworks.com>
>
> Hi Roopa,
>
> I noticed that sparse complains about the following problem in
> the original submission (not handled by current set):
>
> drivers/net/vxlan.c:884:41: warning: dereference of noderef expression
>
> Seems to be fixed by:
>
> diff --git a/drivers/net/vxlan.c b/drivers/net/vxlan.c
> index 39bc10a7fd2e..ea7af03e0957 100644
> --- a/drivers/net/vxlan.c
> +++ b/drivers/net/vxlan.c
> @@ -881,13 +881,13 @@ static int vxlan_fdb_nh_update(struct vxlan_dev *vxlan, struct vxlan_fdb *fdb,
>                         goto err_inval;
>                 }
>
> -               if (!nh->is_group || !nh->nh_grp->mpath) {
> +               nhg = rtnl_dereference(nh->nh_grp);
> +               if (!nh->is_group || !nhg->mpath) {
>                         NL_SET_ERR_MSG(extack, "Nexthop is not a multipath group");
>                         goto err_inval;
>                 }
>
>                 /* check nexthop group family */
> -               nhg = rtnl_dereference(nh->nh_grp);
>                 switch (vxlan->default_dst.remote_ip.sa.sa_family) {
>                 case AF_INET:
>                         if (!nhg->has_v4) {
>
> Assuming it's correct, can you please fold it into v2?
>

Thanks Ido. This makes sense. I will add it to v2.

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

* Re: [PATCH net-next 0/2] vxlan fdb nexthop misc fixes
  2020-05-29  5:12 [PATCH net-next 0/2] vxlan fdb nexthop misc fixes Roopa Prabhu
                   ` (2 preceding siblings ...)
  2020-05-30 13:34 ` [PATCH net-next 0/2] vxlan fdb nexthop misc fixes Ido Schimmel
@ 2020-05-31  4:47 ` David Miller
  3 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2020-05-31  4:47 UTC (permalink / raw)
  To: roopa; +Cc: kuba, netdev, dsahern, nikolay, jiri, idosch, petrm

From: Roopa Prabhu <roopa@cumulusnetworks.com>
Date: Thu, 28 May 2020 22:12:34 -0700

> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> 
> Roopa Prabhu (2):
>   vxlan: add check to prevent use of remote ip attributes with NDA_NH_ID
>   vxlan: few locking fixes in nexthop event handler

Series applied, thanks.

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

end of thread, other threads:[~2020-05-31  4:47 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-29  5:12 [PATCH net-next 0/2] vxlan fdb nexthop misc fixes Roopa Prabhu
2020-05-29  5:12 ` [PATCH net-next 1/2] vxlan: add check to prevent use of remote ip attributes with NDA_NH_ID Roopa Prabhu
2020-05-29  5:12 ` [PATCH net-next 2/2] vxlan: few locking fixes in nexthop event handler Roopa Prabhu
2020-05-30 13:34 ` [PATCH net-next 0/2] vxlan fdb nexthop misc fixes Ido Schimmel
2020-05-30 19:07   ` Roopa Prabhu
2020-05-31  4:47 ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.