* [RFC net-next 1/4] net: ipv6: Make inet6addr_validator a blocking notifier
2017-10-10 16:41 [RFC net-next 0/4] mlxsw: spectrum_router: Add extack messages for RIF and VRF overflow David Ahern
@ 2017-10-10 16:41 ` David Ahern
2017-10-10 19:32 ` David Ahern
` (2 more replies)
2017-10-10 16:41 ` [RFC net-next 2/4] net: Add extack to validator_info structs used for address notifier David Ahern
` (2 subsequent siblings)
3 siblings, 3 replies; 15+ messages in thread
From: David Ahern @ 2017-10-10 16:41 UTC (permalink / raw)
To: netdev; +Cc: jiri, idosch, kjlx, David Ahern
inet6addr_validator chain was added by commit 3ad7d2468f79f ("Ipvlan
should return an error when an address is already in use") to allow
address validation before changes are committed and to be able to
fail the address change with an error back to the user. The address
validation is not done for addresses received from router
advertisements.
Handling RAs in softirq context is the only reason for the notifier
chain to be atomic versus blocking. Since the only current user, ipvlan,
of the validator chain ignores softirq context, the notifier can be made
blocking and simply not invoked for softirq path.
The blocking option is needed by spectrum for example to validate
resources for an adding an address to an interface.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/ipv6/addrconf.c | 24 +++++++++++++++---------
net/ipv6/addrconf_core.c | 9 +++++----
2 files changed, 20 insertions(+), 13 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index d9f6226694eb..632cf4b26277 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -963,7 +963,6 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
struct net *net = dev_net(idev->dev);
struct inet6_ifaddr *ifa = NULL;
struct rt6_info *rt;
- struct in6_validator_info i6vi;
unsigned int hash;
int err = 0;
int addr_type = ipv6_addr_type(addr);
@@ -988,16 +987,23 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
goto out2;
}
- i6vi.i6vi_addr = *addr;
- i6vi.i6vi_dev = idev;
- rcu_read_unlock_bh();
+ /* validator notifier needs to be blocking;
+ * do not call in softirq context
+ */
+ if (!in_softirq()) {
+ struct in6_validator_info i6vi = {
+ .i6vi_addr = *addr,
+ .i6vi_dev = idev,
+ };
- err = inet6addr_validator_notifier_call_chain(NETDEV_UP, &i6vi);
+ rcu_read_unlock_bh();
+ err = inet6addr_validator_notifier_call_chain(NETDEV_UP, &i6vi);
+ rcu_read_lock_bh();
- rcu_read_lock_bh();
- err = notifier_to_errno(err);
- if (err)
- goto out2;
+ err = notifier_to_errno(err);
+ if (err)
+ goto out2;
+ }
spin_lock(&addrconf_hash_lock);
diff --git a/net/ipv6/addrconf_core.c b/net/ipv6/addrconf_core.c
index 9e3488d50b15..32b564dfd02a 100644
--- a/net/ipv6/addrconf_core.c
+++ b/net/ipv6/addrconf_core.c
@@ -88,7 +88,7 @@ int __ipv6_addr_type(const struct in6_addr *addr)
EXPORT_SYMBOL(__ipv6_addr_type);
static ATOMIC_NOTIFIER_HEAD(inet6addr_chain);
-static ATOMIC_NOTIFIER_HEAD(inet6addr_validator_chain);
+static BLOCKING_NOTIFIER_HEAD(inet6addr_validator_chain);
int register_inet6addr_notifier(struct notifier_block *nb)
{
@@ -110,19 +110,20 @@ EXPORT_SYMBOL(inet6addr_notifier_call_chain);
int register_inet6addr_validator_notifier(struct notifier_block *nb)
{
- return atomic_notifier_chain_register(&inet6addr_validator_chain, nb);
+ return blocking_notifier_chain_register(&inet6addr_validator_chain, nb);
}
EXPORT_SYMBOL(register_inet6addr_validator_notifier);
int unregister_inet6addr_validator_notifier(struct notifier_block *nb)
{
- return atomic_notifier_chain_unregister(&inet6addr_validator_chain, nb);
+ return blocking_notifier_chain_unregister(&inet6addr_validator_chain,
+ nb);
}
EXPORT_SYMBOL(unregister_inet6addr_validator_notifier);
int inet6addr_validator_notifier_call_chain(unsigned long val, void *v)
{
- return atomic_notifier_call_chain(&inet6addr_validator_chain, val, v);
+ return blocking_notifier_call_chain(&inet6addr_validator_chain, val, v);
}
EXPORT_SYMBOL(inet6addr_validator_notifier_call_chain);
--
2.1.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [RFC net-next 1/4] net: ipv6: Make inet6addr_validator a blocking notifier
2017-10-10 16:41 ` [RFC net-next 1/4] net: ipv6: Make inet6addr_validator a blocking notifier David Ahern
@ 2017-10-10 19:32 ` David Ahern
2017-10-11 13:08 ` Ido Schimmel
2017-10-11 21:13 ` David Miller
2 siblings, 0 replies; 15+ messages in thread
From: David Ahern @ 2017-10-10 19:32 UTC (permalink / raw)
To: netdev, idosch; +Cc: jiri, kjlx
On 10/10/17 10:41 AM, David Ahern wrote:
> @@ -988,16 +987,23 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
> goto out2;
> }
>
> - i6vi.i6vi_addr = *addr;
> - i6vi.i6vi_dev = idev;
> - rcu_read_unlock_bh();
> + /* validator notifier needs to be blocking;
> + * do not call in softirq context
> + */
> + if (!in_softirq()) {
> + struct in6_validator_info i6vi = {
> + .i6vi_addr = *addr,
> + .i6vi_dev = idev,
> + };
>
> - err = inet6addr_validator_notifier_call_chain(NETDEV_UP, &i6vi);
> + rcu_read_unlock_bh();
> + err = inet6addr_validator_notifier_call_chain(NETDEV_UP, &i6vi);
> + rcu_read_lock_bh();
>
> - rcu_read_lock_bh();
> - err = notifier_to_errno(err);
> - if (err)
> - goto out2;
> + err = notifier_to_errno(err);
> + if (err)
> + goto out2;
> + }
>
> spin_lock(&addrconf_hash_lock);
>
The rcu_read_unlock_bh needs to be done before the in_softirq check.
With the change below I get the RIF overload with IPv6 addresses and I
verified the validator is skipped for RAs.
$ ip -batch vlan-ipv6-addr-batch
Error: spectrum: Exceeded number of supported router interfaces.
Command failed vlan-ipv6-addr-batch:683
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 0bad4a800f73..d9c5b29a3b8b 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -988,6 +988,8 @@ ipv6_add_addr(struct inet6_dev *idev, const struct
in6_addr *addr,
goto out2;
}
+ rcu_read_unlock_bh();
+
/* validator notifier needs to be blocking;
* do not call in softirq context
*/
@@ -998,15 +1000,14 @@ ipv6_add_addr(struct inet6_dev *idev, const
struct in6_addr *addr,
.extack = extack,
};
- rcu_read_unlock_bh();
err = inet6addr_validator_notifier_call_chain(NETDEV_UP,
&i6vi);
- rcu_read_lock_bh();
-
err = notifier_to_errno(err);
if (err)
- goto out2;
+ goto out1;
}
+ rcu_read_lock_bh();
+
spin_lock(&addrconf_hash_lock);
/* Ignore adding duplicate addresses on an interface */
@@ -1079,7 +1080,7 @@ ipv6_add_addr(struct inet6_dev *idev, const struct
in6_addr *addr,
write_unlock(&idev->lock);
out2:
rcu_read_unlock_bh();
-
+out1:
if (likely(err == 0))
inet6addr_notifier_call_chain(NETDEV_UP, ifa);
else {
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [RFC net-next 1/4] net: ipv6: Make inet6addr_validator a blocking notifier
2017-10-10 16:41 ` [RFC net-next 1/4] net: ipv6: Make inet6addr_validator a blocking notifier David Ahern
2017-10-10 19:32 ` David Ahern
@ 2017-10-11 13:08 ` Ido Schimmel
2017-10-11 21:13 ` David Miller
2 siblings, 0 replies; 15+ messages in thread
From: Ido Schimmel @ 2017-10-11 13:08 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, jiri, idosch, kjlx
On Tue, Oct 10, 2017 at 09:41:02AM -0700, David Ahern wrote:
> inet6addr_validator chain was added by commit 3ad7d2468f79f ("Ipvlan
> should return an error when an address is already in use") to allow
> address validation before changes are committed and to be able to
> fail the address change with an error back to the user. The address
> validation is not done for addresses received from router
> advertisements.
>
> Handling RAs in softirq context is the only reason for the notifier
> chain to be atomic versus blocking. Since the only current user, ipvlan,
> of the validator chain ignores softirq context, the notifier can be made
> blocking and simply not invoked for softirq path.
>
> The blocking option is needed by spectrum for example to validate
> resources for an adding an address to an interface.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
With the fixup posted later:
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
BTW, the in_softirq() check in ipvlan_addr6_validator_event() can be
removed after this patch.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC net-next 1/4] net: ipv6: Make inet6addr_validator a blocking notifier
2017-10-10 16:41 ` [RFC net-next 1/4] net: ipv6: Make inet6addr_validator a blocking notifier David Ahern
2017-10-10 19:32 ` David Ahern
2017-10-11 13:08 ` Ido Schimmel
@ 2017-10-11 21:13 ` David Miller
2017-10-11 21:56 ` David Ahern
2 siblings, 1 reply; 15+ messages in thread
From: David Miller @ 2017-10-11 21:13 UTC (permalink / raw)
To: dsahern; +Cc: netdev, jiri, idosch, kjlx
From: David Ahern <dsahern@gmail.com>
Date: Tue, 10 Oct 2017 09:41:02 -0700
> + /* validator notifier needs to be blocking;
> + * do not call in softirq context
> + */
> + if (!in_softirq()) {
I think we can test this better.
You should be able to audit the call sites and for each one set the
value of a new boolean argument properly, and this way you can also
give the boolean argument a descriptive name.
Furthermore, we can also then pull the inet6_addr allocation out of
the locking paths and thus use GFP_KERNEL when possible.
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC net-next 1/4] net: ipv6: Make inet6addr_validator a blocking notifier
2017-10-11 21:13 ` David Miller
@ 2017-10-11 21:56 ` David Ahern
0 siblings, 0 replies; 15+ messages in thread
From: David Ahern @ 2017-10-11 21:56 UTC (permalink / raw)
To: David Miller; +Cc: netdev, jiri, idosch, kjlx
On 10/11/17 3:13 PM, David Miller wrote:
> From: David Ahern <dsahern@gmail.com>
> Date: Tue, 10 Oct 2017 09:41:02 -0700
>
>> + /* validator notifier needs to be blocking;
>> + * do not call in softirq context
>> + */
>> + if (!in_softirq()) {
>
> I think we can test this better.
The callchain we are protecting against is
7fff8149d0dd ipv6_add_addr ([kernel.kallsyms])
7fff814a161b addrconf_prefix_rcv ([kernel.kallsyms])
7fff814afb8a ndisc_router_discovery ([kernel.kallsyms])
7fff814b0310 ndisc_rcv ([kernel.kallsyms])
7fff814b62da icmpv6_rcv ([kernel.kallsyms])
7fff81499c37 ip6_input_finish ([kernel.kallsyms])
7fff81499e96 ip6_input ([kernel.kallsyms])
7fff8149a519 ip6_mc_input ([kernel.kallsyms])
7fff81499f9d ip6_rcv_finish ([kernel.kallsyms])
7fff8149a349 ipv6_rcv ([kernel.kallsyms])
7fff813fbe12 __netif_receive_skb_core ([kernel.kallsyms])
7fff813fc04c __netif_receive_skb ([kernel.kallsyms])
7fff813ff97c netif_receive_skb_internal ([kernel.kallsyms])
>
> You should be able to audit the call sites and for each one set the
> value of a new boolean argument properly, and this way you can also
> give the boolean argument a descriptive name.
The safest is an in_atomic() check, but to your point I'll see if the
caller can pass in atomic vs blocking option as a bool.
>
> Furthermore, we can also then pull the inet6_addr allocation out of
> the locking paths and thus use GFP_KERNEL when possible.
>
Yes, I was thinking about that as a follow on -- how far down can the
rcu_read_lock_bh be pushed.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC net-next 2/4] net: Add extack to validator_info structs used for address notifier
2017-10-10 16:41 [RFC net-next 0/4] mlxsw: spectrum_router: Add extack messages for RIF and VRF overflow David Ahern
2017-10-10 16:41 ` [RFC net-next 1/4] net: ipv6: Make inet6addr_validator a blocking notifier David Ahern
@ 2017-10-10 16:41 ` David Ahern
2017-10-11 13:37 ` Ido Schimmel
2017-10-10 16:41 ` [RFC net-next 3/4] mlxsw: spectrum: router: Add support for address validator notifier David Ahern
2017-10-10 16:41 ` [RFC net-next 4/4] mlxsw: spectrum_router: Add extack message for RIF and VRF overflow David Ahern
3 siblings, 1 reply; 15+ messages in thread
From: David Ahern @ 2017-10-10 16:41 UTC (permalink / raw)
To: netdev; +Cc: jiri, idosch, kjlx, David Ahern
Add extack to in_validator_info and in6_validator_info. Update the one
user of each, ipvlan, to return an error message for failures.
Only manual configuration of an address is plumbed in the IPv6 code path.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
drivers/net/ipvlan/ipvlan_main.c | 10 ++++++++--
include/linux/inetdevice.h | 1 +
include/net/addrconf.h | 1 +
net/ipv4/devinet.c | 8 +++++---
net/ipv6/addrconf.c | 23 +++++++++++++----------
5 files changed, 28 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 57c3856bab05..56a868415ba2 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -846,8 +846,11 @@ static int ipvlan_addr6_validator_event(struct notifier_block *unused,
switch (event) {
case NETDEV_UP:
- if (ipvlan_addr_busy(ipvlan->port, &i6vi->i6vi_addr, true))
+ if (ipvlan_addr_busy(ipvlan->port, &i6vi->i6vi_addr, true)) {
+ NL_SET_ERR_MSG(i6vi->extack,
+ "Address already assigned to an ipvlan device");
return notifier_from_errno(-EADDRINUSE);
+ }
break;
}
@@ -916,8 +919,11 @@ static int ipvlan_addr4_validator_event(struct notifier_block *unused,
switch (event) {
case NETDEV_UP:
- if (ipvlan_addr_busy(ipvlan->port, &ivi->ivi_addr, false))
+ if (ipvlan_addr_busy(ipvlan->port, &ivi->ivi_addr, false)) {
+ NL_SET_ERR_MSG(ivi->extack,
+ "Address already assigned to an ipvlan device");
return notifier_from_errno(-EADDRINUSE);
+ }
break;
}
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index 751d051f0bc7..681dff30940b 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -154,6 +154,7 @@ struct in_ifaddr {
struct in_validator_info {
__be32 ivi_addr;
struct in_device *ivi_dev;
+ struct netlink_ext_ack *extack;
};
int register_inetaddr_notifier(struct notifier_block *nb);
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 87981cd63180..b8b16437c6d5 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -55,6 +55,7 @@ struct prefix_info {
struct in6_validator_info {
struct in6_addr i6vi_addr;
struct inet6_dev *i6vi_dev;
+ struct netlink_ext_ack *extack;
};
#define IN6_ADDR_HSIZE_SHIFT 4
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 7ce22a2c07ce..0118698cd623 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -444,7 +444,7 @@ static void check_lifetime(struct work_struct *work);
static DECLARE_DELAYED_WORK(check_lifetime_work, check_lifetime);
static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
- u32 portid)
+ u32 portid, struct netlink_ext_ack *extack)
{
struct in_device *in_dev = ifa->ifa_dev;
struct in_ifaddr *ifa1, **ifap, **last_primary;
@@ -489,6 +489,7 @@ static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
*/
ivi.ivi_addr = ifa->ifa_address;
ivi.ivi_dev = ifa->ifa_dev;
+ ivi.extack = extack;
ret = blocking_notifier_call_chain(&inetaddr_validator_chain,
NETDEV_UP, &ivi);
ret = notifier_to_errno(ret);
@@ -521,7 +522,7 @@ static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
static int inet_insert_ifa(struct in_ifaddr *ifa)
{
- return __inet_insert_ifa(ifa, NULL, 0);
+ return __inet_insert_ifa(ifa, NULL, 0, NULL);
}
static int inet_set_ifa(struct net_device *dev, struct in_ifaddr *ifa)
@@ -902,7 +903,8 @@ static int inet_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
return ret;
}
}
- return __inet_insert_ifa(ifa, nlh, NETLINK_CB(skb).portid);
+ return __inet_insert_ifa(ifa, nlh, NETLINK_CB(skb).portid,
+ extack);
} else {
inet_free_ifa(ifa);
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 632cf4b26277..0bad4a800f73 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -958,7 +958,8 @@ static u32 inet6_addr_hash(const struct in6_addr *addr)
static struct inet6_ifaddr *
ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
const struct in6_addr *peer_addr, int pfxlen,
- int scope, u32 flags, u32 valid_lft, u32 prefered_lft)
+ int scope, u32 flags, u32 valid_lft, u32 prefered_lft,
+ struct netlink_ext_ack *extack)
{
struct net *net = dev_net(idev->dev);
struct inet6_ifaddr *ifa = NULL;
@@ -994,6 +995,7 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
struct in6_validator_info i6vi = {
.i6vi_addr = *addr,
.i6vi_dev = idev,
+ .extack = extack,
};
rcu_read_unlock_bh();
@@ -1336,7 +1338,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *i
ift = ipv6_add_addr(idev, &addr, NULL, tmp_plen,
ipv6_addr_scope(&addr), addr_flags,
- tmp_valid_lft, tmp_prefered_lft);
+ tmp_valid_lft, tmp_prefered_lft, NULL);
if (IS_ERR(ift)) {
in6_ifa_put(ifp);
in6_dev_put(idev);
@@ -2020,7 +2022,7 @@ void addrconf_dad_failure(struct inet6_ifaddr *ifp)
ifp2 = ipv6_add_addr(idev, &new_addr, NULL, pfxlen,
scope, flags, valid_lft,
- preferred_lft);
+ preferred_lft, NULL);
if (IS_ERR(ifp2))
goto lock_errdad;
@@ -2478,7 +2480,7 @@ int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
pinfo->prefix_len,
addr_type&IPV6_ADDR_SCOPE_MASK,
addr_flags, valid_lft,
- prefered_lft);
+ prefered_lft, NULL);
if (IS_ERR_OR_NULL(ifp))
return -1;
@@ -2788,7 +2790,8 @@ static int inet6_addr_add(struct net *net, int ifindex,
const struct in6_addr *pfx,
const struct in6_addr *peer_pfx,
unsigned int plen, __u32 ifa_flags,
- __u32 prefered_lft, __u32 valid_lft)
+ __u32 prefered_lft, __u32 valid_lft,
+ struct netlink_ext_ack *extack)
{
struct inet6_ifaddr *ifp;
struct inet6_dev *idev;
@@ -2847,7 +2850,7 @@ static int inet6_addr_add(struct net *net, int ifindex,
}
ifp = ipv6_add_addr(idev, pfx, peer_pfx, plen, scope, ifa_flags,
- valid_lft, prefered_lft);
+ valid_lft, prefered_lft, extack);
if (!IS_ERR(ifp)) {
if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
@@ -2932,7 +2935,7 @@ int addrconf_add_ifaddr(struct net *net, void __user *arg)
rtnl_lock();
err = inet6_addr_add(net, ireq.ifr6_ifindex, &ireq.ifr6_addr, NULL,
ireq.ifr6_prefixlen, IFA_F_PERMANENT,
- INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
+ INFINITY_LIFE_TIME, INFINITY_LIFE_TIME, NULL);
rtnl_unlock();
return err;
}
@@ -2962,7 +2965,7 @@ static void add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
ifp = ipv6_add_addr(idev, addr, NULL, plen,
scope, IFA_F_PERMANENT,
- INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
+ INFINITY_LIFE_TIME, INFINITY_LIFE_TIME, NULL);
if (!IS_ERR(ifp)) {
spin_lock_bh(&ifp->lock);
ifp->flags &= ~IFA_F_TENTATIVE;
@@ -3062,7 +3065,7 @@ void addrconf_add_linklocal(struct inet6_dev *idev,
#endif
ifp = ipv6_add_addr(idev, addr, NULL, 64, IFA_LINK, addr_flags,
- INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
+ INFINITY_LIFE_TIME, INFINITY_LIFE_TIME, NULL);
if (!IS_ERR(ifp)) {
addrconf_prefix_route(&ifp->addr, ifp->prefix_len, idev->dev, 0, 0);
addrconf_dad_start(ifp);
@@ -4565,7 +4568,7 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
*/
return inet6_addr_add(net, ifm->ifa_index, pfx, peer_pfx,
ifm->ifa_prefixlen, ifa_flags,
- preferred_lft, valid_lft);
+ preferred_lft, valid_lft, extack);
}
if (nlh->nlmsg_flags & NLM_F_EXCL ||
--
2.1.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [RFC net-next 2/4] net: Add extack to validator_info structs used for address notifier
2017-10-10 16:41 ` [RFC net-next 2/4] net: Add extack to validator_info structs used for address notifier David Ahern
@ 2017-10-11 13:37 ` Ido Schimmel
0 siblings, 0 replies; 15+ messages in thread
From: Ido Schimmel @ 2017-10-11 13:37 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, jiri, idosch, kjlx
On Tue, Oct 10, 2017 at 09:41:03AM -0700, David Ahern wrote:
> Add extack to in_validator_info and in6_validator_info. Update the one
> user of each, ipvlan, to return an error message for failures.
>
> Only manual configuration of an address is plumbed in the IPv6 code path.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
See two nits below. Otherwise:
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
> ---
> drivers/net/ipvlan/ipvlan_main.c | 10 ++++++++--
> include/linux/inetdevice.h | 1 +
> include/net/addrconf.h | 1 +
> net/ipv4/devinet.c | 8 +++++---
> net/ipv6/addrconf.c | 23 +++++++++++++----------
> 5 files changed, 28 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
> index 57c3856bab05..56a868415ba2 100644
> --- a/drivers/net/ipvlan/ipvlan_main.c
> +++ b/drivers/net/ipvlan/ipvlan_main.c
> @@ -846,8 +846,11 @@ static int ipvlan_addr6_validator_event(struct notifier_block *unused,
>
> switch (event) {
> case NETDEV_UP:
> - if (ipvlan_addr_busy(ipvlan->port, &i6vi->i6vi_addr, true))
> + if (ipvlan_addr_busy(ipvlan->port, &i6vi->i6vi_addr, true)) {
> + NL_SET_ERR_MSG(i6vi->extack,
> + "Address already assigned to an ipvlan device");
> return notifier_from_errno(-EADDRINUSE);
> + }
> break;
> }
>
> @@ -916,8 +919,11 @@ static int ipvlan_addr4_validator_event(struct notifier_block *unused,
>
> switch (event) {
> case NETDEV_UP:
> - if (ipvlan_addr_busy(ipvlan->port, &ivi->ivi_addr, false))
> + if (ipvlan_addr_busy(ipvlan->port, &ivi->ivi_addr, false)) {
> + NL_SET_ERR_MSG(ivi->extack,
> + "Address already assigned to an ipvlan device");
> return notifier_from_errno(-EADDRINUSE);
> + }
> break;
> }
>
> diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
> index 751d051f0bc7..681dff30940b 100644
> --- a/include/linux/inetdevice.h
> +++ b/include/linux/inetdevice.h
> @@ -154,6 +154,7 @@ struct in_ifaddr {
> struct in_validator_info {
> __be32 ivi_addr;
> struct in_device *ivi_dev;
> + struct netlink_ext_ack *extack;
> };
>
> int register_inetaddr_notifier(struct notifier_block *nb);
> diff --git a/include/net/addrconf.h b/include/net/addrconf.h
> index 87981cd63180..b8b16437c6d5 100644
> --- a/include/net/addrconf.h
> +++ b/include/net/addrconf.h
> @@ -55,6 +55,7 @@ struct prefix_info {
> struct in6_validator_info {
> struct in6_addr i6vi_addr;
> struct inet6_dev *i6vi_dev;
> + struct netlink_ext_ack *extack;
> };
>
> #define IN6_ADDR_HSIZE_SHIFT 4
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index 7ce22a2c07ce..0118698cd623 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -444,7 +444,7 @@ static void check_lifetime(struct work_struct *work);
> static DECLARE_DELAYED_WORK(check_lifetime_work, check_lifetime);
>
> static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
> - u32 portid)
> + u32 portid, struct netlink_ext_ack *extack)
Double space before `*extack`.
> {
> struct in_device *in_dev = ifa->ifa_dev;
> struct in_ifaddr *ifa1, **ifap, **last_primary;
> @@ -489,6 +489,7 @@ static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
> */
> ivi.ivi_addr = ifa->ifa_address;
> ivi.ivi_dev = ifa->ifa_dev;
> + ivi.extack = extack;
> ret = blocking_notifier_call_chain(&inetaddr_validator_chain,
> NETDEV_UP, &ivi);
> ret = notifier_to_errno(ret);
> @@ -521,7 +522,7 @@ static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
>
> static int inet_insert_ifa(struct in_ifaddr *ifa)
> {
> - return __inet_insert_ifa(ifa, NULL, 0);
> + return __inet_insert_ifa(ifa, NULL, 0, NULL);
> }
>
> static int inet_set_ifa(struct net_device *dev, struct in_ifaddr *ifa)
> @@ -902,7 +903,8 @@ static int inet_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
> return ret;
> }
> }
> - return __inet_insert_ifa(ifa, nlh, NETLINK_CB(skb).portid);
> + return __inet_insert_ifa(ifa, nlh, NETLINK_CB(skb).portid,
> + extack);
> } else {
> inet_free_ifa(ifa);
>
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 632cf4b26277..0bad4a800f73 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -958,7 +958,8 @@ static u32 inet6_addr_hash(const struct in6_addr *addr)
> static struct inet6_ifaddr *
> ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
> const struct in6_addr *peer_addr, int pfxlen,
> - int scope, u32 flags, u32 valid_lft, u32 prefered_lft)
> + int scope, u32 flags, u32 valid_lft, u32 prefered_lft,
> + struct netlink_ext_ack *extack)
Double space before `*extack`.
> {
> struct net *net = dev_net(idev->dev);
> struct inet6_ifaddr *ifa = NULL;
> @@ -994,6 +995,7 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
> struct in6_validator_info i6vi = {
> .i6vi_addr = *addr,
> .i6vi_dev = idev,
> + .extack = extack,
> };
>
> rcu_read_unlock_bh();
> @@ -1336,7 +1338,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *i
>
> ift = ipv6_add_addr(idev, &addr, NULL, tmp_plen,
> ipv6_addr_scope(&addr), addr_flags,
> - tmp_valid_lft, tmp_prefered_lft);
> + tmp_valid_lft, tmp_prefered_lft, NULL);
> if (IS_ERR(ift)) {
> in6_ifa_put(ifp);
> in6_dev_put(idev);
> @@ -2020,7 +2022,7 @@ void addrconf_dad_failure(struct inet6_ifaddr *ifp)
>
> ifp2 = ipv6_add_addr(idev, &new_addr, NULL, pfxlen,
> scope, flags, valid_lft,
> - preferred_lft);
> + preferred_lft, NULL);
> if (IS_ERR(ifp2))
> goto lock_errdad;
>
> @@ -2478,7 +2480,7 @@ int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
> pinfo->prefix_len,
> addr_type&IPV6_ADDR_SCOPE_MASK,
> addr_flags, valid_lft,
> - prefered_lft);
> + prefered_lft, NULL);
>
> if (IS_ERR_OR_NULL(ifp))
> return -1;
> @@ -2788,7 +2790,8 @@ static int inet6_addr_add(struct net *net, int ifindex,
> const struct in6_addr *pfx,
> const struct in6_addr *peer_pfx,
> unsigned int plen, __u32 ifa_flags,
> - __u32 prefered_lft, __u32 valid_lft)
> + __u32 prefered_lft, __u32 valid_lft,
> + struct netlink_ext_ack *extack)
> {
> struct inet6_ifaddr *ifp;
> struct inet6_dev *idev;
> @@ -2847,7 +2850,7 @@ static int inet6_addr_add(struct net *net, int ifindex,
> }
>
> ifp = ipv6_add_addr(idev, pfx, peer_pfx, plen, scope, ifa_flags,
> - valid_lft, prefered_lft);
> + valid_lft, prefered_lft, extack);
>
> if (!IS_ERR(ifp)) {
> if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
> @@ -2932,7 +2935,7 @@ int addrconf_add_ifaddr(struct net *net, void __user *arg)
> rtnl_lock();
> err = inet6_addr_add(net, ireq.ifr6_ifindex, &ireq.ifr6_addr, NULL,
> ireq.ifr6_prefixlen, IFA_F_PERMANENT,
> - INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
> + INFINITY_LIFE_TIME, INFINITY_LIFE_TIME, NULL);
> rtnl_unlock();
> return err;
> }
> @@ -2962,7 +2965,7 @@ static void add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
>
> ifp = ipv6_add_addr(idev, addr, NULL, plen,
> scope, IFA_F_PERMANENT,
> - INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
> + INFINITY_LIFE_TIME, INFINITY_LIFE_TIME, NULL);
> if (!IS_ERR(ifp)) {
> spin_lock_bh(&ifp->lock);
> ifp->flags &= ~IFA_F_TENTATIVE;
> @@ -3062,7 +3065,7 @@ void addrconf_add_linklocal(struct inet6_dev *idev,
> #endif
>
> ifp = ipv6_add_addr(idev, addr, NULL, 64, IFA_LINK, addr_flags,
> - INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
> + INFINITY_LIFE_TIME, INFINITY_LIFE_TIME, NULL);
> if (!IS_ERR(ifp)) {
> addrconf_prefix_route(&ifp->addr, ifp->prefix_len, idev->dev, 0, 0);
> addrconf_dad_start(ifp);
> @@ -4565,7 +4568,7 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
> */
> return inet6_addr_add(net, ifm->ifa_index, pfx, peer_pfx,
> ifm->ifa_prefixlen, ifa_flags,
> - preferred_lft, valid_lft);
> + preferred_lft, valid_lft, extack);
> }
>
> if (nlh->nlmsg_flags & NLM_F_EXCL ||
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC net-next 3/4] mlxsw: spectrum: router: Add support for address validator notifier
2017-10-10 16:41 [RFC net-next 0/4] mlxsw: spectrum_router: Add extack messages for RIF and VRF overflow David Ahern
2017-10-10 16:41 ` [RFC net-next 1/4] net: ipv6: Make inet6addr_validator a blocking notifier David Ahern
2017-10-10 16:41 ` [RFC net-next 2/4] net: Add extack to validator_info structs used for address notifier David Ahern
@ 2017-10-10 16:41 ` David Ahern
2017-10-11 13:54 ` Ido Schimmel
2017-10-10 16:41 ` [RFC net-next 4/4] mlxsw: spectrum_router: Add extack message for RIF and VRF overflow David Ahern
3 siblings, 1 reply; 15+ messages in thread
From: David Ahern @ 2017-10-10 16:41 UTC (permalink / raw)
To: netdev; +Cc: jiri, idosch, kjlx, David Ahern
Add support for inetaddr_validator and inet6addr_validator. The
notifiers provide a means for validating ipv4 and ipv6 addresses
before the addresses are installed and on failure the error
is propagated back to the user.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 10 ++++
drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 4 ++
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 53 ++++++++++++++++++++++
3 files changed, 67 insertions(+)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 321988ac57cc..da4ee91235be 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4505,11 +4505,19 @@ static struct notifier_block mlxsw_sp_netdevice_nb __read_mostly = {
.notifier_call = mlxsw_sp_netdevice_event,
};
+static struct notifier_block mlxsw_sp_inetaddr_valid_nb __read_mostly = {
+ .notifier_call = mlxsw_sp_inetaddr_valid_event,
+};
+
static struct notifier_block mlxsw_sp_inetaddr_nb __read_mostly = {
.notifier_call = mlxsw_sp_inetaddr_event,
.priority = 10, /* Must be called before FIB notifier block */
};
+static struct notifier_block mlxsw_sp_inet6addr_valid_nb __read_mostly = {
+ .notifier_call = mlxsw_sp_inet6addr_valid_event,
+};
+
static struct notifier_block mlxsw_sp_inet6addr_nb __read_mostly = {
.notifier_call = mlxsw_sp_inet6addr_event,
};
@@ -4533,7 +4541,9 @@ static int __init mlxsw_sp_module_init(void)
int err;
register_netdevice_notifier(&mlxsw_sp_netdevice_nb);
+ register_inetaddr_validator_notifier(&mlxsw_sp_inetaddr_valid_nb);
register_inetaddr_notifier(&mlxsw_sp_inetaddr_nb);
+ register_inet6addr_validator_notifier(&mlxsw_sp_inet6addr_valid_nb);
register_inet6addr_notifier(&mlxsw_sp_inet6addr_nb);
register_netevent_notifier(&mlxsw_sp_router_netevent_nb);
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 8e45183dc9bb..4865a6f58c83 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -390,8 +390,12 @@ int mlxsw_sp_router_netevent_event(struct notifier_block *unused,
int mlxsw_sp_netdevice_router_port_event(struct net_device *dev);
int mlxsw_sp_inetaddr_event(struct notifier_block *unused,
unsigned long event, void *ptr);
+int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
+ unsigned long event, void *ptr);
int mlxsw_sp_inet6addr_event(struct notifier_block *unused,
unsigned long event, void *ptr);
+int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
+ unsigned long event, void *ptr);
int mlxsw_sp_netdevice_vrf_event(struct net_device *l3_dev, unsigned long event,
struct netdev_notifier_changeupper_info *info);
void
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 6a356f4b99a3..7d53fdf2c0a8 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -5656,6 +5656,33 @@ int mlxsw_sp_inetaddr_event(struct notifier_block *unused,
struct mlxsw_sp_rif *rif;
int err = 0;
+ /* NETDEV_UP event is handled by mlxsw_sp_inetaddr_valid_event */
+ if (event == NETDEV_UP)
+ goto out;
+
+ mlxsw_sp = mlxsw_sp_lower_get(dev);
+ if (!mlxsw_sp)
+ goto out;
+
+ rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
+ if (!mlxsw_sp_rif_should_config(rif, dev, event))
+ goto out;
+
+ err = __mlxsw_sp_inetaddr_event(dev, event);
+out:
+ return notifier_from_errno(err);
+}
+
+/* only expected to be called for event == NETDEV_UP */
+int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
+ unsigned long event, void *ptr)
+{
+ struct in_validator_info *ivi = (struct in_validator_info *)ptr;
+ struct net_device *dev = ivi->ivi_dev->dev;
+ struct mlxsw_sp *mlxsw_sp;
+ struct mlxsw_sp_rif *rif;
+ int err = 0;
+
mlxsw_sp = mlxsw_sp_lower_get(dev);
if (!mlxsw_sp)
goto out;
@@ -5708,6 +5735,10 @@ int mlxsw_sp_inet6addr_event(struct notifier_block *unused,
struct mlxsw_sp_inet6addr_event_work *inet6addr_work;
struct net_device *dev = if6->idev->dev;
+ /* NETDEV_UP event is handled by mlxsw_sp_inet6addr_valid_event */
+ if (event == NETDEV_UP)
+ return NOTIFY_DONE;
+
if (!mlxsw_sp_port_dev_lower_find_rcu(dev))
return NOTIFY_DONE;
@@ -5724,6 +5755,28 @@ int mlxsw_sp_inet6addr_event(struct notifier_block *unused,
return NOTIFY_DONE;
}
+int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
+ unsigned long event, void *ptr)
+{
+ struct in6_validator_info *i6vi = (struct in6_validator_info *)ptr;
+ struct net_device *dev = i6vi->i6vi_dev->dev;
+ struct mlxsw_sp *mlxsw_sp;
+ struct mlxsw_sp_rif *rif;
+ int err = 0;
+
+ mlxsw_sp = mlxsw_sp_lower_get(dev);
+ if (!mlxsw_sp)
+ goto out;
+
+ rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
+ if (!mlxsw_sp_rif_should_config(rif, dev, event))
+ goto out;
+
+ err = __mlxsw_sp_inetaddr_event(dev, event);
+out:
+ return notifier_from_errno(err);
+}
+
static int mlxsw_sp_rif_edit(struct mlxsw_sp *mlxsw_sp, u16 rif_index,
const char *mac, int mtu)
{
--
2.1.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [RFC net-next 3/4] mlxsw: spectrum: router: Add support for address validator notifier
2017-10-10 16:41 ` [RFC net-next 3/4] mlxsw: spectrum: router: Add support for address validator notifier David Ahern
@ 2017-10-11 13:54 ` Ido Schimmel
2017-10-11 15:19 ` David Ahern
0 siblings, 1 reply; 15+ messages in thread
From: Ido Schimmel @ 2017-10-11 13:54 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, jiri, idosch, kjlx
On Tue, Oct 10, 2017 at 09:41:04AM -0700, David Ahern wrote:
> Add support for inetaddr_validator and inet6addr_validator. The
> notifiers provide a means for validating ipv4 and ipv6 addresses
> before the addresses are installed and on failure the error
> is propagated back to the user.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
> ---
> drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 10 ++++
> drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 4 ++
> .../net/ethernet/mellanox/mlxsw/spectrum_router.c | 53 ++++++++++++++++++++++
> 3 files changed, 67 insertions(+)
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> index 321988ac57cc..da4ee91235be 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
> @@ -4505,11 +4505,19 @@ static struct notifier_block mlxsw_sp_netdevice_nb __read_mostly = {
> .notifier_call = mlxsw_sp_netdevice_event,
> };
>
> +static struct notifier_block mlxsw_sp_inetaddr_valid_nb __read_mostly = {
> + .notifier_call = mlxsw_sp_inetaddr_valid_event,
> +};
> +
> static struct notifier_block mlxsw_sp_inetaddr_nb __read_mostly = {
> .notifier_call = mlxsw_sp_inetaddr_event,
> .priority = 10, /* Must be called before FIB notifier block */
This line can now be removed since the validator notifier is called
before the inetaddr notifier.
It's used to create a RIF before fib_add_ifaddr() is invoked to create a
prefix route. IPv6 doesn't need that since routes aren't created based
on a NETDEV_UP event in inet6addr.
> };
>
> +static struct notifier_block mlxsw_sp_inet6addr_valid_nb __read_mostly = {
> + .notifier_call = mlxsw_sp_inet6addr_valid_event,
> +};
> +
> static struct notifier_block mlxsw_sp_inet6addr_nb __read_mostly = {
> .notifier_call = mlxsw_sp_inet6addr_event,
> };
> @@ -4533,7 +4541,9 @@ static int __init mlxsw_sp_module_init(void)
> int err;
>
> register_netdevice_notifier(&mlxsw_sp_netdevice_nb);
> + register_inetaddr_validator_notifier(&mlxsw_sp_inetaddr_valid_nb);
> register_inetaddr_notifier(&mlxsw_sp_inetaddr_nb);
> + register_inet6addr_validator_notifier(&mlxsw_sp_inet6addr_valid_nb);
> register_inet6addr_notifier(&mlxsw_sp_inet6addr_nb);
> register_netevent_notifier(&mlxsw_sp_router_netevent_nb);
Need unregister in rollback and __exit function.
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> index 8e45183dc9bb..4865a6f58c83 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
> @@ -390,8 +390,12 @@ int mlxsw_sp_router_netevent_event(struct notifier_block *unused,
> int mlxsw_sp_netdevice_router_port_event(struct net_device *dev);
> int mlxsw_sp_inetaddr_event(struct notifier_block *unused,
> unsigned long event, void *ptr);
> +int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
> + unsigned long event, void *ptr);
> int mlxsw_sp_inet6addr_event(struct notifier_block *unused,
> unsigned long event, void *ptr);
> +int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
> + unsigned long event, void *ptr);
> int mlxsw_sp_netdevice_vrf_event(struct net_device *l3_dev, unsigned long event,
> struct netdev_notifier_changeupper_info *info);
> void
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> index 6a356f4b99a3..7d53fdf2c0a8 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> @@ -5656,6 +5656,33 @@ int mlxsw_sp_inetaddr_event(struct notifier_block *unused,
> struct mlxsw_sp_rif *rif;
> int err = 0;
>
> + /* NETDEV_UP event is handled by mlxsw_sp_inetaddr_valid_event */
> + if (event == NETDEV_UP)
> + goto out;
> +
> + mlxsw_sp = mlxsw_sp_lower_get(dev);
> + if (!mlxsw_sp)
> + goto out;
> +
> + rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
> + if (!mlxsw_sp_rif_should_config(rif, dev, event))
> + goto out;
> +
> + err = __mlxsw_sp_inetaddr_event(dev, event);
> +out:
> + return notifier_from_errno(err);
> +}
> +
> +/* only expected to be called for event == NETDEV_UP */
I don't really mind, but I don't think the comment is necessary.
> +int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
> + unsigned long event, void *ptr)
> +{
> + struct in_validator_info *ivi = (struct in_validator_info *)ptr;
We usually put a space after the cast in mlxsw (I'm aware of the
checkpatch CHECK).
> + struct net_device *dev = ivi->ivi_dev->dev;
> + struct mlxsw_sp *mlxsw_sp;
> + struct mlxsw_sp_rif *rif;
> + int err = 0;
> +
> mlxsw_sp = mlxsw_sp_lower_get(dev);
> if (!mlxsw_sp)
> goto out;
> @@ -5708,6 +5735,10 @@ int mlxsw_sp_inet6addr_event(struct notifier_block *unused,
> struct mlxsw_sp_inet6addr_event_work *inet6addr_work;
> struct net_device *dev = if6->idev->dev;
>
> + /* NETDEV_UP event is handled by mlxsw_sp_inet6addr_valid_event */
> + if (event == NETDEV_UP)
> + return NOTIFY_DONE;
> +
> if (!mlxsw_sp_port_dev_lower_find_rcu(dev))
> return NOTIFY_DONE;
>
> @@ -5724,6 +5755,28 @@ int mlxsw_sp_inet6addr_event(struct notifier_block *unused,
> return NOTIFY_DONE;
> }
>
> +int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
> + unsigned long event, void *ptr)
> +{
> + struct in6_validator_info *i6vi = (struct in6_validator_info *)ptr;
Same as above.
Looks good otherwise, thanks!
> + struct net_device *dev = i6vi->i6vi_dev->dev;
> + struct mlxsw_sp *mlxsw_sp;
> + struct mlxsw_sp_rif *rif;
> + int err = 0;
> +
> + mlxsw_sp = mlxsw_sp_lower_get(dev);
> + if (!mlxsw_sp)
> + goto out;
> +
> + rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
> + if (!mlxsw_sp_rif_should_config(rif, dev, event))
> + goto out;
> +
> + err = __mlxsw_sp_inetaddr_event(dev, event);
> +out:
> + return notifier_from_errno(err);
> +}
> +
> static int mlxsw_sp_rif_edit(struct mlxsw_sp *mlxsw_sp, u16 rif_index,
> const char *mac, int mtu)
> {
> --
> 2.1.4
>
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC net-next 3/4] mlxsw: spectrum: router: Add support for address validator notifier
2017-10-11 13:54 ` Ido Schimmel
@ 2017-10-11 15:19 ` David Ahern
0 siblings, 0 replies; 15+ messages in thread
From: David Ahern @ 2017-10-11 15:19 UTC (permalink / raw)
To: Ido Schimmel; +Cc: netdev, jiri, idosch, kjlx
On 10/11/17 7:54 AM, Ido Schimmel wrote:
>> @@ -4533,7 +4541,9 @@ static int __init mlxsw_sp_module_init(void)
>> int err;
>>
>> register_netdevice_notifier(&mlxsw_sp_netdevice_nb);
>> + register_inetaddr_validator_notifier(&mlxsw_sp_inetaddr_valid_nb);
>> register_inetaddr_notifier(&mlxsw_sp_inetaddr_nb);
>> + register_inet6addr_validator_notifier(&mlxsw_sp_inet6addr_valid_nb);
>> register_inet6addr_notifier(&mlxsw_sp_inet6addr_nb);
>> register_netevent_notifier(&mlxsw_sp_router_netevent_nb);
> Need unregister in rollback and __exit function.
>
oops, thanks for noticing that. Fixed and took care of your other comments.
^ permalink raw reply [flat|nested] 15+ messages in thread
* [RFC net-next 4/4] mlxsw: spectrum_router: Add extack message for RIF and VRF overflow
2017-10-10 16:41 [RFC net-next 0/4] mlxsw: spectrum_router: Add extack messages for RIF and VRF overflow David Ahern
` (2 preceding siblings ...)
2017-10-10 16:41 ` [RFC net-next 3/4] mlxsw: spectrum: router: Add support for address validator notifier David Ahern
@ 2017-10-10 16:41 ` David Ahern
2017-10-11 14:13 ` Ido Schimmel
3 siblings, 1 reply; 15+ messages in thread
From: David Ahern @ 2017-10-10 16:41 UTC (permalink / raw)
To: netdev; +Cc: jiri, idosch, kjlx, David Ahern
Add extack argument down to mlxsw_sp_rif_create and mlxsw_sp_vr_create
to set an error message on RIF or VR overflow. Now an overflow of
either resource the use gets an informative message as opposed to
failing with EBUSY.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 114 +++++++++++++--------
1 file changed, 69 insertions(+), 45 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 7d53fdf2c0a8..ec4d313b9eca 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -731,14 +731,17 @@ static struct mlxsw_sp_fib *mlxsw_sp_vr_fib(const struct mlxsw_sp_vr *vr,
}
static struct mlxsw_sp_vr *mlxsw_sp_vr_create(struct mlxsw_sp *mlxsw_sp,
- u32 tb_id)
+ u32 tb_id,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp_vr *vr;
int err;
vr = mlxsw_sp_vr_find_unused(mlxsw_sp);
- if (!vr)
+ if (!vr) {
+ NL_SET_ERR_MSG(extack, "spectrum: Exceeded number of supported VRF");
return ERR_PTR(-EBUSY);
+ }
vr->fib4 = mlxsw_sp_fib_create(vr, MLXSW_SP_L3_PROTO_IPV4);
if (IS_ERR(vr->fib4))
return ERR_CAST(vr->fib4);
@@ -775,14 +778,15 @@ static void mlxsw_sp_vr_destroy(struct mlxsw_sp_vr *vr)
vr->fib4 = NULL;
}
-static struct mlxsw_sp_vr *mlxsw_sp_vr_get(struct mlxsw_sp *mlxsw_sp, u32 tb_id)
+static struct mlxsw_sp_vr *mlxsw_sp_vr_get(struct mlxsw_sp *mlxsw_sp, u32 tb_id,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp_vr *vr;
tb_id = mlxsw_sp_fix_tb_id(tb_id);
vr = mlxsw_sp_vr_find(mlxsw_sp, tb_id);
if (!vr)
- vr = mlxsw_sp_vr_create(mlxsw_sp, tb_id);
+ vr = mlxsw_sp_vr_create(mlxsw_sp, tb_id, extack);
return vr;
}
@@ -948,7 +952,8 @@ static u32 mlxsw_sp_ipip_dev_ul_tb_id(const struct net_device *ol_dev)
static struct mlxsw_sp_rif *
mlxsw_sp_rif_create(struct mlxsw_sp *mlxsw_sp,
- const struct mlxsw_sp_rif_params *params);
+ const struct mlxsw_sp_rif_params *params,
+ struct netlink_ext_ack *extack);
static struct mlxsw_sp_rif_ipip_lb *
mlxsw_sp_ipip_ol_ipip_lb_create(struct mlxsw_sp *mlxsw_sp,
@@ -966,7 +971,7 @@ mlxsw_sp_ipip_ol_ipip_lb_create(struct mlxsw_sp *mlxsw_sp,
.lb_config = ipip_ops->ol_loopback_config(mlxsw_sp, ol_dev),
};
- rif = mlxsw_sp_rif_create(mlxsw_sp, &lb_params.common);
+ rif = mlxsw_sp_rif_create(mlxsw_sp, &lb_params.common, NULL);
if (IS_ERR(rif))
return ERR_CAST(rif);
return container_of(rif, struct mlxsw_sp_rif_ipip_lb, common);
@@ -3711,7 +3716,7 @@ mlxsw_sp_fib_node_get(struct mlxsw_sp *mlxsw_sp, u32 tb_id, const void *addr,
struct mlxsw_sp_vr *vr;
int err;
- vr = mlxsw_sp_vr_get(mlxsw_sp, tb_id);
+ vr = mlxsw_sp_vr_get(mlxsw_sp, tb_id, NULL);
if (IS_ERR(vr))
return ERR_CAST(vr);
fib = mlxsw_sp_vr_fib(vr, proto);
@@ -4750,7 +4755,7 @@ static int mlxsw_sp_router_fibmr_add(struct mlxsw_sp *mlxsw_sp,
if (mlxsw_sp->router->aborted)
return 0;
- vr = mlxsw_sp_vr_get(mlxsw_sp, men_info->tb_id);
+ vr = mlxsw_sp_vr_get(mlxsw_sp, men_info->tb_id, NULL);
if (IS_ERR(vr))
return PTR_ERR(vr);
@@ -4783,7 +4788,7 @@ mlxsw_sp_router_fibmr_vif_add(struct mlxsw_sp *mlxsw_sp,
if (mlxsw_sp->router->aborted)
return 0;
- vr = mlxsw_sp_vr_get(mlxsw_sp, ven_info->tb_id);
+ vr = mlxsw_sp_vr_get(mlxsw_sp, ven_info->tb_id, NULL);
if (IS_ERR(vr))
return PTR_ERR(vr);
@@ -5346,7 +5351,8 @@ const struct net_device *mlxsw_sp_rif_dev(const struct mlxsw_sp_rif *rif)
static struct mlxsw_sp_rif *
mlxsw_sp_rif_create(struct mlxsw_sp *mlxsw_sp,
- const struct mlxsw_sp_rif_params *params)
+ const struct mlxsw_sp_rif_params *params,
+ struct netlink_ext_ack *extack)
{
u32 tb_id = l3mdev_fib_table(params->dev);
const struct mlxsw_sp_rif_ops *ops;
@@ -5360,14 +5366,16 @@ mlxsw_sp_rif_create(struct mlxsw_sp *mlxsw_sp,
type = mlxsw_sp_dev_rif_type(mlxsw_sp, params->dev);
ops = mlxsw_sp->router->rif_ops_arr[type];
- vr = mlxsw_sp_vr_get(mlxsw_sp, tb_id ? : RT_TABLE_MAIN);
+ vr = mlxsw_sp_vr_get(mlxsw_sp, tb_id ? : RT_TABLE_MAIN, extack);
if (IS_ERR(vr))
return ERR_CAST(vr);
vr->rif_count++;
err = mlxsw_sp_rif_index_alloc(mlxsw_sp, &rif_index);
- if (err)
+ if (err) {
+ NL_SET_ERR_MSG(extack, "spectrum: Exceeded number of supported router interfaces");
goto err_rif_index_alloc;
+ }
rif = mlxsw_sp_rif_alloc(ops->rif_size, rif_index, vr->id, params->dev);
if (!rif) {
@@ -5454,7 +5462,8 @@ mlxsw_sp_rif_subport_params_init(struct mlxsw_sp_rif_params *params,
static int
mlxsw_sp_port_vlan_router_join(struct mlxsw_sp_port_vlan *mlxsw_sp_port_vlan,
- struct net_device *l3_dev)
+ struct net_device *l3_dev,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp_port *mlxsw_sp_port = mlxsw_sp_port_vlan->mlxsw_sp_port;
struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
@@ -5470,7 +5479,7 @@ mlxsw_sp_port_vlan_router_join(struct mlxsw_sp_port_vlan *mlxsw_sp_port_vlan,
};
mlxsw_sp_rif_subport_params_init(¶ms, mlxsw_sp_port_vlan);
- rif = mlxsw_sp_rif_create(mlxsw_sp, ¶ms);
+ rif = mlxsw_sp_rif_create(mlxsw_sp, ¶ms, extack);
if (IS_ERR(rif))
return PTR_ERR(rif);
}
@@ -5525,7 +5534,8 @@ mlxsw_sp_port_vlan_router_leave(struct mlxsw_sp_port_vlan *mlxsw_sp_port_vlan)
static int mlxsw_sp_inetaddr_port_vlan_event(struct net_device *l3_dev,
struct net_device *port_dev,
- unsigned long event, u16 vid)
+ unsigned long event, u16 vid,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(port_dev);
struct mlxsw_sp_port_vlan *mlxsw_sp_port_vlan;
@@ -5537,7 +5547,7 @@ static int mlxsw_sp_inetaddr_port_vlan_event(struct net_device *l3_dev,
switch (event) {
case NETDEV_UP:
return mlxsw_sp_port_vlan_router_join(mlxsw_sp_port_vlan,
- l3_dev);
+ l3_dev, extack);
case NETDEV_DOWN:
mlxsw_sp_port_vlan_router_leave(mlxsw_sp_port_vlan);
break;
@@ -5547,19 +5557,22 @@ static int mlxsw_sp_inetaddr_port_vlan_event(struct net_device *l3_dev,
}
static int mlxsw_sp_inetaddr_port_event(struct net_device *port_dev,
- unsigned long event)
+ unsigned long event,
+ struct netlink_ext_ack *extack)
{
if (netif_is_bridge_port(port_dev) ||
netif_is_lag_port(port_dev) ||
netif_is_ovs_port(port_dev))
return 0;
- return mlxsw_sp_inetaddr_port_vlan_event(port_dev, port_dev, event, 1);
+ return mlxsw_sp_inetaddr_port_vlan_event(port_dev, port_dev, event, 1,
+ extack);
}
static int __mlxsw_sp_inetaddr_lag_event(struct net_device *l3_dev,
struct net_device *lag_dev,
- unsigned long event, u16 vid)
+ unsigned long event, u16 vid,
+ struct netlink_ext_ack *extack)
{
struct net_device *port_dev;
struct list_head *iter;
@@ -5569,7 +5582,8 @@ static int __mlxsw_sp_inetaddr_lag_event(struct net_device *l3_dev,
if (mlxsw_sp_port_dev_check(port_dev)) {
err = mlxsw_sp_inetaddr_port_vlan_event(l3_dev,
port_dev,
- event, vid);
+ event, vid,
+ extack);
if (err)
return err;
}
@@ -5579,16 +5593,19 @@ static int __mlxsw_sp_inetaddr_lag_event(struct net_device *l3_dev,
}
static int mlxsw_sp_inetaddr_lag_event(struct net_device *lag_dev,
- unsigned long event)
+ unsigned long event,
+ struct netlink_ext_ack *extack)
{
if (netif_is_bridge_port(lag_dev))
return 0;
- return __mlxsw_sp_inetaddr_lag_event(lag_dev, lag_dev, event, 1);
+ return __mlxsw_sp_inetaddr_lag_event(lag_dev, lag_dev, event, 1,
+ extack);
}
static int mlxsw_sp_inetaddr_bridge_event(struct net_device *l3_dev,
- unsigned long event)
+ unsigned long event,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp *mlxsw_sp = mlxsw_sp_lower_get(l3_dev);
struct mlxsw_sp_rif_params params = {
@@ -5598,7 +5615,7 @@ static int mlxsw_sp_inetaddr_bridge_event(struct net_device *l3_dev,
switch (event) {
case NETDEV_UP:
- rif = mlxsw_sp_rif_create(mlxsw_sp, ¶ms);
+ rif = mlxsw_sp_rif_create(mlxsw_sp, ¶ms, extack);
if (IS_ERR(rif))
return PTR_ERR(rif);
break;
@@ -5612,7 +5629,8 @@ static int mlxsw_sp_inetaddr_bridge_event(struct net_device *l3_dev,
}
static int mlxsw_sp_inetaddr_vlan_event(struct net_device *vlan_dev,
- unsigned long event)
+ unsigned long event,
+ struct netlink_ext_ack *extack)
{
struct net_device *real_dev = vlan_dev_real_dev(vlan_dev);
u16 vid = vlan_dev_vlan_id(vlan_dev);
@@ -5622,27 +5640,28 @@ static int mlxsw_sp_inetaddr_vlan_event(struct net_device *vlan_dev,
if (mlxsw_sp_port_dev_check(real_dev))
return mlxsw_sp_inetaddr_port_vlan_event(vlan_dev, real_dev,
- event, vid);
+ event, vid, extack);
else if (netif_is_lag_master(real_dev))
return __mlxsw_sp_inetaddr_lag_event(vlan_dev, real_dev, event,
- vid);
+ vid, extack);
else if (netif_is_bridge_master(real_dev) && br_vlan_enabled(real_dev))
- return mlxsw_sp_inetaddr_bridge_event(vlan_dev, event);
+ return mlxsw_sp_inetaddr_bridge_event(vlan_dev, event, extack);
return 0;
}
static int __mlxsw_sp_inetaddr_event(struct net_device *dev,
- unsigned long event)
+ unsigned long event,
+ struct netlink_ext_ack *extack)
{
if (mlxsw_sp_port_dev_check(dev))
- return mlxsw_sp_inetaddr_port_event(dev, event);
+ return mlxsw_sp_inetaddr_port_event(dev, event, extack);
else if (netif_is_lag_master(dev))
- return mlxsw_sp_inetaddr_lag_event(dev, event);
+ return mlxsw_sp_inetaddr_lag_event(dev, event, extack);
else if (netif_is_bridge_master(dev))
- return mlxsw_sp_inetaddr_bridge_event(dev, event);
+ return mlxsw_sp_inetaddr_bridge_event(dev, event, extack);
else if (is_vlan_dev(dev))
- return mlxsw_sp_inetaddr_vlan_event(dev, event);
+ return mlxsw_sp_inetaddr_vlan_event(dev, event, extack);
else
return 0;
}
@@ -5668,7 +5687,7 @@ int mlxsw_sp_inetaddr_event(struct notifier_block *unused,
if (!mlxsw_sp_rif_should_config(rif, dev, event))
goto out;
- err = __mlxsw_sp_inetaddr_event(dev, event);
+ err = __mlxsw_sp_inetaddr_event(dev, event, NULL);
out:
return notifier_from_errno(err);
}
@@ -5691,7 +5710,7 @@ int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
if (!mlxsw_sp_rif_should_config(rif, dev, event))
goto out;
- err = __mlxsw_sp_inetaddr_event(dev, event);
+ err = __mlxsw_sp_inetaddr_event(dev, event, ivi->extack);
out:
return notifier_from_errno(err);
}
@@ -5720,7 +5739,7 @@ static void mlxsw_sp_inet6addr_event_work(struct work_struct *work)
if (!mlxsw_sp_rif_should_config(rif, dev, event))
goto out;
- __mlxsw_sp_inetaddr_event(dev, event);
+ __mlxsw_sp_inetaddr_event(dev, event, NULL);
out:
rtnl_unlock();
dev_put(dev);
@@ -5772,7 +5791,7 @@ int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
if (!mlxsw_sp_rif_should_config(rif, dev, event))
goto out;
- err = __mlxsw_sp_inetaddr_event(dev, event);
+ err = __mlxsw_sp_inetaddr_event(dev, event, i6vi->extack);
out:
return notifier_from_errno(err);
}
@@ -5849,7 +5868,8 @@ int mlxsw_sp_netdevice_router_port_event(struct net_device *dev)
}
static int mlxsw_sp_port_vrf_join(struct mlxsw_sp *mlxsw_sp,
- struct net_device *l3_dev)
+ struct net_device *l3_dev,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp_rif *rif;
@@ -5858,9 +5878,9 @@ static int mlxsw_sp_port_vrf_join(struct mlxsw_sp *mlxsw_sp,
*/
rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, l3_dev);
if (rif)
- __mlxsw_sp_inetaddr_event(l3_dev, NETDEV_DOWN);
+ __mlxsw_sp_inetaddr_event(l3_dev, NETDEV_DOWN, extack);
- return __mlxsw_sp_inetaddr_event(l3_dev, NETDEV_UP);
+ return __mlxsw_sp_inetaddr_event(l3_dev, NETDEV_UP, extack);
}
static void mlxsw_sp_port_vrf_leave(struct mlxsw_sp *mlxsw_sp,
@@ -5871,7 +5891,7 @@ static void mlxsw_sp_port_vrf_leave(struct mlxsw_sp *mlxsw_sp,
rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, l3_dev);
if (!rif)
return;
- __mlxsw_sp_inetaddr_event(l3_dev, NETDEV_DOWN);
+ __mlxsw_sp_inetaddr_event(l3_dev, NETDEV_DOWN, NULL);
}
int mlxsw_sp_netdevice_vrf_event(struct net_device *l3_dev, unsigned long event,
@@ -5887,10 +5907,14 @@ int mlxsw_sp_netdevice_vrf_event(struct net_device *l3_dev, unsigned long event,
case NETDEV_PRECHANGEUPPER:
return 0;
case NETDEV_CHANGEUPPER:
- if (info->linking)
- err = mlxsw_sp_port_vrf_join(mlxsw_sp, l3_dev);
- else
+ if (info->linking) {
+ struct netlink_ext_ack *extack;
+
+ extack = netdev_notifier_info_to_extack(&info->info);
+ err = mlxsw_sp_port_vrf_join(mlxsw_sp, l3_dev, extack);
+ } else {
mlxsw_sp_port_vrf_leave(mlxsw_sp, l3_dev);
+ }
break;
}
@@ -6197,7 +6221,7 @@ mlxsw_sp_rif_ipip_lb_configure(struct mlxsw_sp_rif *rif)
struct mlxsw_sp_vr *ul_vr;
int err;
- ul_vr = mlxsw_sp_vr_get(mlxsw_sp, ul_tb_id);
+ ul_vr = mlxsw_sp_vr_get(mlxsw_sp, ul_tb_id, NULL);
if (IS_ERR(ul_vr))
return PTR_ERR(ul_vr);
--
2.1.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [RFC net-next 4/4] mlxsw: spectrum_router: Add extack message for RIF and VRF overflow
2017-10-10 16:41 ` [RFC net-next 4/4] mlxsw: spectrum_router: Add extack message for RIF and VRF overflow David Ahern
@ 2017-10-11 14:13 ` Ido Schimmel
2017-10-11 15:07 ` David Ahern
0 siblings, 1 reply; 15+ messages in thread
From: Ido Schimmel @ 2017-10-11 14:13 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, jiri, idosch, kjlx
On Tue, Oct 10, 2017 at 09:41:05AM -0700, David Ahern wrote:
> Add extack argument down to mlxsw_sp_rif_create and mlxsw_sp_vr_create
> to set an error message on RIF or VR overflow. Now an overflow of
> either resource the use gets an informative message as opposed to
s/the/in/ ?
> failing with EBUSY.
>
> Signed-off-by: David Ahern <dsahern@gmail.com>
One comment below, but other than that:
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
I'll run some tests and report if anything blows up.
Thanks David!
> ---
> .../net/ethernet/mellanox/mlxsw/spectrum_router.c | 114 +++++++++++++--------
> 1 file changed, 69 insertions(+), 45 deletions(-)
>
> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> index 7d53fdf2c0a8..ec4d313b9eca 100644
> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
> @@ -731,14 +731,17 @@ static struct mlxsw_sp_fib *mlxsw_sp_vr_fib(const struct mlxsw_sp_vr *vr,
> }
>
> static struct mlxsw_sp_vr *mlxsw_sp_vr_create(struct mlxsw_sp *mlxsw_sp,
> - u32 tb_id)
> + u32 tb_id,
> + struct netlink_ext_ack *extack)
> {
> struct mlxsw_sp_vr *vr;
> int err;
>
> vr = mlxsw_sp_vr_find_unused(mlxsw_sp);
> - if (!vr)
> + if (!vr) {
> + NL_SET_ERR_MSG(extack, "spectrum: Exceeded number of supported VRF");
Maybe:
"spectrum: Exceeded number of supported VRF devices"
To be consistent with previously added:
"spectrum: Exceeded number of supported LAG devices"
> return ERR_PTR(-EBUSY);
> + }
> vr->fib4 = mlxsw_sp_fib_create(vr, MLXSW_SP_L3_PROTO_IPV4);
> if (IS_ERR(vr->fib4))
> return ERR_CAST(vr->fib4);
> @@ -775,14 +778,15 @@ static void mlxsw_sp_vr_destroy(struct mlxsw_sp_vr *vr)
> vr->fib4 = NULL;
> }
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC net-next 4/4] mlxsw: spectrum_router: Add extack message for RIF and VRF overflow
2017-10-11 14:13 ` Ido Schimmel
@ 2017-10-11 15:07 ` David Ahern
2017-10-11 15:10 ` Ido Schimmel
0 siblings, 1 reply; 15+ messages in thread
From: David Ahern @ 2017-10-11 15:07 UTC (permalink / raw)
To: Ido Schimmel; +Cc: netdev, jiri, idosch, kjlx
On 10/11/17 8:13 AM, Ido Schimmel wrote:
> On Tue, Oct 10, 2017 at 09:41:05AM -0700, David Ahern wrote:
>> Add extack argument down to mlxsw_sp_rif_create and mlxsw_sp_vr_create
>> to set an error message on RIF or VR overflow. Now an overflow of
>> either resource the use gets an informative message as opposed to
>
> s/the/in/ ?
'use' is supposed to be 'user'
>> diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
>> index 7d53fdf2c0a8..ec4d313b9eca 100644
>> --- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
>> +++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
>> @@ -731,14 +731,17 @@ static struct mlxsw_sp_fib *mlxsw_sp_vr_fib(const struct mlxsw_sp_vr *vr,
>> }
>>
>> static struct mlxsw_sp_vr *mlxsw_sp_vr_create(struct mlxsw_sp *mlxsw_sp,
>> - u32 tb_id)
>> + u32 tb_id,
>> + struct netlink_ext_ack *extack)
>> {
>> struct mlxsw_sp_vr *vr;
>> int err;
>>
>> vr = mlxsw_sp_vr_find_unused(mlxsw_sp);
>> - if (!vr)
>> + if (!vr) {
>> + NL_SET_ERR_MSG(extack, "spectrum: Exceeded number of supported VRF");
>
> Maybe:
> "spectrum: Exceeded number of supported VRF devices"
In this context the overflow is virtual routers in spectrum as opposed
to VRF devices in the kernel context. The existence of the VRF device
has no bearing until a port is enslaved to it.
How about:
"spectrum: Exceeded number of supported virtual routers"
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [RFC net-next 4/4] mlxsw: spectrum_router: Add extack message for RIF and VRF overflow
2017-10-11 15:07 ` David Ahern
@ 2017-10-11 15:10 ` Ido Schimmel
0 siblings, 0 replies; 15+ messages in thread
From: Ido Schimmel @ 2017-10-11 15:10 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, jiri, idosch, kjlx
On Wed, Oct 11, 2017 at 09:07:20AM -0600, David Ahern wrote:
> On 10/11/17 8:13 AM, Ido Schimmel wrote:
> > On Tue, Oct 10, 2017 at 09:41:05AM -0700, David Ahern wrote:
> >> static struct mlxsw_sp_vr *mlxsw_sp_vr_create(struct mlxsw_sp *mlxsw_sp,
> >> - u32 tb_id)
> >> + u32 tb_id,
> >> + struct netlink_ext_ack *extack)
> >> {
> >> struct mlxsw_sp_vr *vr;
> >> int err;
> >>
> >> vr = mlxsw_sp_vr_find_unused(mlxsw_sp);
> >> - if (!vr)
> >> + if (!vr) {
> >> + NL_SET_ERR_MSG(extack, "spectrum: Exceeded number of supported VRF");
> >
> > Maybe:
> > "spectrum: Exceeded number of supported VRF devices"
>
> In this context the overflow is virtual routers in spectrum as opposed
> to VRF devices in the kernel context. The existence of the VRF device
> has no bearing until a port is enslaved to it.
>
> How about:
> "spectrum: Exceeded number of supported virtual routers"
OK.
^ permalink raw reply [flat|nested] 15+ messages in thread