* [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