* [PATCH net-next 0/7] net: Add address attribute to control metric of prefix route
From: dsahern @ 2018-05-27 15:09 UTC (permalink / raw)
To: netdev; +Cc: roopa, David Ahern
From: David Ahern <dsahern@gmail.com>
For use cases such as VRR (Virtual Router Redundancy) interface managers
want efficient control over the order of prefix routes when multiple
interfaces have addresses with overlapping/duplicate subnets.
Currently, if two interfaces have addresses in the same subnet, the order
of the prefix route entries is determined by the order in which the
addresses are assigned or the links brought up. Any actions like cycling
an interface up and down changes that order. This set adds a new attribute
for addresses to allow a user to specify the metric of the prefix route
associated with an address giving interface managers better and more
efficient control of the order of prefix routes.
Patches 1-3 refactor IPv6 address add functions to pass an ifa6_config
struct. The functions currently have a long list of arguments and adding
the metric just makes it worse. Because of the overall diff size in
moving the arguments to a struct, the change is done in stages to make
it easier to review starting with the bottom function and pushing the
struct up to callers in each successive patch.
Patch 4 introduces the new attribute.
Patches 5 and 6 add support for the new attribute to IPv4 and IPv6
addresses.
Patch 7 adds a set of test cases.
Patch 8 adds support to iproute2
Changes since RFC
- collapsed patches 1 and 3 into patch 2
- simplified stack variables in fib_modify_prefix_metric in patch 5
David Ahern (7):
net/ipv6: Convert ipv6_add_addr to struct ifa6_config
net/ipv6: Pass ifa6_config struct to inet6_addr_add
net/ipv6: Pass ifa6_config struct to inet6_addr_modify
net: Add IFA_RT_PRIORITY address attribute
net/ipv4: Add support for specifying metric of connected routes
net/ipv6: Add support for specifying metric of connected routes
selftests: fib_tests: Add prefix route tests with metric
include/linux/inetdevice.h | 1 +
include/net/addrconf.h | 13 ++
include/net/if_inet6.h | 1 +
include/net/route.h | 1 +
include/uapi/linux/if_addr.h | 1 +
net/ipv4/devinet.c | 15 ++
net/ipv4/fib_frontend.c | 53 ++++-
net/ipv6/addrconf.c | 359 +++++++++++++++++++------------
tools/testing/selftests/net/fib_tests.sh | 181 +++++++++++++++-
9 files changed, 472 insertions(+), 153 deletions(-)
mode change 100755 => 100644 tools/testing/selftests/net/fib_tests.sh
--
2.11.0
^ permalink raw reply
* [PATCH net-next 1/7] net/ipv6: Convert ipv6_add_addr to struct ifa6_config
From: dsahern @ 2018-05-27 15:09 UTC (permalink / raw)
To: netdev; +Cc: roopa, David Ahern
In-Reply-To: <20180527151000.30488-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
Move config parameters for adding an ipv6 address to a struct. struct
names stem from inet6_rtm_newaddr which is the modern handler for
adding an address.
Start the conversion to ifa6_config with ipv6_add_addr. This is an argument
move only; no functional change intended. Mapping of variable changes:
addr --> cfg->pfx
peer_addr --> cfg->peer_pfx
pfxlen --> cfg->plen
flags --> cfg->ifa_flags
scope, valid_lft, prefered_lft have the same names within cfg
(with corrected spelling).
Signed-off-by: David Ahern <dsahern@gmail.com>
---
include/net/addrconf.h | 12 +++++
net/ipv6/addrconf.c | 134 +++++++++++++++++++++++++++----------------------
2 files changed, 87 insertions(+), 59 deletions(-)
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index c07d4dd09361..f766af2cd1a4 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -59,6 +59,18 @@ struct in6_validator_info {
struct netlink_ext_ack *extack;
};
+struct ifa6_config {
+ const struct in6_addr *pfx;
+ unsigned int plen;
+
+ const struct in6_addr *peer_pfx;
+
+ u32 ifa_flags;
+ u32 preferred_lft;
+ u32 valid_lft;
+ u16 scope;
+};
+
int addrconf_init(void);
void addrconf_cleanup(void);
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index fbfd71a2d9c8..4988f2265882 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -986,17 +986,15 @@ static int ipv6_add_addr_hash(struct net_device *dev, struct inet6_ifaddr *ifa)
/* On success it returns ifp with increased reference count */
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,
+ipv6_add_addr(struct inet6_dev *idev, struct ifa6_config *cfg,
bool can_block, struct netlink_ext_ack *extack)
{
gfp_t gfp_flags = can_block ? GFP_KERNEL : GFP_ATOMIC;
+ int addr_type = ipv6_addr_type(cfg->pfx);
struct net *net = dev_net(idev->dev);
struct inet6_ifaddr *ifa = NULL;
struct fib6_info *f6i = NULL;
int err = 0;
- int addr_type = ipv6_addr_type(addr);
if (addr_type == IPV6_ADDR_ANY ||
addr_type & IPV6_ADDR_MULTICAST ||
@@ -1019,7 +1017,7 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
*/
if (can_block) {
struct in6_validator_info i6vi = {
- .i6vi_addr = *addr,
+ .i6vi_addr = *cfg->pfx,
.i6vi_dev = idev,
.extack = extack,
};
@@ -1036,7 +1034,7 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
goto out;
}
- f6i = addrconf_f6i_alloc(net, idev, addr, false, gfp_flags);
+ f6i = addrconf_f6i_alloc(net, idev, cfg->pfx, false, gfp_flags);
if (IS_ERR(f6i)) {
err = PTR_ERR(f6i);
f6i = NULL;
@@ -1049,21 +1047,21 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
neigh_parms_data_state_setall(idev->nd_parms);
- ifa->addr = *addr;
- if (peer_addr)
- ifa->peer_addr = *peer_addr;
+ ifa->addr = *cfg->pfx;
+ if (cfg->peer_pfx)
+ ifa->peer_addr = *cfg->peer_pfx;
spin_lock_init(&ifa->lock);
INIT_DELAYED_WORK(&ifa->dad_work, addrconf_dad_work);
INIT_HLIST_NODE(&ifa->addr_lst);
- ifa->scope = scope;
- ifa->prefix_len = pfxlen;
- ifa->flags = flags;
+ ifa->scope = cfg->scope;
+ ifa->prefix_len = cfg->plen;
+ ifa->flags = cfg->ifa_flags;
/* No need to add the TENTATIVE flag for addresses with NODAD */
- if (!(flags & IFA_F_NODAD))
+ if (!(cfg->ifa_flags & IFA_F_NODAD))
ifa->flags |= IFA_F_TENTATIVE;
- ifa->valid_lft = valid_lft;
- ifa->prefered_lft = prefered_lft;
+ ifa->valid_lft = cfg->valid_lft;
+ ifa->prefered_lft = cfg->preferred_lft;
ifa->cstamp = ifa->tstamp = jiffies;
ifa->tokenized = false;
@@ -1260,11 +1258,10 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp,
{
struct inet6_dev *idev = ifp->idev;
struct in6_addr addr, *tmpaddr;
- unsigned long tmp_prefered_lft, tmp_valid_lft, tmp_tstamp, age;
+ unsigned long tmp_tstamp, age;
unsigned long regen_advance;
- int tmp_plen;
+ struct ifa6_config cfg;
int ret = 0;
- u32 addr_flags;
unsigned long now = jiffies;
long max_desync_factor;
s32 cnf_temp_preferred_lft;
@@ -1326,13 +1323,12 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp,
}
}
- tmp_valid_lft = min_t(__u32,
- ifp->valid_lft,
+ cfg.valid_lft = min_t(__u32, ifp->valid_lft,
idev->cnf.temp_valid_lft + age);
- tmp_prefered_lft = cnf_temp_preferred_lft + age -
- idev->desync_factor;
- tmp_prefered_lft = min_t(__u32, ifp->prefered_lft, tmp_prefered_lft);
- tmp_plen = ifp->prefix_len;
+ cfg.preferred_lft = cnf_temp_preferred_lft + age - idev->desync_factor;
+ cfg.preferred_lft = min_t(__u32, ifp->prefered_lft, cfg.preferred_lft);
+
+ cfg.plen = ifp->prefix_len;
tmp_tstamp = ifp->tstamp;
spin_unlock_bh(&ifp->lock);
@@ -1346,21 +1342,22 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp,
* temporary addresses being generated.
*/
age = (now - tmp_tstamp + ADDRCONF_TIMER_FUZZ_MINUS) / HZ;
- if (tmp_prefered_lft <= regen_advance + age) {
+ if (cfg.preferred_lft <= regen_advance + age) {
in6_ifa_put(ifp);
in6_dev_put(idev);
ret = -1;
goto out;
}
- addr_flags = IFA_F_TEMPORARY;
+ cfg.ifa_flags = IFA_F_TEMPORARY;
/* set in addrconf_prefix_rcv() */
if (ifp->flags & IFA_F_OPTIMISTIC)
- addr_flags |= IFA_F_OPTIMISTIC;
+ cfg.ifa_flags |= IFA_F_OPTIMISTIC;
+
+ cfg.pfx = &addr;
+ cfg.scope = ipv6_addr_scope(cfg.pfx);
- ift = ipv6_add_addr(idev, &addr, NULL, tmp_plen,
- ipv6_addr_scope(&addr), addr_flags,
- tmp_valid_lft, tmp_prefered_lft, block, NULL);
+ ift = ipv6_add_addr(idev, &cfg, block, NULL);
if (IS_ERR(ift)) {
in6_ifa_put(ifp);
in6_dev_put(idev);
@@ -2031,13 +2028,17 @@ void addrconf_dad_failure(struct sk_buff *skb, struct inet6_ifaddr *ifp)
spin_lock_bh(&ifp->lock);
if (ifp->flags & IFA_F_STABLE_PRIVACY) {
- int scope = ifp->scope;
- u32 flags = ifp->flags;
struct in6_addr new_addr;
struct inet6_ifaddr *ifp2;
- u32 valid_lft, preferred_lft;
- int pfxlen = ifp->prefix_len;
int retries = ifp->stable_privacy_retry + 1;
+ struct ifa6_config cfg = {
+ .pfx = &new_addr,
+ .plen = ifp->prefix_len,
+ .ifa_flags = ifp->flags,
+ .valid_lft = ifp->valid_lft,
+ .preferred_lft = ifp->prefered_lft,
+ .scope = ifp->scope,
+ };
if (retries > net->ipv6.sysctl.idgen_retries) {
net_info_ratelimited("%s: privacy stable address generation failed because of DAD conflicts!\n",
@@ -2050,9 +2051,6 @@ void addrconf_dad_failure(struct sk_buff *skb, struct inet6_ifaddr *ifp)
idev))
goto errdad;
- valid_lft = ifp->valid_lft;
- preferred_lft = ifp->prefered_lft;
-
spin_unlock_bh(&ifp->lock);
if (idev->cnf.max_addresses &&
@@ -2063,9 +2061,7 @@ void addrconf_dad_failure(struct sk_buff *skb, struct inet6_ifaddr *ifp)
net_info_ratelimited("%s: generating new stable privacy address because of DAD conflict\n",
ifp->idev->dev->name);
- ifp2 = ipv6_add_addr(idev, &new_addr, NULL, pfxlen,
- scope, flags, valid_lft,
- preferred_lft, false, NULL);
+ ifp2 = ipv6_add_addr(idev, &cfg, false, NULL);
if (IS_ERR(ifp2))
goto lock_errdad;
@@ -2507,12 +2503,20 @@ int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
if (!ifp && valid_lft) {
int max_addresses = in6_dev->cnf.max_addresses;
+ struct ifa6_config cfg = {
+ .pfx = addr,
+ .plen = pinfo->prefix_len,
+ .ifa_flags = addr_flags,
+ .valid_lft = valid_lft,
+ .preferred_lft = prefered_lft,
+ .scope = addr_type & IPV6_ADDR_SCOPE_MASK,
+ };
#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
if ((net->ipv6.devconf_all->optimistic_dad ||
in6_dev->cnf.optimistic_dad) &&
!net->ipv6.devconf_all->forwarding && sllao)
- addr_flags |= IFA_F_OPTIMISTIC;
+ cfg.ifa_flags |= IFA_F_OPTIMISTIC;
#endif
/* Do not allow to create too much of autoconfigured
@@ -2520,11 +2524,7 @@ int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
*/
if (!max_addresses ||
ipv6_count_addresses(in6_dev) < max_addresses)
- ifp = ipv6_add_addr(in6_dev, addr, NULL,
- pinfo->prefix_len,
- addr_type&IPV6_ADDR_SCOPE_MASK,
- addr_flags, valid_lft,
- prefered_lft, false, NULL);
+ ifp = ipv6_add_addr(in6_dev, &cfg, false, NULL);
if (IS_ERR_OR_NULL(ifp))
return -1;
@@ -2836,12 +2836,19 @@ static int inet6_addr_add(struct net *net, int ifindex,
__u32 prefered_lft, __u32 valid_lft,
struct netlink_ext_ack *extack)
{
+ struct ifa6_config cfg = {
+ .pfx = pfx,
+ .plen = plen,
+ .peer_pfx = peer_pfx,
+ .ifa_flags = ifa_flags,
+ .preferred_lft = prefered_lft,
+ .valid_lft = valid_lft,
+ };
struct inet6_ifaddr *ifp;
struct inet6_dev *idev;
struct net_device *dev;
unsigned long timeout;
clock_t expires;
- int scope;
u32 flags;
ASSERT_RTNL();
@@ -2872,7 +2879,7 @@ static int inet6_addr_add(struct net *net, int ifindex,
return ret;
}
- scope = ipv6_addr_scope(pfx);
+ cfg.scope = ipv6_addr_scope(pfx);
timeout = addrconf_timeout_fixup(valid_lft, HZ);
if (addrconf_finite_timeout(timeout)) {
@@ -2892,9 +2899,7 @@ static int inet6_addr_add(struct net *net, int ifindex,
prefered_lft = timeout;
}
- ifp = ipv6_add_addr(idev, pfx, peer_pfx, plen, scope, ifa_flags,
- valid_lft, prefered_lft, true, extack);
-
+ ifp = ipv6_add_addr(idev, &cfg, true, extack);
if (!IS_ERR(ifp)) {
if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
addrconf_prefix_route(&ifp->addr, ifp->prefix_len, dev,
@@ -3010,11 +3015,16 @@ static void add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
int plen, int scope)
{
struct inet6_ifaddr *ifp;
+ struct ifa6_config cfg = {
+ .pfx = addr,
+ .plen = plen,
+ .ifa_flags = IFA_F_PERMANENT,
+ .valid_lft = INFINITY_LIFE_TIME,
+ .preferred_lft = INFINITY_LIFE_TIME,
+ .scope = scope
+ };
- ifp = ipv6_add_addr(idev, addr, NULL, plen,
- scope, IFA_F_PERMANENT,
- INFINITY_LIFE_TIME, INFINITY_LIFE_TIME,
- true, NULL);
+ ifp = ipv6_add_addr(idev, &cfg, true, NULL);
if (!IS_ERR(ifp)) {
spin_lock_bh(&ifp->lock);
ifp->flags &= ~IFA_F_TENTATIVE;
@@ -3104,18 +3114,24 @@ static void init_loopback(struct net_device *dev)
void addrconf_add_linklocal(struct inet6_dev *idev,
const struct in6_addr *addr, u32 flags)
{
+ struct ifa6_config cfg = {
+ .pfx = addr,
+ .plen = 64,
+ .ifa_flags = flags | IFA_F_PERMANENT,
+ .valid_lft = INFINITY_LIFE_TIME,
+ .preferred_lft = INFINITY_LIFE_TIME,
+ .scope = IFA_LINK
+ };
struct inet6_ifaddr *ifp;
- u32 addr_flags = flags | IFA_F_PERMANENT;
#ifdef CONFIG_IPV6_OPTIMISTIC_DAD
if ((dev_net(idev->dev)->ipv6.devconf_all->optimistic_dad ||
idev->cnf.optimistic_dad) &&
!dev_net(idev->dev)->ipv6.devconf_all->forwarding)
- addr_flags |= IFA_F_OPTIMISTIC;
+ cfg.ifa_flags |= IFA_F_OPTIMISTIC;
#endif
- ifp = ipv6_add_addr(idev, addr, NULL, 64, IFA_LINK, addr_flags,
- INFINITY_LIFE_TIME, INFINITY_LIFE_TIME, true, NULL);
+ ifp = ipv6_add_addr(idev, &cfg, true, NULL);
if (!IS_ERR(ifp)) {
addrconf_prefix_route(&ifp->addr, ifp->prefix_len, idev->dev,
0, 0, GFP_ATOMIC);
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 2/7] net/ipv6: Pass ifa6_config struct to inet6_addr_add
From: dsahern @ 2018-05-27 15:09 UTC (permalink / raw)
To: netdev; +Cc: roopa, David Ahern
In-Reply-To: <20180527151000.30488-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
Move the creation of struct ifa6_config up to callers of inet6_addr_add.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/ipv6/addrconf.c | 112 ++++++++++++++++++++++++++--------------------------
1 file changed, 57 insertions(+), 55 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 4988f2265882..2db1acf70610 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2830,20 +2830,9 @@ static int ipv6_mc_config(struct sock *sk, bool join,
* Manual configuration of address on an interface
*/
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,
+ struct ifa6_config *cfg,
struct netlink_ext_ack *extack)
{
- struct ifa6_config cfg = {
- .pfx = pfx,
- .plen = plen,
- .peer_pfx = peer_pfx,
- .ifa_flags = ifa_flags,
- .preferred_lft = prefered_lft,
- .valid_lft = valid_lft,
- };
struct inet6_ifaddr *ifp;
struct inet6_dev *idev;
struct net_device *dev;
@@ -2853,14 +2842,14 @@ static int inet6_addr_add(struct net *net, int ifindex,
ASSERT_RTNL();
- if (plen > 128)
+ if (cfg->plen > 128)
return -EINVAL;
/* check the lifetime */
- if (!valid_lft || prefered_lft > valid_lft)
+ if (!cfg->valid_lft || cfg->preferred_lft > cfg->valid_lft)
return -EINVAL;
- if (ifa_flags & IFA_F_MANAGETEMPADDR && plen != 64)
+ if (cfg->ifa_flags & IFA_F_MANAGETEMPADDR && cfg->plen != 64)
return -EINVAL;
dev = __dev_get_by_index(net, ifindex);
@@ -2871,37 +2860,37 @@ static int inet6_addr_add(struct net *net, int ifindex,
if (IS_ERR(idev))
return PTR_ERR(idev);
- if (ifa_flags & IFA_F_MCAUTOJOIN) {
+ if (cfg->ifa_flags & IFA_F_MCAUTOJOIN) {
int ret = ipv6_mc_config(net->ipv6.mc_autojoin_sk,
- true, pfx, ifindex);
+ true, cfg->pfx, ifindex);
if (ret < 0)
return ret;
}
- cfg.scope = ipv6_addr_scope(pfx);
+ cfg->scope = ipv6_addr_scope(cfg->pfx);
- timeout = addrconf_timeout_fixup(valid_lft, HZ);
+ timeout = addrconf_timeout_fixup(cfg->valid_lft, HZ);
if (addrconf_finite_timeout(timeout)) {
expires = jiffies_to_clock_t(timeout * HZ);
- valid_lft = timeout;
+ cfg->valid_lft = timeout;
flags = RTF_EXPIRES;
} else {
expires = 0;
flags = 0;
- ifa_flags |= IFA_F_PERMANENT;
+ cfg->ifa_flags |= IFA_F_PERMANENT;
}
- timeout = addrconf_timeout_fixup(prefered_lft, HZ);
+ timeout = addrconf_timeout_fixup(cfg->preferred_lft, HZ);
if (addrconf_finite_timeout(timeout)) {
if (timeout == 0)
- ifa_flags |= IFA_F_DEPRECATED;
- prefered_lft = timeout;
+ cfg->ifa_flags |= IFA_F_DEPRECATED;
+ cfg->preferred_lft = timeout;
}
- ifp = ipv6_add_addr(idev, &cfg, true, extack);
+ ifp = ipv6_add_addr(idev, cfg, true, extack);
if (!IS_ERR(ifp)) {
- if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
+ if (!(cfg->ifa_flags & IFA_F_NOPREFIXROUTE)) {
addrconf_prefix_route(&ifp->addr, ifp->prefix_len, dev,
expires, flags, GFP_KERNEL);
}
@@ -2917,15 +2906,15 @@ static int inet6_addr_add(struct net *net, int ifindex,
* manually configured addresses
*/
addrconf_dad_start(ifp);
- if (ifa_flags & IFA_F_MANAGETEMPADDR)
- manage_tempaddrs(idev, ifp, valid_lft, prefered_lft,
- true, jiffies);
+ if (cfg->ifa_flags & IFA_F_MANAGETEMPADDR)
+ manage_tempaddrs(idev, ifp, cfg->valid_lft,
+ cfg->preferred_lft, true, jiffies);
in6_ifa_put(ifp);
addrconf_verify_rtnl();
return 0;
- } else if (ifa_flags & IFA_F_MCAUTOJOIN) {
- ipv6_mc_config(net->ipv6.mc_autojoin_sk,
- false, pfx, ifindex);
+ } else if (cfg->ifa_flags & IFA_F_MCAUTOJOIN) {
+ ipv6_mc_config(net->ipv6.mc_autojoin_sk, false,
+ cfg->pfx, ifindex);
}
return PTR_ERR(ifp);
@@ -2976,6 +2965,11 @@ static int inet6_addr_del(struct net *net, int ifindex, u32 ifa_flags,
int addrconf_add_ifaddr(struct net *net, void __user *arg)
{
+ struct ifa6_config cfg = {
+ .ifa_flags = IFA_F_PERMANENT,
+ .preferred_lft = INFINITY_LIFE_TIME,
+ .valid_lft = INFINITY_LIFE_TIME,
+ };
struct in6_ifreq ireq;
int err;
@@ -2985,10 +2979,11 @@ int addrconf_add_ifaddr(struct net *net, void __user *arg)
if (copy_from_user(&ireq, arg, sizeof(struct in6_ifreq)))
return -EFAULT;
+ cfg.pfx = &ireq.ifr6_addr;
+ cfg.plen = ireq.ifr6_prefixlen;
+
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, NULL);
+ err = inet6_addr_add(net, ireq.ifr6_ifindex, &cfg, NULL);
rtnl_unlock();
return err;
}
@@ -4624,12 +4619,11 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
struct net *net = sock_net(skb->sk);
struct ifaddrmsg *ifm;
struct nlattr *tb[IFA_MAX+1];
- struct in6_addr *pfx, *peer_pfx;
+ struct in6_addr *peer_pfx;
struct inet6_ifaddr *ifa;
struct net_device *dev;
struct inet6_dev *idev;
- u32 valid_lft = INFINITY_LIFE_TIME, preferred_lft = INFINITY_LIFE_TIME;
- u32 ifa_flags;
+ struct ifa6_config cfg;
int err;
err = nlmsg_parse(nlh, sizeof(*ifm), tb, IFA_MAX, ifa_ipv6_policy,
@@ -4637,60 +4631,68 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
if (err < 0)
return err;
+ memset(&cfg, 0, sizeof(cfg));
+
ifm = nlmsg_data(nlh);
- pfx = extract_addr(tb[IFA_ADDRESS], tb[IFA_LOCAL], &peer_pfx);
- if (!pfx)
+ cfg.pfx = extract_addr(tb[IFA_ADDRESS], tb[IFA_LOCAL], &peer_pfx);
+ if (!cfg.pfx)
return -EINVAL;
+ cfg.peer_pfx = peer_pfx;
+ cfg.plen = ifm->ifa_prefixlen;
+ cfg.valid_lft = INFINITY_LIFE_TIME;
+ cfg.preferred_lft = INFINITY_LIFE_TIME;
+
if (tb[IFA_CACHEINFO]) {
struct ifa_cacheinfo *ci;
ci = nla_data(tb[IFA_CACHEINFO]);
- valid_lft = ci->ifa_valid;
- preferred_lft = ci->ifa_prefered;
- } else {
- preferred_lft = INFINITY_LIFE_TIME;
- valid_lft = INFINITY_LIFE_TIME;
+ cfg.valid_lft = ci->ifa_valid;
+ cfg.preferred_lft = ci->ifa_prefered;
}
dev = __dev_get_by_index(net, ifm->ifa_index);
if (!dev)
return -ENODEV;
- ifa_flags = tb[IFA_FLAGS] ? nla_get_u32(tb[IFA_FLAGS]) : ifm->ifa_flags;
+ if (tb[IFA_FLAGS])
+ cfg.ifa_flags = nla_get_u32(tb[IFA_FLAGS]);
+ else
+ cfg.ifa_flags = ifm->ifa_flags;
/* We ignore other flags so far. */
- ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR |
- IFA_F_NOPREFIXROUTE | IFA_F_MCAUTOJOIN | IFA_F_OPTIMISTIC;
+ cfg.ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS |
+ IFA_F_MANAGETEMPADDR | IFA_F_NOPREFIXROUTE |
+ IFA_F_MCAUTOJOIN | IFA_F_OPTIMISTIC;
idev = ipv6_find_idev(dev);
if (IS_ERR(idev))
return PTR_ERR(idev);
if (!ipv6_allow_optimistic_dad(net, idev))
- ifa_flags &= ~IFA_F_OPTIMISTIC;
+ cfg.ifa_flags &= ~IFA_F_OPTIMISTIC;
- if (ifa_flags & IFA_F_NODAD && ifa_flags & IFA_F_OPTIMISTIC) {
+ if (cfg.ifa_flags & IFA_F_NODAD &&
+ cfg.ifa_flags & IFA_F_OPTIMISTIC) {
NL_SET_ERR_MSG(extack, "IFA_F_NODAD and IFA_F_OPTIMISTIC are mutually exclusive");
return -EINVAL;
}
- ifa = ipv6_get_ifaddr(net, pfx, dev, 1);
+ ifa = ipv6_get_ifaddr(net, cfg.pfx, dev, 1);
if (!ifa) {
/*
* It would be best to check for !NLM_F_CREATE here but
* userspace already relies on not having to provide this.
*/
- return inet6_addr_add(net, ifm->ifa_index, pfx, peer_pfx,
- ifm->ifa_prefixlen, ifa_flags,
- preferred_lft, valid_lft, extack);
+ return inet6_addr_add(net, ifm->ifa_index, &cfg, extack);
}
if (nlh->nlmsg_flags & NLM_F_EXCL ||
!(nlh->nlmsg_flags & NLM_F_REPLACE))
err = -EEXIST;
else
- err = inet6_addr_modify(ifa, ifa_flags, preferred_lft, valid_lft);
+ err = inet6_addr_modify(ifa, cfg.ifa_flags, cfg.preferred_lft,
+ cfg.valid_lft);
in6_ifa_put(ifa);
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 3/7] net/ipv6: Pass ifa6_config struct to inet6_addr_modify
From: dsahern @ 2018-05-27 15:09 UTC (permalink / raw)
To: netdev; +Cc: roopa, David Ahern
In-Reply-To: <20180527151000.30488-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
Update inet6_addr_modify to take ifa6_config argument versus a parameter
list. This is an argument move only; no functional change intended.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/ipv6/addrconf.c | 44 +++++++++++++++++++++++---------------------
1 file changed, 23 insertions(+), 21 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 2db1acf70610..1b1e63a4520b 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -4527,8 +4527,7 @@ inet6_rtm_deladdr(struct sk_buff *skb, struct nlmsghdr *nlh,
ifm->ifa_prefixlen);
}
-static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
- u32 prefered_lft, u32 valid_lft)
+static int inet6_addr_modify(struct inet6_ifaddr *ifp, struct ifa6_config *cfg)
{
u32 flags;
clock_t expires;
@@ -4538,32 +4537,32 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
ASSERT_RTNL();
- if (!valid_lft || (prefered_lft > valid_lft))
+ if (!cfg->valid_lft || cfg->preferred_lft > cfg->valid_lft)
return -EINVAL;
- if (ifa_flags & IFA_F_MANAGETEMPADDR &&
+ if (cfg->ifa_flags & IFA_F_MANAGETEMPADDR &&
(ifp->flags & IFA_F_TEMPORARY || ifp->prefix_len != 64))
return -EINVAL;
if (!(ifp->flags & IFA_F_TENTATIVE) || ifp->flags & IFA_F_DADFAILED)
- ifa_flags &= ~IFA_F_OPTIMISTIC;
+ cfg->ifa_flags &= ~IFA_F_OPTIMISTIC;
- timeout = addrconf_timeout_fixup(valid_lft, HZ);
+ timeout = addrconf_timeout_fixup(cfg->valid_lft, HZ);
if (addrconf_finite_timeout(timeout)) {
expires = jiffies_to_clock_t(timeout * HZ);
- valid_lft = timeout;
+ cfg->valid_lft = timeout;
flags = RTF_EXPIRES;
} else {
expires = 0;
flags = 0;
- ifa_flags |= IFA_F_PERMANENT;
+ cfg->ifa_flags |= IFA_F_PERMANENT;
}
- timeout = addrconf_timeout_fixup(prefered_lft, HZ);
+ timeout = addrconf_timeout_fixup(cfg->preferred_lft, HZ);
if (addrconf_finite_timeout(timeout)) {
if (timeout == 0)
- ifa_flags |= IFA_F_DEPRECATED;
- prefered_lft = timeout;
+ cfg->ifa_flags |= IFA_F_DEPRECATED;
+ cfg->preferred_lft = timeout;
}
spin_lock_bh(&ifp->lock);
@@ -4573,16 +4572,16 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
ifp->flags &= ~(IFA_F_DEPRECATED | IFA_F_PERMANENT | IFA_F_NODAD |
IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR |
IFA_F_NOPREFIXROUTE);
- ifp->flags |= ifa_flags;
+ ifp->flags |= cfg->ifa_flags;
ifp->tstamp = jiffies;
- ifp->valid_lft = valid_lft;
- ifp->prefered_lft = prefered_lft;
+ ifp->valid_lft = cfg->valid_lft;
+ ifp->prefered_lft = cfg->preferred_lft;
spin_unlock_bh(&ifp->lock);
if (!(ifp->flags&IFA_F_TENTATIVE))
ipv6_ifa_notify(0, ifp);
- if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
+ if (!(cfg->ifa_flags & IFA_F_NOPREFIXROUTE)) {
addrconf_prefix_route(&ifp->addr, ifp->prefix_len,
ifp->idev->dev, expires, flags,
GFP_KERNEL);
@@ -4601,10 +4600,14 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, u32 ifa_flags,
}
if (was_managetempaddr || ifp->flags & IFA_F_MANAGETEMPADDR) {
- if (was_managetempaddr && !(ifp->flags & IFA_F_MANAGETEMPADDR))
- valid_lft = prefered_lft = 0;
- manage_tempaddrs(ifp->idev, ifp, valid_lft, prefered_lft,
- !was_managetempaddr, jiffies);
+ if (was_managetempaddr &&
+ !(ifp->flags & IFA_F_MANAGETEMPADDR)) {
+ cfg->valid_lft = 0;
+ cfg->preferred_lft = 0;
+ }
+ manage_tempaddrs(ifp->idev, ifp, cfg->valid_lft,
+ cfg->preferred_lft, !was_managetempaddr,
+ jiffies);
}
addrconf_verify_rtnl();
@@ -4691,8 +4694,7 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
!(nlh->nlmsg_flags & NLM_F_REPLACE))
err = -EEXIST;
else
- err = inet6_addr_modify(ifa, cfg.ifa_flags, cfg.preferred_lft,
- cfg.valid_lft);
+ err = inet6_addr_modify(ifa, &cfg);
in6_ifa_put(ifa);
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 4/7] net: Add IFA_RT_PRIORITY address attribute
From: dsahern @ 2018-05-27 15:09 UTC (permalink / raw)
To: netdev; +Cc: roopa, David Ahern
In-Reply-To: <20180527151000.30488-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
Currently, if two interfaces have addresses in the same connected route,
then the order of the prefix route entries is determined by the order in
which the addresses are assigned or the links brought up.
Add IFA_RT_PRIORITY to allow user to specify the metric of the prefix
route associated with an address giving interface managers better
control of the order of prefix routes.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
include/uapi/linux/if_addr.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h
index 2ef053d265de..ebaf5701c9db 100644
--- a/include/uapi/linux/if_addr.h
+++ b/include/uapi/linux/if_addr.h
@@ -33,6 +33,7 @@ enum {
IFA_CACHEINFO,
IFA_MULTICAST,
IFA_FLAGS,
+ IFA_RT_PRIORITY, /* u32, priority/metric for prefix route */
__IFA_MAX,
};
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 6/7] net/ipv6: Add support for specifying metric of connected routes
From: dsahern @ 2018-05-27 15:09 UTC (permalink / raw)
To: netdev; +Cc: roopa, David Ahern
In-Reply-To: <20180527151000.30488-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
Add support for IFA_RT_PRIORITY to ipv6 addresses.
If the metric is changed on an existing address then the new route
is inserted before removing the old one. Since the metric is one
of the route keys, the prefix route can not be atomically replaced.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
include/net/addrconf.h | 1 +
include/net/if_inet6.h | 1 +
net/ipv6/addrconf.c | 93 ++++++++++++++++++++++++++++++++++++++++----------
3 files changed, 77 insertions(+), 18 deletions(-)
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index f766af2cd1a4..5f43f7a70fe6 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -65,6 +65,7 @@ struct ifa6_config {
const struct in6_addr *peer_pfx;
+ u32 rt_priority;
u32 ifa_flags;
u32 preferred_lft;
u32 valid_lft;
diff --git a/include/net/if_inet6.h b/include/net/if_inet6.h
index db389253dc2a..d7578cf49c3a 100644
--- a/include/net/if_inet6.h
+++ b/include/net/if_inet6.h
@@ -42,6 +42,7 @@ enum {
struct inet6_ifaddr {
struct in6_addr addr;
__u32 prefix_len;
+ __u32 rt_priority;
/* In seconds, relative to tstamp. Expiry is at tstamp + HZ * lft. */
__u32 valid_lft;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 1b1e63a4520b..f09afc232da4 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -1056,6 +1056,7 @@ ipv6_add_addr(struct inet6_dev *idev, struct ifa6_config *cfg,
INIT_HLIST_NODE(&ifa->addr_lst);
ifa->scope = cfg->scope;
ifa->prefix_len = cfg->plen;
+ ifa->rt_priority = cfg->rt_priority;
ifa->flags = cfg->ifa_flags;
/* No need to add the TENTATIVE flag for addresses with NODAD */
if (!(cfg->ifa_flags & IFA_F_NODAD))
@@ -1356,6 +1357,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp,
cfg.pfx = &addr;
cfg.scope = ipv6_addr_scope(cfg.pfx);
+ cfg.rt_priority = 0;
ift = ipv6_add_addr(idev, &cfg, block, NULL);
if (IS_ERR(ift)) {
@@ -2314,12 +2316,13 @@ static void ipv6_try_regen_rndid(struct inet6_dev *idev, struct in6_addr *tmpad
*/
static void
-addrconf_prefix_route(struct in6_addr *pfx, int plen, struct net_device *dev,
- unsigned long expires, u32 flags, gfp_t gfp_flags)
+addrconf_prefix_route(struct in6_addr *pfx, int plen, u32 metric,
+ struct net_device *dev, unsigned long expires,
+ u32 flags, gfp_t gfp_flags)
{
struct fib6_config cfg = {
.fc_table = l3mdev_fib_table(dev) ? : RT6_TABLE_PREFIX,
- .fc_metric = IP6_RT_PRIO_ADDRCONF,
+ .fc_metric = metric ? : IP6_RT_PRIO_ADDRCONF,
.fc_ifindex = dev->ifindex,
.fc_expires = expires,
.fc_dst_len = plen,
@@ -2683,7 +2686,8 @@ void addrconf_prefix_rcv(struct net_device *dev, u8 *opt, int len, bool sllao)
expires = jiffies_to_clock_t(rt_expires);
}
addrconf_prefix_route(&pinfo->prefix, pinfo->prefix_len,
- dev, expires, flags, GFP_ATOMIC);
+ 0, dev, expires, flags,
+ GFP_ATOMIC);
}
fib6_info_release(rt);
}
@@ -2891,8 +2895,9 @@ static int inet6_addr_add(struct net *net, int ifindex,
ifp = ipv6_add_addr(idev, cfg, true, extack);
if (!IS_ERR(ifp)) {
if (!(cfg->ifa_flags & IFA_F_NOPREFIXROUTE)) {
- addrconf_prefix_route(&ifp->addr, ifp->prefix_len, dev,
- expires, flags, GFP_KERNEL);
+ addrconf_prefix_route(&ifp->addr, ifp->prefix_len,
+ ifp->rt_priority, dev, expires,
+ flags, GFP_KERNEL);
}
/* Send a netlink notification if DAD is enabled and
@@ -3056,7 +3061,7 @@ static void sit_add_v4_addrs(struct inet6_dev *idev)
if (addr.s6_addr32[3]) {
add_addr(idev, &addr, plen, scope);
- addrconf_prefix_route(&addr, plen, idev->dev, 0, pflags,
+ addrconf_prefix_route(&addr, plen, 0, idev->dev, 0, pflags,
GFP_ATOMIC);
return;
}
@@ -3081,8 +3086,8 @@ static void sit_add_v4_addrs(struct inet6_dev *idev)
}
add_addr(idev, &addr, plen, flag);
- addrconf_prefix_route(&addr, plen, idev->dev, 0,
- pflags, GFP_ATOMIC);
+ addrconf_prefix_route(&addr, plen, 0, idev->dev,
+ 0, pflags, GFP_ATOMIC);
}
}
}
@@ -3128,7 +3133,7 @@ void addrconf_add_linklocal(struct inet6_dev *idev,
ifp = ipv6_add_addr(idev, &cfg, true, NULL);
if (!IS_ERR(ifp)) {
- addrconf_prefix_route(&ifp->addr, ifp->prefix_len, idev->dev,
+ addrconf_prefix_route(&ifp->addr, ifp->prefix_len, 0, idev->dev,
0, 0, GFP_ATOMIC);
addrconf_dad_start(ifp);
in6_ifa_put(ifp);
@@ -3244,7 +3249,7 @@ static void addrconf_addr_gen(struct inet6_dev *idev, bool prefix_route)
addrconf_add_linklocal(idev, &addr,
IFA_F_STABLE_PRIVACY);
else if (prefix_route)
- addrconf_prefix_route(&addr, 64, idev->dev,
+ addrconf_prefix_route(&addr, 64, 0, idev->dev,
0, 0, GFP_KERNEL);
break;
case IN6_ADDR_GEN_MODE_EUI64:
@@ -3255,7 +3260,7 @@ static void addrconf_addr_gen(struct inet6_dev *idev, bool prefix_route)
if (ipv6_generate_eui64(addr.s6_addr + 8, idev->dev) == 0)
addrconf_add_linklocal(idev, &addr, 0);
else if (prefix_route)
- addrconf_prefix_route(&addr, 64, idev->dev,
+ addrconf_prefix_route(&addr, 64, 0, idev->dev,
0, 0, GFP_KERNEL);
break;
case IN6_ADDR_GEN_MODE_NONE:
@@ -3375,7 +3380,8 @@ static int fixup_permanent_addr(struct net *net,
if (!(ifp->flags & IFA_F_NOPREFIXROUTE)) {
addrconf_prefix_route(&ifp->addr, ifp->prefix_len,
- idev->dev, 0, 0, GFP_ATOMIC);
+ ifp->rt_priority, idev->dev, 0, 0,
+ GFP_ATOMIC);
}
if (ifp->state == INET6_IFADDR_STATE_PREDAD)
@@ -4495,6 +4501,7 @@ static const struct nla_policy ifa_ipv6_policy[IFA_MAX+1] = {
[IFA_LOCAL] = { .len = sizeof(struct in6_addr) },
[IFA_CACHEINFO] = { .len = sizeof(struct ifa_cacheinfo) },
[IFA_FLAGS] = { .len = sizeof(u32) },
+ [IFA_RT_PRIORITY] = { .len = sizeof(u32) },
};
static int
@@ -4527,6 +4534,37 @@ inet6_rtm_deladdr(struct sk_buff *skb, struct nlmsghdr *nlh,
ifm->ifa_prefixlen);
}
+static int modify_prefix_route(struct inet6_ifaddr *ifp,
+ unsigned long expires, u32 flags)
+{
+ struct fib6_info *f6i;
+
+ f6i = addrconf_get_prefix_route(&ifp->addr,
+ ifp->prefix_len,
+ ifp->idev->dev,
+ 0, RTF_GATEWAY | RTF_DEFAULT);
+ if (!f6i)
+ return -ENOENT;
+
+ if (f6i->fib6_metric != ifp->rt_priority) {
+ /* add new one */
+ addrconf_prefix_route(&ifp->addr, ifp->prefix_len,
+ ifp->rt_priority, ifp->idev->dev,
+ expires, flags, GFP_KERNEL);
+ /* delete old one */
+ ip6_del_rt(dev_net(ifp->idev->dev), f6i);
+ } else {
+ if (!expires)
+ fib6_clean_expires(f6i);
+ else
+ fib6_set_expires(f6i, expires);
+
+ fib6_info_release(f6i);
+ }
+
+ return 0;
+}
+
static int inet6_addr_modify(struct inet6_ifaddr *ifp, struct ifa6_config *cfg)
{
u32 flags;
@@ -4577,14 +4615,25 @@ static int inet6_addr_modify(struct inet6_ifaddr *ifp, struct ifa6_config *cfg)
ifp->valid_lft = cfg->valid_lft;
ifp->prefered_lft = cfg->preferred_lft;
+ if (cfg->rt_priority && cfg->rt_priority != ifp->rt_priority)
+ ifp->rt_priority = cfg->rt_priority;
+
spin_unlock_bh(&ifp->lock);
if (!(ifp->flags&IFA_F_TENTATIVE))
ipv6_ifa_notify(0, ifp);
if (!(cfg->ifa_flags & IFA_F_NOPREFIXROUTE)) {
- addrconf_prefix_route(&ifp->addr, ifp->prefix_len,
- ifp->idev->dev, expires, flags,
- GFP_KERNEL);
+ int rc = -ENOENT;
+
+ if (had_prefixroute)
+ rc = modify_prefix_route(ifp, expires, flags);
+
+ /* prefix route could have been deleted; if so restore it */
+ if (rc == -ENOENT) {
+ addrconf_prefix_route(&ifp->addr, ifp->prefix_len,
+ ifp->rt_priority, ifp->idev->dev,
+ expires, flags, GFP_KERNEL);
+ }
} else if (had_prefixroute) {
enum cleanup_prefix_rt_t action;
unsigned long rt_expires;
@@ -4643,6 +4692,9 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
cfg.peer_pfx = peer_pfx;
cfg.plen = ifm->ifa_prefixlen;
+ if (tb[IFA_RT_PRIORITY])
+ cfg.rt_priority = nla_get_u32(tb[IFA_RT_PRIORITY]);
+
cfg.valid_lft = INFINITY_LIFE_TIME;
cfg.preferred_lft = INFINITY_LIFE_TIME;
@@ -4745,7 +4797,8 @@ static inline int inet6_ifaddr_msgsize(void)
+ nla_total_size(16) /* IFA_LOCAL */
+ nla_total_size(16) /* IFA_ADDRESS */
+ nla_total_size(sizeof(struct ifa_cacheinfo))
- + nla_total_size(4) /* IFA_FLAGS */;
+ + nla_total_size(4) /* IFA_FLAGS */
+ + nla_total_size(4) /* IFA_RT_PRIORITY */;
}
static int inet6_fill_ifaddr(struct sk_buff *skb, struct inet6_ifaddr *ifa,
@@ -4791,6 +4844,10 @@ static int inet6_fill_ifaddr(struct sk_buff *skb, struct inet6_ifaddr *ifa,
if (nla_put_in6_addr(skb, IFA_ADDRESS, &ifa->addr) < 0)
goto error;
+ if (ifa->rt_priority &&
+ nla_put_u32(skb, IFA_RT_PRIORITY, ifa->rt_priority))
+ goto error;
+
if (put_cacheinfo(skb, ifa->cstamp, ifa->tstamp, preferred, valid) < 0)
goto error;
@@ -5635,7 +5692,7 @@ static void __ipv6_ifa_notify(int event, struct inet6_ifaddr *ifp)
if (ifp->idev->cnf.forwarding)
addrconf_join_anycast(ifp);
if (!ipv6_addr_any(&ifp->peer_addr))
- addrconf_prefix_route(&ifp->peer_addr, 128,
+ addrconf_prefix_route(&ifp->peer_addr, 128, 0,
ifp->idev->dev, 0, 0,
GFP_ATOMIC);
break;
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 5/7] net/ipv4: Add support for specifying metric of connected routes
From: dsahern @ 2018-05-27 15:09 UTC (permalink / raw)
To: netdev; +Cc: roopa, David Ahern
In-Reply-To: <20180527151000.30488-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
Add support for IFA_RT_PRIORITY to ipv4 addresses.
If the metric is changed on an existing address then the new route
is inserted before removing the old one. Since the metric is one
of the route keys, the prefix route can not be replaced.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
include/linux/inetdevice.h | 1 +
include/net/route.h | 1 +
net/ipv4/devinet.c | 15 +++++++++++++
net/ipv4/fib_frontend.c | 53 ++++++++++++++++++++++++++++++++++++----------
4 files changed, 59 insertions(+), 11 deletions(-)
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index e16fe7d44a71..27650f1bff3d 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -139,6 +139,7 @@ struct in_ifaddr {
__be32 ifa_local;
__be32 ifa_address;
__be32 ifa_mask;
+ __u32 ifa_rt_priority;
__be32 ifa_broadcast;
unsigned char ifa_scope;
unsigned char ifa_prefixlen;
diff --git a/include/net/route.h b/include/net/route.h
index dbb032d5921b..bb53cdba38dc 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -225,6 +225,7 @@ struct rtable *rt_dst_alloc(struct net_device *dev,
struct in_ifaddr;
void fib_add_ifaddr(struct in_ifaddr *);
void fib_del_ifaddr(struct in_ifaddr *, struct in_ifaddr *);
+void fib_modify_prefix_metric(struct in_ifaddr *ifa, u32 new_metric);
void rt_add_uncached_list(struct rtable *rt);
void rt_del_uncached_list(struct rtable *rt);
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 40f001782c1b..d7585ab1a77a 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -99,6 +99,7 @@ static const struct nla_policy ifa_ipv4_policy[IFA_MAX+1] = {
[IFA_LABEL] = { .type = NLA_STRING, .len = IFNAMSIZ - 1 },
[IFA_CACHEINFO] = { .len = sizeof(struct ifa_cacheinfo) },
[IFA_FLAGS] = { .type = NLA_U32 },
+ [IFA_RT_PRIORITY] = { .type = NLA_U32 },
};
#define IN4_ADDR_HSIZE_SHIFT 8
@@ -835,6 +836,9 @@ static struct in_ifaddr *rtm_to_ifaddr(struct net *net, struct nlmsghdr *nlh,
else
memcpy(ifa->ifa_label, dev->name, IFNAMSIZ);
+ if (tb[IFA_RT_PRIORITY])
+ ifa->ifa_rt_priority = nla_get_u32(tb[IFA_RT_PRIORITY]);
+
if (tb[IFA_CACHEINFO]) {
struct ifa_cacheinfo *ci;
@@ -906,12 +910,20 @@ static int inet_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
return __inet_insert_ifa(ifa, nlh, NETLINK_CB(skb).portid,
extack);
} else {
+ u32 new_metric = ifa->ifa_rt_priority;
+
inet_free_ifa(ifa);
if (nlh->nlmsg_flags & NLM_F_EXCL ||
!(nlh->nlmsg_flags & NLM_F_REPLACE))
return -EEXIST;
ifa = ifa_existing;
+
+ if (ifa->ifa_rt_priority != new_metric) {
+ fib_modify_prefix_metric(ifa, new_metric);
+ ifa->ifa_rt_priority = new_metric;
+ }
+
set_ifa_lifetime(ifa, valid_lft, prefered_lft);
cancel_delayed_work(&check_lifetime_work);
queue_delayed_work(system_power_efficient_wq,
@@ -1549,6 +1561,7 @@ static size_t inet_nlmsg_size(void)
+ nla_total_size(4) /* IFA_BROADCAST */
+ nla_total_size(IFNAMSIZ) /* IFA_LABEL */
+ nla_total_size(4) /* IFA_FLAGS */
+ + nla_total_size(4) /* IFA_RT_PRIORITY */
+ nla_total_size(sizeof(struct ifa_cacheinfo)); /* IFA_CACHEINFO */
}
@@ -1618,6 +1631,8 @@ static int inet_fill_ifaddr(struct sk_buff *skb, struct in_ifaddr *ifa,
(ifa->ifa_label[0] &&
nla_put_string(skb, IFA_LABEL, ifa->ifa_label)) ||
nla_put_u32(skb, IFA_FLAGS, ifa->ifa_flags) ||
+ (ifa->ifa_rt_priority &&
+ nla_put_u32(skb, IFA_RT_PRIORITY, ifa->ifa_rt_priority)) ||
put_cacheinfo(skb, ifa->ifa_cstamp, ifa->ifa_tstamp,
preferred, valid))
goto nla_put_failure;
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 045c43a27c12..07989d9ab2f2 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -846,7 +846,8 @@ static int inet_dump_fib(struct sk_buff *skb, struct netlink_callback *cb)
* to fib engine. It is legal, because all events occur
* only when netlink is already locked.
*/
-static void fib_magic(int cmd, int type, __be32 dst, int dst_len, struct in_ifaddr *ifa)
+static void fib_magic(int cmd, int type, __be32 dst, int dst_len,
+ struct in_ifaddr *ifa, u32 rt_priority)
{
struct net *net = dev_net(ifa->ifa_dev->dev);
u32 tb_id = l3mdev_fib_table(ifa->ifa_dev->dev);
@@ -856,6 +857,7 @@ static void fib_magic(int cmd, int type, __be32 dst, int dst_len, struct in_ifad
.fc_type = type,
.fc_dst = dst,
.fc_dst_len = dst_len,
+ .fc_priority = rt_priority,
.fc_prefsrc = ifa->ifa_local,
.fc_oif = ifa->ifa_dev->dev->ifindex,
.fc_nlflags = NLM_F_CREATE | NLM_F_APPEND,
@@ -901,31 +903,57 @@ void fib_add_ifaddr(struct in_ifaddr *ifa)
}
}
- fib_magic(RTM_NEWROUTE, RTN_LOCAL, addr, 32, prim);
+ fib_magic(RTM_NEWROUTE, RTN_LOCAL, addr, 32, prim, 0);
if (!(dev->flags & IFF_UP))
return;
/* Add broadcast address, if it is explicitly assigned. */
if (ifa->ifa_broadcast && ifa->ifa_broadcast != htonl(0xFFFFFFFF))
- fib_magic(RTM_NEWROUTE, RTN_BROADCAST, ifa->ifa_broadcast, 32, prim);
+ fib_magic(RTM_NEWROUTE, RTN_BROADCAST, ifa->ifa_broadcast, 32,
+ prim, 0);
if (!ipv4_is_zeronet(prefix) && !(ifa->ifa_flags & IFA_F_SECONDARY) &&
(prefix != addr || ifa->ifa_prefixlen < 32)) {
if (!(ifa->ifa_flags & IFA_F_NOPREFIXROUTE))
fib_magic(RTM_NEWROUTE,
dev->flags & IFF_LOOPBACK ? RTN_LOCAL : RTN_UNICAST,
- prefix, ifa->ifa_prefixlen, prim);
+ prefix, ifa->ifa_prefixlen, prim,
+ ifa->ifa_rt_priority);
/* Add network specific broadcasts, when it takes a sense */
if (ifa->ifa_prefixlen < 31) {
- fib_magic(RTM_NEWROUTE, RTN_BROADCAST, prefix, 32, prim);
+ fib_magic(RTM_NEWROUTE, RTN_BROADCAST, prefix, 32,
+ prim, 0);
fib_magic(RTM_NEWROUTE, RTN_BROADCAST, prefix | ~mask,
- 32, prim);
+ 32, prim, 0);
}
}
}
+void fib_modify_prefix_metric(struct in_ifaddr *ifa, u32 new_metric)
+{
+ __be32 prefix = ifa->ifa_address & ifa->ifa_mask;
+ struct in_device *in_dev = ifa->ifa_dev;
+ struct net_device *dev = in_dev->dev;
+
+ if (!(dev->flags & IFF_UP) ||
+ ifa->ifa_flags & (IFA_F_SECONDARY | IFA_F_NOPREFIXROUTE) ||
+ ipv4_is_zeronet(prefix) ||
+ prefix == ifa->ifa_local || ifa->ifa_prefixlen == 32)
+ return;
+
+ /* add the new */
+ fib_magic(RTM_NEWROUTE,
+ dev->flags & IFF_LOOPBACK ? RTN_LOCAL : RTN_UNICAST,
+ prefix, ifa->ifa_prefixlen, ifa, new_metric);
+
+ /* delete the old */
+ fib_magic(RTM_DELROUTE,
+ dev->flags & IFF_LOOPBACK ? RTN_LOCAL : RTN_UNICAST,
+ prefix, ifa->ifa_prefixlen, ifa, ifa->ifa_rt_priority);
+}
+
/* Delete primary or secondary address.
* Optionally, on secondary address promotion consider the addresses
* from subnet iprim as deleted, even if they are in device list.
@@ -967,7 +995,7 @@ void fib_del_ifaddr(struct in_ifaddr *ifa, struct in_ifaddr *iprim)
if (!(ifa->ifa_flags & IFA_F_NOPREFIXROUTE))
fib_magic(RTM_DELROUTE,
dev->flags & IFF_LOOPBACK ? RTN_LOCAL : RTN_UNICAST,
- any, ifa->ifa_prefixlen, prim);
+ any, ifa->ifa_prefixlen, prim, 0);
subnet = 1;
}
@@ -1051,17 +1079,20 @@ void fib_del_ifaddr(struct in_ifaddr *ifa, struct in_ifaddr *iprim)
no_promotions:
if (!(ok & BRD_OK))
- fib_magic(RTM_DELROUTE, RTN_BROADCAST, ifa->ifa_broadcast, 32, prim);
+ fib_magic(RTM_DELROUTE, RTN_BROADCAST, ifa->ifa_broadcast, 32,
+ prim, 0);
if (subnet && ifa->ifa_prefixlen < 31) {
if (!(ok & BRD1_OK))
- fib_magic(RTM_DELROUTE, RTN_BROADCAST, brd, 32, prim);
+ fib_magic(RTM_DELROUTE, RTN_BROADCAST, brd, 32,
+ prim, 0);
if (!(ok & BRD0_OK))
- fib_magic(RTM_DELROUTE, RTN_BROADCAST, any, 32, prim);
+ fib_magic(RTM_DELROUTE, RTN_BROADCAST, any, 32,
+ prim, 0);
}
if (!(ok & LOCAL_OK)) {
unsigned int addr_type;
- fib_magic(RTM_DELROUTE, RTN_LOCAL, ifa->ifa_local, 32, prim);
+ fib_magic(RTM_DELROUTE, RTN_LOCAL, ifa->ifa_local, 32, prim, 0);
/* Check, that this local address finally disappeared. */
addr_type = inet_addr_type_dev_table(dev_net(dev), dev,
--
2.11.0
^ permalink raw reply related
* [PATCH iproute2-next] ipaddress: Add support for address metric
From: dsahern @ 2018-05-27 15:10 UTC (permalink / raw)
To: netdev; +Cc: roopa, David Ahern
In-Reply-To: <20180527151000.30488-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
Add support for IFA_RT_PRIORITY using the same keywords as iproute for
RTA_PRIORITY.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
include/uapi/linux/if_addr.h | 1 +
ip/ipaddress.c | 15 ++++++++++++++-
man/man8/ip-address.8.in | 6 ++++++
3 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/if_addr.h b/include/uapi/linux/if_addr.h
index c4899e22cdaa..a924606f36e5 100644
--- a/include/uapi/linux/if_addr.h
+++ b/include/uapi/linux/if_addr.h
@@ -33,6 +33,7 @@ enum {
IFA_CACHEINFO,
IFA_MULTICAST,
IFA_FLAGS,
+ IFA_RT_PRIORITY, /* u32, priority/metric for prefix route */
__IFA_MAX,
};
diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 75539e057f6a..6b53b753ead9 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -63,7 +63,7 @@ static void usage(void)
fprintf(stderr, " ip address {showdump|restore}\n");
fprintf(stderr, "IFADDR := PREFIX | ADDR peer PREFIX\n");
fprintf(stderr, " [ broadcast ADDR ] [ anycast ADDR ]\n");
- fprintf(stderr, " [ label IFNAME ] [ scope SCOPE-ID ]\n");
+ fprintf(stderr, " [ label IFNAME ] [ scope SCOPE-ID ] [ metric METRIC ]\n");
fprintf(stderr, "SCOPE-ID := [ host | link | global | NUMBER ]\n");
fprintf(stderr, "FLAG-LIST := [ FLAG-LIST ] FLAG\n");
fprintf(stderr, "FLAG := [ permanent | dynamic | secondary | primary |\n");
@@ -1328,6 +1328,10 @@ int print_addrinfo(const struct sockaddr_nl *who, struct nlmsghdr *n,
rta_tb[IFA_ADDRESS]));
}
print_int(PRINT_ANY, "prefixlen", "/%d ", ifa->ifa_prefixlen);
+
+ if (rta_tb[IFA_RT_PRIORITY])
+ print_uint(PRINT_ANY, "metric", "metric %u ",
+ rta_getattr_u32(rta_tb[IFA_RT_PRIORITY]));
}
if (brief)
@@ -2119,6 +2123,15 @@ static int ipaddr_modify(int cmd, int flags, int argc, char **argv)
NEXT_ARG();
l = *argv;
addattr_l(&req.n, sizeof(req), IFA_LABEL, l, strlen(l)+1);
+ } else if (matches(*argv, "metric") == 0 ||
+ matches(*argv, "priority") == 0 ||
+ matches(*argv, "preference") == 0) {
+ __u32 metric;
+
+ NEXT_ARG();
+ if (get_u32(&metric, *argv, 0))
+ invarg("\"metric\" value is invalid\n", *argv);
+ addattr32(&req.n, sizeof(req), IFA_RT_PRIORITY, metric);
} else if (matches(*argv, "valid_lft") == 0) {
if (valid_lftp)
duparg("valid_lft", *argv);
diff --git a/man/man8/ip-address.8.in b/man/man8/ip-address.8.in
index 7ebf0bc98dcd..c3861b3725cc 100644
--- a/man/man8/ip-address.8.in
+++ b/man/man8/ip-address.8.in
@@ -27,6 +27,8 @@ ip-address \- protocol address management
.IR IFNAME " ] [ "
.B scope
.IR SCOPE-ID " ] [ "
+.B metric
+.IR METRIC " ] [ "
.B to
.IR PREFIX " ] [ " FLAG-LIST " ] [ "
.B label
@@ -215,6 +217,10 @@ valid inside this site.
.in -8
.TP
+.BI metric " NUMBER"
+priority of prefix route associated with address.
+
+.TP
.BI valid_lft " LFT"
the valid lifetime of this address; see section 5.5.4 of
RFC 4862. When it expires, the address is removed by the kernel.
--
2.11.0
^ permalink raw reply related
* [PATCH net-next 7/7] selftests: fib_tests: Add prefix route tests with metric
From: dsahern @ 2018-05-27 15:09 UTC (permalink / raw)
To: netdev; +Cc: roopa, David Ahern
In-Reply-To: <20180527151000.30488-1-dsahern@kernel.org>
From: David Ahern <dsahern@gmail.com>
Add tests verifying prefix routes are inserted with expected metric.
IPv6 prefix route tests
TEST: Default metric [ OK ]
TEST: User specified metric on first device [ OK ]
TEST: User specified metric on second device [ OK ]
TEST: Delete of address on first device [ OK ]
TEST: Modify metric of address [ OK ]
TEST: Prefix route removed on link down [ OK ]
TEST: Prefix route with metric on link up [ OK ]
IPv4 prefix route tests
TEST: Default metric [ OK ]
TEST: User specified metric on first device [ OK ]
TEST: User specified metric on second device [ OK ]
TEST: Delete of address on first device [ OK ]
TEST: Modify metric of address [ OK ]
TEST: Prefix route removed on link down [ OK ]
TEST: Prefix route with metric on link up [ OK ]
Signed-off-by: David Ahern <dsahern@gmail.com>
---
tools/testing/selftests/net/fib_tests.sh | 181 ++++++++++++++++++++++++++++++-
1 file changed, 180 insertions(+), 1 deletion(-)
mode change 100755 => 100644 tools/testing/selftests/net/fib_tests.sh
diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
old mode 100755
new mode 100644
index e7d76fbc36e9..9780c5a2d73f
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -6,7 +6,8 @@
ret=0
-TESTS="unregister down carrier nexthop ipv6_rt ipv4_rt"
+# all tests in this script. Can be overridden with -t option
+TESTS="unregister down carrier nexthop ipv6_rt ipv4_rt ipv6_addr_metric ipv4_addr_metric"
VERBOSE=0
PAUSE_ON_FAIL=no
PAUSE=no
@@ -642,6 +643,8 @@ check_route6()
local rc=0
out=$($IP -6 ro ls match ${pfx} | sed -e 's/ pref medium//')
+ [ "${out}" = "${expected}" ] && return 0
+
if [ -z "${out}" ]; then
if [ "$VERBOSE" = "1" ]; then
printf "\nNo route entry found\n"
@@ -911,6 +914,98 @@ ipv6_route_test()
route_cleanup
}
+ip_addr_metric_check()
+{
+ ip addr help 2>&1 | grep -q metric
+ if [ $? -ne 0 ]; then
+ echo "iproute2 command does not support metric for addresses. Skipping test"
+ return 1
+ fi
+
+ return 0
+}
+
+ipv6_addr_metric_test()
+{
+ local rc
+
+ echo
+ echo "IPv6 prefix route tests"
+
+ ip_addr_metric_check || return 1
+
+ setup
+
+ set -e
+ $IP li add dummy1 type dummy
+ $IP li add dummy2 type dummy
+ $IP li set dummy1 up
+ $IP li set dummy2 up
+
+ # default entry is metric 256
+ run_cmd "$IP -6 addr add dev dummy1 2001:db8:104::1/64"
+ run_cmd "$IP -6 addr add dev dummy2 2001:db8:104::2/64"
+ set +e
+
+ check_route6 "2001:db8:104::/64 dev dummy1 proto kernel metric 256 2001:db8:104::/64 dev dummy2 proto kernel metric 256"
+ log_test $? 0 "Default metric"
+
+ set -e
+ run_cmd "$IP -6 addr flush dev dummy1"
+ run_cmd "$IP -6 addr add dev dummy1 2001:db8:104::1/64 metric 257"
+ set +e
+
+ check_route6 "2001:db8:104::/64 dev dummy2 proto kernel metric 256 2001:db8:104::/64 dev dummy1 proto kernel metric 257"
+ log_test $? 0 "User specified metric on first device"
+
+ set -e
+ run_cmd "$IP -6 addr flush dev dummy2"
+ run_cmd "$IP -6 addr add dev dummy2 2001:db8:104::2/64 metric 258"
+ set +e
+
+ check_route6 "2001:db8:104::/64 dev dummy1 proto kernel metric 257 2001:db8:104::/64 dev dummy2 proto kernel metric 258"
+ log_test $? 0 "User specified metric on second device"
+
+ run_cmd "$IP -6 addr del dev dummy1 2001:db8:104::1/64 metric 257"
+ rc=$?
+ if [ $rc -eq 0 ]; then
+ check_route6 "2001:db8:104::/64 dev dummy2 proto kernel metric 258"
+ rc=$?
+ fi
+ log_test $rc 0 "Delete of address on first device"
+
+ run_cmd "$IP -6 addr change dev dummy2 2001:db8:104::2/64 metric 259"
+ rc=$?
+ if [ $rc -eq 0 ]; then
+ check_route6 "2001:db8:104::/64 dev dummy2 proto kernel metric 259"
+ rc=$?
+ fi
+ log_test $rc 0 "Modify metric of address"
+
+ # verify prefix route removed on down
+ run_cmd "ip netns exec testns sysctl -qw net.ipv6.conf.all.keep_addr_on_down=1"
+ run_cmd "$IP li set dev dummy2 down"
+ rc=$?
+ if [ $rc -eq 0 ]; then
+ check_route6 ""
+ rc=$?
+ fi
+ log_test $rc 0 "Prefix route removed on link down"
+
+ # verify prefix route re-inserted with assigned metric
+ run_cmd "$IP li set dev dummy2 up"
+ rc=$?
+ if [ $rc -eq 0 ]; then
+ check_route6 "2001:db8:104::/64 dev dummy2 proto kernel metric 259"
+ rc=$?
+ fi
+ log_test $rc 0 "Prefix route with metric on link up"
+
+ $IP li del dummy1
+ $IP li del dummy2
+ cleanup
+}
+
# add route for a prefix, flushing any existing routes first
# expected to be the first step of a test
add_route()
@@ -955,6 +1050,8 @@ check_route()
local rc=0
out=$($IP ro ls match ${pfx})
+ [ "${out}" = "${expected}" ] && return 0
+
if [ -z "${out}" ]; then
if [ "$VERBOSE" = "1" ]; then
printf "\nNo route entry found\n"
@@ -1181,6 +1278,86 @@ ipv4_route_test()
route_cleanup
}
+ipv4_addr_metric_test()
+{
+ local rc
+
+ echo
+ echo "IPv4 prefix route tests"
+
+ ip_addr_metric_check || return 1
+
+ setup
+
+ set -e
+ $IP li add dummy1 type dummy
+ $IP li add dummy2 type dummy
+ $IP li set dummy1 up
+ $IP li set dummy2 up
+
+ # default entry is metric 256
+ run_cmd "$IP addr add dev dummy1 172.16.104.1/24"
+ run_cmd "$IP addr add dev dummy2 172.16.104.2/24"
+ set +e
+
+ check_route "172.16.104.0/24 dev dummy1 proto kernel scope link src 172.16.104.1 172.16.104.0/24 dev dummy2 proto kernel scope link src 172.16.104.2"
+ log_test $? 0 "Default metric"
+
+ set -e
+ run_cmd "$IP addr flush dev dummy1"
+ run_cmd "$IP addr add dev dummy1 172.16.104.1/24 metric 257"
+ set +e
+
+ check_route "172.16.104.0/24 dev dummy2 proto kernel scope link src 172.16.104.2 172.16.104.0/24 dev dummy1 proto kernel scope link src 172.16.104.1 metric 257"
+ log_test $? 0 "User specified metric on first device"
+
+ set -e
+ run_cmd "$IP addr flush dev dummy2"
+ run_cmd "$IP addr add dev dummy2 172.16.104.2/24 metric 258"
+ set +e
+
+ check_route "172.16.104.0/24 dev dummy1 proto kernel scope link src 172.16.104.1 metric 257 172.16.104.0/24 dev dummy2 proto kernel scope link src 172.16.104.2 metric 258"
+ log_test $? 0 "User specified metric on second device"
+
+ run_cmd "$IP addr del dev dummy1 172.16.104.1/24 metric 257"
+ rc=$?
+ if [ $rc -eq 0 ]; then
+ check_route "172.16.104.0/24 dev dummy2 proto kernel scope link src 172.16.104.2 metric 258"
+ rc=$?
+ fi
+ log_test $rc 0 "Delete of address on first device"
+
+ run_cmd "$IP addr change dev dummy2 172.16.104.2/24 metric 259"
+ rc=$?
+ if [ $rc -eq 0 ]; then
+ check_route "172.16.104.0/24 dev dummy2 proto kernel scope link src 172.16.104.2 metric 259"
+ rc=$?
+ fi
+ log_test $rc 0 "Modify metric of address"
+
+ # verify prefix route removed on down
+ run_cmd "$IP li set dev dummy2 down"
+ rc=$?
+ if [ $rc -eq 0 ]; then
+ check_route ""
+ rc=$?
+ fi
+ log_test $rc 0 "Prefix route removed on link down"
+
+ # verify prefix route re-inserted with assigned metric
+ run_cmd "$IP li set dev dummy2 up"
+ rc=$?
+ if [ $rc -eq 0 ]; then
+ check_route "172.16.104.0/24 dev dummy2 proto kernel scope link src 172.16.104.2 metric 259"
+ rc=$?
+ fi
+ log_test $rc 0 "Prefix route with metric on link up"
+
+ $IP li del dummy1
+ $IP li del dummy2
+ cleanup
+}
+
################################################################################
# usage
@@ -1245,6 +1422,8 @@ do
fib_nexthop_test|nexthop) fib_nexthop_test;;
ipv6_route_test|ipv6_rt) ipv6_route_test;;
ipv4_route_test|ipv4_rt) ipv4_route_test;;
+ ipv6_addr_metric) ipv6_addr_metric_test;;
+ ipv4_addr_metric) ipv4_addr_metric_test;;
help) echo "Test names: $TESTS"; exit 0;;
esac
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net] VSOCK: check sk state before receive
From: Hangbin Liu @ 2018-05-27 15:29 UTC (permalink / raw)
To: netdev; +Cc: Stefan Hajnoczi, Jorgen Hansen, David S. Miller
In-Reply-To: <1527382936-4850-1-git-send-email-liuhangbin@gmail.com>
Hmm...Although I won't reproduce this bug with my reproducer after
apply my patch. I could still get a similiar issue with syzkaller sock vnet test.
It looks this patch is not complete. Here is the KASAN call trace with my patch.
I can also reproduce it without my patch.
==================================================================
BUG: KASAN: use-after-free in vmci_transport_allow_dgram.part.7+0x155/0x1a0 [vmw_vsock_vmci_transport]
Read of size 4 at addr ffff880026a3a914 by task kworker/0:2/96
CPU: 0 PID: 96 Comm: kworker/0:2 Not tainted 4.17.0-rc6.vsock+ #28
Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
Workqueue: events dg_delayed_dispatch [vmw_vmci]
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0xdd/0x18e lib/dump_stack.c:113
print_address_description+0x7a/0x3e0 mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report+0x1dd/0x460 mm/kasan/report.c:412
vmci_transport_allow_dgram.part.7+0x155/0x1a0 [vmw_vsock_vmci_transport]
vmci_transport_recv_dgram_cb+0x5d/0x200 [vmw_vsock_vmci_transport]
dg_delayed_dispatch+0x99/0x1b0 [vmw_vmci]
process_one_work+0xa4e/0x1720 kernel/workqueue.c:2145
worker_thread+0x1df/0x1400 kernel/workqueue.c:2279
kthread+0x343/0x4b0 kernel/kthread.c:240
ret_from_fork+0x35/0x40 arch/x86/entry/entry_64.S:412
Allocated by task 2684:
set_track mm/kasan/kasan.c:460 [inline]
kasan_kmalloc+0xa0/0xd0 mm/kasan/kasan.c:553
slab_post_alloc_hook mm/slab.h:444 [inline]
slab_alloc_node mm/slub.c:2741 [inline]
slab_alloc mm/slub.c:2749 [inline]
kmem_cache_alloc+0x105/0x330 mm/slub.c:2754
sk_prot_alloc+0x6a/0x2c0 net/core/sock.c:1468
sk_alloc+0xc9/0xbb0 net/core/sock.c:1528
__vsock_create+0xc8/0x9b0 [vsock]
vsock_create+0xfd/0x1a0 [vsock]
__sock_create+0x310/0x690 net/socket.c:1285
sock_create net/socket.c:1325 [inline]
__sys_socket+0x101/0x240 net/socket.c:1355
__do_sys_socket net/socket.c:1364 [inline]
__se_sys_socket net/socket.c:1362 [inline]
__x64_sys_socket+0x7d/0xd0 net/socket.c:1362
do_syscall_64+0x175/0x630 arch/x86/entry/common.c:287
entry_SYSCALL_64_after_hwframe+0x44/0xa9
Freed by task 2684:
set_track mm/kasan/kasan.c:460 [inline]
__kasan_slab_free+0x130/0x180 mm/kasan/kasan.c:521
slab_free_hook mm/slub.c:1388 [inline]
slab_free_freelist_hook mm/slub.c:1415 [inline]
slab_free mm/slub.c:2988 [inline]
kmem_cache_free+0xce/0x410 mm/slub.c:3004
sk_prot_free net/core/sock.c:1509 [inline]
__sk_destruct+0x629/0x940 net/core/sock.c:1593
sk_destruct+0x4e/0x90 net/core/sock.c:1601
__sk_free+0xd3/0x320 net/core/sock.c:1612
sk_free+0x2a/0x30 net/core/sock.c:1623
__vsock_release+0x431/0x610 [vsock]
vsock_release+0x3c/0xc0 [vsock]
sock_release+0x91/0x200 net/socket.c:594
sock_close+0x17/0x20 net/socket.c:1149
__fput+0x368/0xa20 fs/file_table.c:209
task_work_run+0x1c5/0x2a0 kernel/task_work.c:113
exit_task_work include/linux/task_work.h:22 [inline]
do_exit+0x1876/0x26c0 kernel/exit.c:865
do_group_exit+0x159/0x3e0 kernel/exit.c:968
get_signal+0x65a/0x1780 kernel/signal.c:2482
do_signal+0xa4/0x1fe0 arch/x86/kernel/signal.c:810
exit_to_usermode_loop+0x1b8/0x260 arch/x86/entry/common.c:162
prepare_exit_to_usermode arch/x86/entry/common.c:196 [inline]
syscall_return_slowpath arch/x86/entry/common.c:265 [inline]
do_syscall_64+0x505/0x630 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x44/0xa9
The buggy address belongs to the object at ffff880026a3a600
which belongs to the cache AF_VSOCK of size 1056
The buggy address is located 788 bytes inside of
1056-byte region [ffff880026a3a600, ffff880026a3aa20)
The buggy address belongs to the page:
page:ffffea00009a8e00 count:1 mapcount:0 mapping:0000000000000000 index:0x0 compound_mapcount: 0
flags: 0xfffffc0008100(slab|head)
raw: 000fffffc0008100 0000000000000000 0000000000000000 00000001000d000d
raw: dead000000000100 dead000000000200 ffff880034471a40 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff880026a3a800: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff880026a3a880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff880026a3a900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff880026a3a980: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff880026a3aa00: fb fb fb fb fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================
^ permalink raw reply
* [PATCH v2 0/5] build warnings cleanup
From: Atul Gupta @ 2018-05-27 15:45 UTC (permalink / raw)
To: herbert, linux-crypto; +Cc: gustavo, dan.carpenter, netdev, davem, atul.gupta
Build warnings cleanup reported for
- using only 128b key
- wait for buffer in sendmsg/sendpage
- check for null before using skb
- free rspq_skb_cache in error path
- indentation
v2:
Added bug report description for 0002
Incorported comments from Dan Carpenter
Atul Gupta (5):
crypto:chtls: key len correction
crypto: chtls: wait for memory sendmsg, sendpage
crypto: chtls: dereference null variable
crypto: chtls: kbuild warnings
crypto: chtls: free beyond end rspq_skb_cache
drivers/crypto/chelsio/chtls/chtls.h | 1 +
drivers/crypto/chelsio/chtls/chtls_hw.c | 6 +-
drivers/crypto/chelsio/chtls/chtls_io.c | 104 +++++++++++++++++++++++++++---
drivers/crypto/chelsio/chtls/chtls_main.c | 3 +-
4 files changed, 98 insertions(+), 16 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH v2 1/5] crypto:chtls: key len correction
From: Atul Gupta @ 2018-05-27 15:45 UTC (permalink / raw)
To: herbert, linux-crypto; +Cc: gustavo, dan.carpenter, netdev, davem, atul.gupta
In-Reply-To: <1527435922-6727-1-git-send-email-atul.gupta@chelsio.com>
corrected the key length to copy 128b key. Removed 192b and 256b
key as user input supports key of size 128b in gcm_ctx
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Atul Gupta <atul.gupta@chelsio.com>
---
drivers/crypto/chelsio/chtls/chtls_hw.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/crypto/chelsio/chtls/chtls_hw.c b/drivers/crypto/chelsio/chtls/chtls_hw.c
index 54a13aa9..55d5014 100644
--- a/drivers/crypto/chelsio/chtls/chtls_hw.c
+++ b/drivers/crypto/chelsio/chtls/chtls_hw.c
@@ -213,7 +213,7 @@ static int chtls_key_info(struct chtls_sock *csk,
struct _key_ctx *kctx,
u32 keylen, u32 optname)
{
- unsigned char key[CHCR_KEYCTX_CIPHER_KEY_SIZE_256];
+ unsigned char key[AES_KEYSIZE_128];
struct tls12_crypto_info_aes_gcm_128 *gcm_ctx;
unsigned char ghash_h[AEAD_H_SIZE];
struct crypto_cipher *cipher;
@@ -228,10 +228,6 @@ static int chtls_key_info(struct chtls_sock *csk,
if (keylen == AES_KEYSIZE_128) {
ck_size = CHCR_KEYCTX_CIPHER_KEY_SIZE_128;
- } else if (keylen == AES_KEYSIZE_192) {
- ck_size = CHCR_KEYCTX_CIPHER_KEY_SIZE_192;
- } else if (keylen == AES_KEYSIZE_256) {
- ck_size = CHCR_KEYCTX_CIPHER_KEY_SIZE_256;
} else {
pr_err("GCM: Invalid key length %d\n", keylen);
return -EINVAL;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 2/5] crypto: chtls: wait for memory sendmsg, sendpage
From: Atul Gupta @ 2018-05-27 15:45 UTC (permalink / raw)
To: herbert, linux-crypto; +Cc: gustavo, dan.carpenter, netdev, davem, atul.gupta
In-Reply-To: <1527435922-6727-1-git-send-email-atul.gupta@chelsio.com>
address suspicious code <gustavo@embeddedor.com>
1210 set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
1211 }
The issue is that in the code above, set_bit is never reached
due to the 'continue' statement at line 1208.
Also reported by bug report:<dan.carpenter@oracle.com>
1210 set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Not reachable.
Its required to wait for buffer in the send path and takes care of
unaddress and un-handled SOCK_NOSPACE.
v2: use csk_mem_free where appropriate
proper indent of goto do_nonblock
replace out with do_rm_wq
Reported-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Atul Gupta <atul.gupta@chelsio.com>
---
drivers/crypto/chelsio/chtls/chtls.h | 1 +
drivers/crypto/chelsio/chtls/chtls_io.c | 90 +++++++++++++++++++++++++++++--
drivers/crypto/chelsio/chtls/chtls_main.c | 1 +
3 files changed, 89 insertions(+), 3 deletions(-)
diff --git a/drivers/crypto/chelsio/chtls/chtls.h b/drivers/crypto/chelsio/chtls/chtls.h
index 1b2f43c..a53a0e6 100644
--- a/drivers/crypto/chelsio/chtls/chtls.h
+++ b/drivers/crypto/chelsio/chtls/chtls.h
@@ -144,6 +144,7 @@ struct chtls_dev {
struct list_head rcu_node;
struct list_head na_node;
unsigned int send_page_order;
+ int max_host_sndbuf;
struct key_map kmap;
};
diff --git a/drivers/crypto/chelsio/chtls/chtls_io.c b/drivers/crypto/chelsio/chtls/chtls_io.c
index 840dd01..7aa5d90 100644
--- a/drivers/crypto/chelsio/chtls/chtls_io.c
+++ b/drivers/crypto/chelsio/chtls/chtls_io.c
@@ -914,6 +914,78 @@ static u16 tls_header_read(struct tls_hdr *thdr, struct iov_iter *from)
return (__force u16)cpu_to_be16(thdr->length);
}
+static int csk_mem_free(struct chtls_dev *cdev, struct sock *sk)
+{
+ return (cdev->max_host_sndbuf - sk->sk_wmem_queued);
+}
+
+static int csk_wait_memory(struct chtls_dev *cdev,
+ struct sock *sk, long *timeo_p)
+{
+ DEFINE_WAIT_FUNC(wait, woken_wake_function);
+ int sndbuf, err = 0;
+ long current_timeo;
+ long vm_wait = 0;
+ bool noblock;
+
+ current_timeo = *timeo_p;
+ noblock = (*timeo_p ? false : true);
+ sndbuf = cdev->max_host_sndbuf;
+ if (csk_mem_free(cdev, sk)) {
+ current_timeo = (prandom_u32() % (HZ / 5)) + 2;
+ vm_wait = (prandom_u32() % (HZ / 5)) + 2;
+ }
+
+ add_wait_queue(sk_sleep(sk), &wait);
+ while (1) {
+ sk_set_bit(SOCKWQ_ASYNC_NOSPACE, sk);
+
+ if (sk->sk_err || (sk->sk_shutdown & SEND_SHUTDOWN))
+ goto do_error;
+ if (!*timeo_p) {
+ if (noblock)
+ set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+ goto do_nonblock;
+ }
+ if (signal_pending(current))
+ goto do_interrupted;
+ sk_clear_bit(SOCKWQ_ASYNC_NOSPACE, sk);
+ if (csk_mem_free(cdev, sk) && !vm_wait)
+ break;
+
+ set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+ sk->sk_write_pending++;
+ sk_wait_event(sk, ¤t_timeo, sk->sk_err ||
+ (sk->sk_shutdown & SEND_SHUTDOWN) ||
+ (csk_mem_free(cdev, sk) && !vm_wait), &wait);
+ sk->sk_write_pending--;
+
+ if (vm_wait) {
+ vm_wait -= current_timeo;
+ current_timeo = *timeo_p;
+ if (current_timeo != MAX_SCHEDULE_TIMEOUT) {
+ current_timeo -= vm_wait;
+ if (current_timeo < 0)
+ current_timeo = 0;
+ }
+ vm_wait = 0;
+ }
+ *timeo_p = current_timeo;
+ }
+do_rm_wq:
+ remove_wait_queue(sk_sleep(sk), &wait);
+ return err;
+do_error:
+ err = -EPIPE;
+ goto do_rm_wq;
+do_nonblock:
+ err = -EAGAIN;
+ goto do_rm_wq;
+do_interrupted:
+ err = sock_intr_errno(*timeo_p);
+ goto do_rm_wq;
+}
+
int chtls_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
{
struct chtls_sock *csk = rcu_dereference_sk_user_data(sk);
@@ -952,6 +1024,8 @@ int chtls_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
copy = mss - skb->len;
skb->ip_summed = CHECKSUM_UNNECESSARY;
}
+ if (!csk_mem_free(cdev, sk))
+ goto wait_for_sndbuf;
if (is_tls_tx(csk) && !csk->tlshws.txleft) {
struct tls_hdr hdr;
@@ -1099,8 +1173,10 @@ int chtls_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
if (ULP_SKB_CB(skb)->flags & ULPCB_FLAG_NO_APPEND)
push_frames_if_head(sk);
continue;
+wait_for_sndbuf:
+ set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
wait_for_memory:
- err = sk_stream_wait_memory(sk, &timeo);
+ err = csk_wait_memory(cdev, sk, &timeo);
if (err)
goto do_error;
}
@@ -1131,6 +1207,7 @@ int chtls_sendpage(struct sock *sk, struct page *page,
int offset, size_t size, int flags)
{
struct chtls_sock *csk;
+ struct chtls_dev *cdev;
int mss, err, copied;
struct tcp_sock *tp;
long timeo;
@@ -1138,6 +1215,7 @@ int chtls_sendpage(struct sock *sk, struct page *page,
tp = tcp_sk(sk);
copied = 0;
csk = rcu_dereference_sk_user_data(sk);
+ cdev = csk->cdev;
timeo = sock_sndtimeo(sk, flags & MSG_DONTWAIT);
err = sk_stream_wait_connect(sk, &timeo);
@@ -1156,6 +1234,8 @@ int chtls_sendpage(struct sock *sk, struct page *page,
if (!skb || (ULP_SKB_CB(skb)->flags & ULPCB_FLAG_NO_APPEND) ||
copy <= 0) {
new_buf:
+ if (!csk_mem_free(cdev, sk))
+ goto wait_for_sndbuf;
if (is_tls_tx(csk)) {
skb = get_record_skb(sk,
@@ -1167,7 +1247,7 @@ int chtls_sendpage(struct sock *sk, struct page *page,
skb = get_tx_skb(sk, 0);
}
if (!skb)
- goto do_error;
+ goto wait_for_memory;
copy = mss;
}
if (copy > size)
@@ -1206,8 +1286,12 @@ int chtls_sendpage(struct sock *sk, struct page *page,
if (unlikely(ULP_SKB_CB(skb)->flags & ULPCB_FLAG_NO_APPEND))
push_frames_if_head(sk);
continue;
-
+wait_for_sndbuf:
set_bit(SOCK_NOSPACE, &sk->sk_socket->flags);
+wait_for_memory:
+ err = csk_wait_memory(cdev, sk, &timeo);
+ if (err)
+ goto do_error;
}
out:
csk_reset_flag(csk, CSK_TX_MORE_DATA);
diff --git a/drivers/crypto/chelsio/chtls/chtls_main.c b/drivers/crypto/chelsio/chtls/chtls_main.c
index 53ffb00..273afd3 100644
--- a/drivers/crypto/chelsio/chtls/chtls_main.c
+++ b/drivers/crypto/chelsio/chtls/chtls_main.c
@@ -238,6 +238,7 @@ static void *chtls_uld_add(const struct cxgb4_lld_info *info)
spin_lock_init(&cdev->idr_lock);
cdev->send_page_order = min_t(uint, get_order(32768),
send_page_order);
+ cdev->max_host_sndbuf = 48 * 1024;
if (lldi->vr->key.size)
if (chtls_init_kmap(cdev, lldi))
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 3/5] crypto: chtls: dereference null variable
From: Atul Gupta @ 2018-05-27 15:45 UTC (permalink / raw)
To: herbert, linux-crypto; +Cc: gustavo, dan.carpenter, netdev, davem, atul.gupta
In-Reply-To: <1527435922-6727-1-git-send-email-atul.gupta@chelsio.com>
skb dereferenced before check in sendpage
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Atul Gupta <atul.gupta@chelsio.com>
---
drivers/crypto/chelsio/chtls/chtls_io.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/crypto/chelsio/chtls/chtls_io.c b/drivers/crypto/chelsio/chtls/chtls_io.c
index 7aa5d90..8cfc27b 100644
--- a/drivers/crypto/chelsio/chtls/chtls_io.c
+++ b/drivers/crypto/chelsio/chtls/chtls_io.c
@@ -1230,9 +1230,8 @@ int chtls_sendpage(struct sock *sk, struct page *page,
struct sk_buff *skb = skb_peek_tail(&csk->txq);
int copy, i;
- copy = mss - skb->len;
if (!skb || (ULP_SKB_CB(skb)->flags & ULPCB_FLAG_NO_APPEND) ||
- copy <= 0) {
+ (copy = mss - skb->len) <= 0) {
new_buf:
if (!csk_mem_free(cdev, sk))
goto wait_for_sndbuf;
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 4/5] crypto: chtls: kbuild warnings
From: Atul Gupta @ 2018-05-27 15:45 UTC (permalink / raw)
To: herbert, linux-crypto; +Cc: gustavo, dan.carpenter, netdev, davem, atul.gupta
In-Reply-To: <1527435922-6727-1-git-send-email-atul.gupta@chelsio.com>
- unindented continue
- check for null page
- signed return
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Atul Gupta <atul.gupta@chelsio.com>
---
drivers/crypto/chelsio/chtls/chtls_io.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/crypto/chelsio/chtls/chtls_io.c b/drivers/crypto/chelsio/chtls/chtls_io.c
index 8cfc27b..51fc682 100644
--- a/drivers/crypto/chelsio/chtls/chtls_io.c
+++ b/drivers/crypto/chelsio/chtls/chtls_io.c
@@ -907,11 +907,11 @@ static int chtls_skb_copy_to_page_nocache(struct sock *sk,
}
/* Read TLS header to find content type and data length */
-static u16 tls_header_read(struct tls_hdr *thdr, struct iov_iter *from)
+static int tls_header_read(struct tls_hdr *thdr, struct iov_iter *from)
{
if (copy_from_iter(thdr, sizeof(*thdr), from) != sizeof(*thdr))
return -EFAULT;
- return (__force u16)cpu_to_be16(thdr->length);
+ return (__force int)cpu_to_be16(thdr->length);
}
static int csk_mem_free(struct chtls_dev *cdev, struct sock *sk)
@@ -1083,9 +1083,10 @@ int chtls_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
int off = TCP_OFF(sk);
bool merge;
- if (page)
- pg_size <<= compound_order(page);
+ if (!page)
+ goto wait_for_memory;
+ pg_size <<= compound_order(page);
if (off < pg_size &&
skb_can_coalesce(skb, i, page, off)) {
merge = 1;
@@ -1492,7 +1493,7 @@ static int chtls_pt_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
break;
chtls_cleanup_rbuf(sk, copied);
sk_wait_data(sk, &timeo, NULL);
- continue;
+ continue;
found_ok_skb:
if (!skb->len) {
skb_dst_set(skb, NULL);
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 5/5] crypto: chtls: free beyond end rspq_skb_cache
From: Atul Gupta @ 2018-05-27 15:45 UTC (permalink / raw)
To: herbert, linux-crypto; +Cc: gustavo, dan.carpenter, netdev, davem, atul.gupta
In-Reply-To: <1527435922-6727-1-git-send-email-atul.gupta@chelsio.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Atul Gupta <atul.gupta@chelsio.com>
---
drivers/crypto/chelsio/chtls/chtls_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/chelsio/chtls/chtls_main.c b/drivers/crypto/chelsio/chtls/chtls_main.c
index 273afd3..9b07f91 100644
--- a/drivers/crypto/chelsio/chtls/chtls_main.c
+++ b/drivers/crypto/chelsio/chtls/chtls_main.c
@@ -250,7 +250,7 @@ static void *chtls_uld_add(const struct cxgb4_lld_info *info)
return cdev;
out_rspq_skb:
- for (j = 0; j <= i; j++)
+ for (j = 0; j < i; j++)
kfree_skb(cdev->rspq_skb_cache[j]);
kfree_skb(cdev->askb);
out_skb:
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH rdma-next v1 00/13] Verbs flow counters support
From: Leon Romanovsky @ 2018-05-27 16:22 UTC (permalink / raw)
To: Doug Ledford, Jason Gunthorpe
Cc: RDMA mailing list, Boris Pismenny, Matan Barak, Raed Salem,
Yishai Hadas, Saeed Mahameed, linux-netdev
In-Reply-To: <20180527102346.15149-1-leon@kernel.org>
[-- Attachment #1: Type: text/plain, Size: 904 bytes --]
On Sun, May 27, 2018 at 01:23:33PM +0300, Leon Romanovsky wrote:
> From: Leon Romanovsky <leonro@mellanox.com>
>
> Changelog v0->v1:
> * Decouple from DevX submission
> * Use uverbs_attr_get_obj at counters read method
> * Added define for max read buffer size (MAX_COUNTERS_BUFF_SIZE)
> * Removed the struct mlx5_ib_flow_counter basic_flow_cnts and
> the related structs used, used define instead
> * Took Matan's patch from DevX
> * uverbs_free_counters removed void* casting
> * Added check to bound ncounters value (added define
> * Changed user supplied data buffer structure to be array of
> struct <desc,index> pair (applied this change to user space also)
>
> Not changed:
> * UAPI files
> * Addition of uhw to flow
>
> Thanks
Sorry for the noise, but please drop this series.
We found improper locking sequence in one of error paths which can be
triggered by the user.
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* [PATCH v2 04/11] net: sched: always take reference to action
From: Vlad Buslov @ 2018-05-27 16:41 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jiri, ast, daniel, kliteyn,
Vlad Buslov
In-Reply-To: <1527439298-22306-1-git-send-email-vladbu@mellanox.com>
Without rtnl lock protection it is no longer safe to use pointer to tc
action without holding reference to it. (it can be destroyed concurrently)
Remove unsafe action idr lookup function. Instead of it, implement safe tcf
idr check function that atomically looks up action in idr and increments
its reference and bind counters. Implement both action search and check
using new safe function
Reference taken by idr check is temporal and should not be accounted by
userspace clients (both logically and to preserver current API behavior).
Subtract temporal reference when dumping action to userspace using existing
tca_get_fill function arguments.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_api.c | 46 ++++++++++++++++++++--------------------------
1 file changed, 20 insertions(+), 26 deletions(-)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 256b0c93916c..aa304d36fee0 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -284,44 +284,38 @@ int tcf_generic_walker(struct tc_action_net *tn, struct sk_buff *skb,
}
EXPORT_SYMBOL(tcf_generic_walker);
-static struct tc_action *tcf_idr_lookup(u32 index, struct tcf_idrinfo *idrinfo)
+static bool __tcf_idr_check(struct tc_action_net *tn, u32 index,
+ struct tc_action **a, int bind)
{
- struct tc_action *p = NULL;
+ struct tcf_idrinfo *idrinfo = tn->idrinfo;
+ struct tc_action *p;
spin_lock(&idrinfo->lock);
p = idr_find(&idrinfo->action_idr, index);
+ if (p) {
+ refcount_inc(&p->tcfa_refcnt);
+ if (bind)
+ atomic_inc(&p->tcfa_bindcnt);
+ }
spin_unlock(&idrinfo->lock);
- return p;
+ if (p) {
+ *a = p;
+ return true;
+ }
+ return false;
}
int tcf_idr_search(struct tc_action_net *tn, struct tc_action **a, u32 index)
{
- struct tcf_idrinfo *idrinfo = tn->idrinfo;
- struct tc_action *p = tcf_idr_lookup(index, idrinfo);
-
- if (p) {
- *a = p;
- return 1;
- }
- return 0;
+ return __tcf_idr_check(tn, index, a, 0);
}
EXPORT_SYMBOL(tcf_idr_search);
bool tcf_idr_check(struct tc_action_net *tn, u32 index, struct tc_action **a,
int bind)
{
- struct tcf_idrinfo *idrinfo = tn->idrinfo;
- struct tc_action *p = tcf_idr_lookup(index, idrinfo);
-
- if (index && p) {
- if (bind)
- atomic_inc(&p->tcfa_bindcnt);
- refcount_inc(&p->tcfa_refcnt);
- *a = p;
- return true;
- }
- return false;
+ return __tcf_idr_check(tn, index, a, bind);
}
EXPORT_SYMBOL(tcf_idr_check);
@@ -932,7 +926,7 @@ tcf_get_notify(struct net *net, u32 portid, struct nlmsghdr *n,
if (!skb)
return -ENOBUFS;
if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, event,
- 0, 0) <= 0) {
+ 0, 1) <= 0) {
NL_SET_ERR_MSG(extack, "Failed to fill netlink attributes while adding TC action");
kfree_skb(skb);
return -EINVAL;
@@ -1072,7 +1066,7 @@ tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
return -ENOBUFS;
if (tca_get_fill(skb, actions, portid, n->nlmsg_seq, 0, RTM_DELACTION,
- 0, 1) <= 0) {
+ 0, 2) <= 0) {
NL_SET_ERR_MSG(extack, "Failed to fill netlink TC action attributes");
kfree_skb(skb);
return -EINVAL;
@@ -1131,14 +1125,14 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
if (event == RTM_GETACTION)
ret = tcf_get_notify(net, portid, n, &actions, event, extack);
else { /* delete */
+ cleanup_a(&actions, 1); /* lookup took reference */
ret = tcf_del_notify(net, n, &actions, portid, attr_size, extack);
if (ret)
goto err;
return ret;
}
err:
- if (event != RTM_GETACTION)
- tcf_action_destroy(&actions, 0);
+ tcf_action_destroy(&actions, 0);
return ret;
}
--
2.7.5
^ permalink raw reply related
* [PATCH v2 09/11] net: sched: use reference counting action init
From: Vlad Buslov @ 2018-05-27 16:41 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jiri, ast, daniel, kliteyn,
Vlad Buslov
In-Reply-To: <1527439298-22306-1-git-send-email-vladbu@mellanox.com>
Change action API to assume that action init function always takes
reference to action, even when overwriting existing action. This is
necessary because action API continues to use action pointer after init
function is done. At this point action becomes accessible for concurrent
modifications, so user must always hold reference to it.
Implement helper put list function to atomically release list of actions
after action API init code is done using them.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_api.c | 35 +++++++++++++++++------------------
1 file changed, 17 insertions(+), 18 deletions(-)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index f019f0464cec..eefe8c2fe667 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -627,6 +627,18 @@ static int tcf_action_put(struct tc_action *p)
return __tcf_action_put(p, false);
}
+static void tcf_action_put_lst(struct list_head *actions)
+{
+ struct tc_action *a, *tmp;
+
+ list_for_each_entry_safe(a, tmp, actions, list) {
+ const struct tc_action_ops *ops = a->ops;
+
+ if (tcf_action_put(a))
+ module_put(ops->owner);
+ }
+}
+
int
tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
{
@@ -835,17 +847,6 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
return ERR_PTR(err);
}
-static void cleanup_a(struct list_head *actions, int ovr)
-{
- struct tc_action *a;
-
- if (!ovr)
- return;
-
- list_for_each_entry(a, actions, list)
- refcount_dec(&a->tcfa_refcnt);
-}
-
int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
struct nlattr *est, char *name, int ovr, int bind,
struct list_head *actions, size_t *attr_size,
@@ -874,11 +875,6 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
}
*attr_size = tcf_action_full_attrs_size(sz);
-
- /* Remove the temp refcnt which was necessary to protect against
- * destroying an existing action which was being replaced
- */
- cleanup_a(actions, ovr);
return 0;
err:
@@ -1209,7 +1205,7 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
return ret;
}
err:
- tcf_action_destroy(&actions, 0);
+ tcf_action_put_lst(&actions);
return ret;
}
@@ -1251,8 +1247,11 @@ static int tcf_action_add(struct net *net, struct nlattr *nla,
&attr_size, true, extack);
if (ret)
return ret;
+ ret = tcf_add_notify(net, n, &actions, portid, attr_size, extack);
+ if (ovr)
+ tcf_action_put_lst(&actions);
- return tcf_add_notify(net, n, &actions, portid, attr_size, extack);
+ return ret;
}
static u32 tcaa_root_flags_allowed = TCA_FLAG_LARGE_DUMP_ON;
--
2.7.5
^ permalink raw reply related
* [PATCH v2 00/11] Modify action API for implementing lockless actions
From: Vlad Buslov @ 2018-05-27 16:41 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jiri, ast, daniel, kliteyn,
Vlad Buslov
Currently, all netlink protocol handlers for updating rules, actions and
qdiscs are protected with single global rtnl lock which removes any
possibility for parallelism. This patch set is a first step to remove
rtnl lock dependency from TC rules update path.
Recently, new rtnl registration flag RTNL_FLAG_DOIT_UNLOCKED was added.
Handlers registered with this flag are called without RTNL taken. End
goal is to have rule update handlers(RTM_NEWTFILTER, RTM_DELTFILTER,
etc.) to be registered with UNLOCKED flag to allow parallel execution.
However, there is no intention to completely remove or split rtnl lock
itself. This patch set addresses specific problems in action API that
prevents it from being executed concurrently.
As a preparation for executing TC rules update handlers without rtnl
lock, action API code was audited to determine areas that assume
external synchronization with rtnl lock and must be changed to allow
safe concurrent access with following results:
1. Action idr is already protected with spinlock. However, some code
paths assume that idr state is not changes between several
consecutive tcf_idr_* function calls.
2. tc_action reference and bind counters are implemented as plain
integers. They purpose was to allow single actions to be shared
between multiple filters, not to provide means for concurrent
modification.
3. tc_action 'cookie' pointer field is not protected against
modification.
4. Action API functions, that work with set of actions, use intrusive
linked list, which cannot be used concurrently without additional
synchronization.
5. Action API functions don't take reference to actions while using
them, assuming external synchronization with rtnl lock.
Following solutions to these problems are implemented:
1. To remove assumption that idr state doesn't change between tcf_idr_*
calls, implement new functions that atomically perform several
operations on idr without releasing idr spinlock. (function to
atomically lookup and delete action by index, function to atomically
check if action exists and allocate new one if necessary, etc.)
2. Use atomic operations on counters to make them suitable for
concurrent get/put operations.
3. Data that 'cookie' points to is never modified, so it enough to
refactor it to rcu pointer to prevent concurrent de-allocation.
4. Action API doesn't actually use any linked list specific operations
on actions intrusive linked list, so it can be refactored to array in
straightforward manner.
5. Always take reference to action while accessing it in action API.
tcf_idr_search function modified to take reference to action before
returning it, so there is no way to lookup an action without
incrementing its reference counter. All users of this function are
modified to release the reference, after they done using action. With
all users using reference counting, it is now safe to concurrently
delete actions.
Since only shared state in action API module are actions themselves and
action idr, these changes are sufficient to not to rely on global rtnl
lock for protection of internal action API data structures.
Changes from V1 to V2:
- Removed redundant actions ops lookup during delete.
- Merge action ops delete definition and implementation.
- Assume all actions have delete implemented and don't check for it
explicitly.
- Resplit action lookup/release code to prevent memory leaks in
individual patches.
- Make __tcf_idr_check function static
- Remove unique idr insertion function. Change original idr insert to do
the same thing.
- Merge changes that take reference to action when performing lookup and
changes that account for this additional reference when dumping action
to user space into single patch.
- Change convoluted commit message.
- Rename "unlocked" to "rtnl_held" for clarity.
- Remove estimator lock add patch.
- Refactor action check-alloc code into standalone function.
- Rename tcf_idr_find_delete to tcf_idr_delete_index.
- Rearrange variable definitions in tc_action_delete.
- Add patch that refactors action API code to use array of pointers to
actions instead of intrusive linked list.
- Expand cover letter.
Vlad Buslov (11):
net: sched: use rcu for action cookie update
net: sched: change type of reference and bind counters
net: sched: implement unlocked action init API
net: sched: always take reference to action
net: sched: implement action API that deletes action by index
net: sched: add 'delete' function to action ops
net: sched: implement reference counted action release
net: sched: don't release reference on action overwrite
net: sched: use reference counting action init
net: sched: atomically check-allocate action
net: sched: change action API to use array of pointers to actions
include/net/act_api.h | 25 ++-
include/net/pkt_cls.h | 1 +
net/sched/act_api.c | 408 +++++++++++++++++++++++++++++++--------------
net/sched/act_bpf.c | 34 ++--
net/sched/act_connmark.c | 29 +++-
net/sched/act_csum.c | 34 ++--
net/sched/act_gact.c | 31 +++-
net/sched/act_ife.c | 33 ++--
net/sched/act_ipt.c | 44 ++++-
net/sched/act_mirred.c | 38 +++--
net/sched/act_nat.c | 30 +++-
net/sched/act_pedit.c | 35 +++-
net/sched/act_police.c | 31 ++--
net/sched/act_sample.c | 34 ++--
net/sched/act_simple.c | 31 +++-
net/sched/act_skbedit.c | 31 +++-
net/sched/act_skbmod.c | 34 ++--
net/sched/act_tunnel_key.c | 32 ++--
net/sched/act_vlan.c | 40 +++--
net/sched/cls_api.c | 25 +--
20 files changed, 704 insertions(+), 296 deletions(-)
--
2.7.5
^ permalink raw reply
* [PATCH v2 05/11] net: sched: implement action API that deletes action by index
From: Vlad Buslov @ 2018-05-27 16:41 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jiri, ast, daniel, kliteyn,
Vlad Buslov
In-Reply-To: <1527439298-22306-1-git-send-email-vladbu@mellanox.com>
Implement new action API function that atomically finds and deletes action
from idr by index. Intended to be used by lockless actions that do not rely
on rtnl lock.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
include/net/act_api.h | 1 +
net/sched/act_api.c | 39 +++++++++++++++++++++++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index 888ff471bbf6..d94ec6400673 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -153,6 +153,7 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
int bind, bool cpustats);
void tcf_idr_insert(struct tc_action_net *tn, struct tc_action *a);
+int tcf_idr_delete_index(struct tc_action_net *tn, u32 index);
int __tcf_idr_release(struct tc_action *a, bool bind, bool strict);
static inline int tcf_idr_release(struct tc_action *a, bool bind)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index aa304d36fee0..0f31f09946ab 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -319,6 +319,45 @@ bool tcf_idr_check(struct tc_action_net *tn, u32 index, struct tc_action **a,
}
EXPORT_SYMBOL(tcf_idr_check);
+int tcf_idr_delete_index(struct tc_action_net *tn, u32 index)
+{
+ struct tcf_idrinfo *idrinfo = tn->idrinfo;
+ struct tc_action *p;
+ int ret = 0;
+
+ spin_lock(&idrinfo->lock);
+ p = idr_find(&idrinfo->action_idr, index);
+ if (!p) {
+ spin_unlock(&idrinfo->lock);
+ return -ENOENT;
+ }
+
+ if (!atomic_read(&p->tcfa_bindcnt)) {
+ if (refcount_dec_and_test(&p->tcfa_refcnt)) {
+ struct module *owner = p->ops->owner;
+
+ WARN_ON(p != idr_remove(&idrinfo->action_idr,
+ p->tcfa_index));
+ spin_unlock(&idrinfo->lock);
+
+ if (p->ops->cleanup)
+ p->ops->cleanup(p);
+
+ gen_kill_estimator(&p->tcfa_rate_est);
+ free_tcf(p);
+ module_put(owner);
+ return 0;
+ }
+ ret = 0;
+ } else {
+ ret = -EPERM;
+ }
+
+ spin_unlock(&idrinfo->lock);
+ return ret;
+}
+EXPORT_SYMBOL(tcf_idr_delete_index);
+
int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
struct tc_action **a, const struct tc_action_ops *ops,
int bind, bool cpustats)
--
2.7.5
^ permalink raw reply related
* [PATCH v2 11/11] net: sched: change action API to use array of pointers to actions
From: Vlad Buslov @ 2018-05-27 16:41 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jiri, ast, daniel, kliteyn,
Vlad Buslov
In-Reply-To: <1527439298-22306-1-git-send-email-vladbu@mellanox.com>
Act API used linked list to pass set of actions to functions. It is
intrusive data structure that stores list nodes inside action structure
itself, which means it is not safe to modify such list concurrently.
However, action API doesn't use any linked list specific operations on this
set of actions, so it can be safely refactored into plain pointer array.
Refactor action API to use array of pointers to tc_actions instead of
linked list. Change argument 'actions' type of exported action init,
destroy and dump functions.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
include/net/act_api.h | 7 ++---
net/sched/act_api.c | 74 ++++++++++++++++++++++++++++-----------------------
net/sched/cls_api.c | 21 +++++----------
3 files changed, 50 insertions(+), 52 deletions(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index cd4547476074..43dfa5e1b3b3 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -168,19 +168,20 @@ static inline int tcf_idr_release(struct tc_action *a, bool bind)
int tcf_register_action(struct tc_action_ops *a, struct pernet_operations *ops);
int tcf_unregister_action(struct tc_action_ops *a,
struct pernet_operations *ops);
-int tcf_action_destroy(struct list_head *actions, int bind);
+int tcf_action_destroy(struct tc_action *actions[], int bind);
int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
int nr_actions, struct tcf_result *res);
int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
struct nlattr *est, char *name, int ovr, int bind,
- struct list_head *actions, size_t *attr_size,
+ struct tc_action *actions[], size_t *attr_size,
bool rtnl_held, struct netlink_ext_ack *extack);
struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
struct nlattr *nla, struct nlattr *est,
char *name, int ovr, int bind,
bool rtnl_held,
struct netlink_ext_ack *extack);
-int tcf_action_dump(struct sk_buff *skb, struct list_head *, int, int);
+int tcf_action_dump(struct sk_buff *skb, struct tc_action *actions[], int bind,
+ int ref);
int tcf_action_dump_old(struct sk_buff *skb, struct tc_action *a, int, int);
int tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int, int);
int tcf_action_copy_stats(struct sk_buff *, struct tc_action *, int);
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 9511502e1cbb..7f904bb84aab 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -657,13 +657,14 @@ int tcf_action_exec(struct sk_buff *skb, struct tc_action **actions,
}
EXPORT_SYMBOL(tcf_action_exec);
-int tcf_action_destroy(struct list_head *actions, int bind)
+int tcf_action_destroy(struct tc_action *actions[], int bind)
{
const struct tc_action_ops *ops;
- struct tc_action *a, *tmp;
- int ret = 0;
+ struct tc_action *a;
+ int ret = 0, i;
- list_for_each_entry_safe(a, tmp, actions, list) {
+ for (i = 0; i < TCA_ACT_MAX_PRIO && actions[i]; i++) {
+ a = actions[i];
ops = a->ops;
ret = __tcf_idr_release(a, bind, true);
if (ret == ACT_P_DELETED)
@@ -679,11 +680,12 @@ static int tcf_action_put(struct tc_action *p)
return __tcf_action_put(p, false);
}
-static void tcf_action_put_lst(struct list_head *actions)
+static void tcf_action_put_many(struct tc_action *actions[])
{
- struct tc_action *a, *tmp;
+ int i;
- list_for_each_entry_safe(a, tmp, actions, list) {
+ for (i = 0; i < TCA_ACT_MAX_PRIO && actions[i]; i++) {
+ struct tc_action *a = actions[i];
const struct tc_action_ops *ops = a->ops;
if (tcf_action_put(a))
@@ -735,14 +737,15 @@ tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
}
EXPORT_SYMBOL(tcf_action_dump_1);
-int tcf_action_dump(struct sk_buff *skb, struct list_head *actions,
+int tcf_action_dump(struct sk_buff *skb, struct tc_action *actions[],
int bind, int ref)
{
struct tc_action *a;
- int err = -EINVAL;
+ int err = -EINVAL, i;
struct nlattr *nest;
- list_for_each_entry(a, actions, list) {
+ for (i = 0; i < TCA_ACT_MAX_PRIO && actions[i]; i++) {
+ a = actions[i];
nest = nla_nest_start(skb, a->order);
if (nest == NULL)
goto nla_put_failure;
@@ -878,10 +881,9 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
if (TC_ACT_EXT_CMP(a->tcfa_action, TC_ACT_GOTO_CHAIN)) {
err = tcf_action_goto_chain_init(a, tp);
if (err) {
- LIST_HEAD(actions);
+ struct tc_action *actions[TCA_ACT_MAX_PRIO] = { a };
- list_add_tail(&a->list, &actions);
- tcf_action_destroy(&actions, bind);
+ tcf_action_destroy(actions, bind);
NL_SET_ERR_MSG(extack, "Failed to init TC action chain");
return ERR_PTR(err);
}
@@ -899,9 +901,11 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
return ERR_PTR(err);
}
+/* Returns numbers of initialized actions or negative error. */
+
int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
struct nlattr *est, char *name, int ovr, int bind,
- struct list_head *actions, size_t *attr_size,
+ struct tc_action *actions[], size_t *attr_size,
bool rtnl_held, struct netlink_ext_ack *extack)
{
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
@@ -923,11 +927,12 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
}
act->order = i;
sz += tcf_action_fill_size(act);
- list_add_tail(&act->list, actions);
+ /* Start from index 0 */
+ actions[i - 1] = act;
}
*attr_size = tcf_action_full_attrs_size(sz);
- return 0;
+ return i - 1;
err:
tcf_action_destroy(actions, bind);
@@ -978,7 +983,7 @@ int tcf_action_copy_stats(struct sk_buff *skb, struct tc_action *p,
return -1;
}
-static int tca_get_fill(struct sk_buff *skb, struct list_head *actions,
+static int tca_get_fill(struct sk_buff *skb, struct tc_action *actions[],
u32 portid, u32 seq, u16 flags, int event, int bind,
int ref)
{
@@ -1014,7 +1019,7 @@ static int tca_get_fill(struct sk_buff *skb, struct list_head *actions,
static int
tcf_get_notify(struct net *net, u32 portid, struct nlmsghdr *n,
- struct list_head *actions, int event,
+ struct tc_action *actions[], int event,
struct netlink_ext_ack *extack)
{
struct sk_buff *skb;
@@ -1150,14 +1155,14 @@ static int tca_action_flush(struct net *net, struct nlattr *nla,
return err;
}
-static int tcf_action_delete(struct net *net, struct list_head *actions,
+static int tcf_action_delete(struct net *net, struct tc_action *actions[],
struct netlink_ext_ack *extack)
{
- struct tc_action *a, *tmp;
u32 act_index;
- int ret;
+ int ret, i;
- list_for_each_entry_safe(a, tmp, actions, list) {
+ for (i = 0; i < TCA_ACT_MAX_PRIO && actions[i]; i++) {
+ struct tc_action *a = actions[i];
const struct tc_action_ops *ops = a->ops;
/* Actions can be deleted concurrently so we must save their
@@ -1180,7 +1185,7 @@ static int tcf_action_delete(struct net *net, struct list_head *actions,
}
static int
-tcf_del_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
+tcf_del_notify(struct net *net, struct nlmsghdr *n, struct tc_action *actions[],
u32 portid, size_t attr_size, struct netlink_ext_ack *extack)
{
int ret;
@@ -1221,7 +1226,7 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
struct tc_action *act;
size_t attr_size = 0;
- LIST_HEAD(actions);
+ struct tc_action *actions[TCA_ACT_MAX_PRIO] = {};
ret = nla_parse_nested(tb, TCA_ACT_MAX_PRIO, nla, NULL, extack);
if (ret < 0)
@@ -1243,26 +1248,27 @@ tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
}
act->order = i;
attr_size += tcf_action_fill_size(act);
- list_add_tail(&act->list, &actions);
+ actions[i - 1] = act;
}
attr_size = tcf_action_full_attrs_size(attr_size);
if (event == RTM_GETACTION)
- ret = tcf_get_notify(net, portid, n, &actions, event, extack);
+ ret = tcf_get_notify(net, portid, n, actions, event, extack);
else { /* delete */
- ret = tcf_del_notify(net, n, &actions, portid, attr_size, extack);
+ ret = tcf_del_notify(net, n, actions, portid, attr_size,
+ extack);
if (ret)
goto err;
return ret;
}
err:
- tcf_action_put_lst(&actions);
+ tcf_action_put_many(actions);
return ret;
}
static int
-tcf_add_notify(struct net *net, struct nlmsghdr *n, struct list_head *actions,
+tcf_add_notify(struct net *net, struct nlmsghdr *n, struct tc_action *actions[],
u32 portid, size_t attr_size, struct netlink_ext_ack *extack)
{
struct sk_buff *skb;
@@ -1293,15 +1299,15 @@ static int tcf_action_add(struct net *net, struct nlattr *nla,
{
size_t attr_size = 0;
int ret = 0;
- LIST_HEAD(actions);
+ struct tc_action *actions[TCA_ACT_MAX_PRIO] = {};
- ret = tcf_action_init(net, NULL, nla, NULL, NULL, ovr, 0, &actions,
+ ret = tcf_action_init(net, NULL, nla, NULL, NULL, ovr, 0, actions,
&attr_size, true, extack);
- if (ret)
+ if (ret < 0)
return ret;
- ret = tcf_add_notify(net, n, &actions, portid, attr_size, extack);
+ ret = tcf_add_notify(net, n, actions, portid, attr_size, extack);
if (ovr)
- tcf_action_put_lst(&actions);
+ tcf_action_put_many(actions);
return ret;
}
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index dd37d0ae3fce..b2bb6b7b038a 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1415,10 +1415,7 @@ static int tc_dump_tfilter(struct sk_buff *skb, struct netlink_callback *cb)
void tcf_exts_destroy(struct tcf_exts *exts)
{
#ifdef CONFIG_NET_CLS_ACT
- LIST_HEAD(actions);
-
- tcf_exts_to_list(exts, &actions);
- tcf_action_destroy(&actions, TCA_ACT_UNBIND);
+ tcf_action_destroy(exts->actions, TCA_ACT_UNBIND);
kfree(exts->actions);
exts->nr_actions = 0;
#endif
@@ -1445,18 +1442,15 @@ int tcf_exts_validate(struct net *net, struct tcf_proto *tp, struct nlattr **tb,
exts->actions[0] = act;
exts->nr_actions = 1;
} else if (exts->action && tb[exts->action]) {
- LIST_HEAD(actions);
- int err, i = 0;
+ int err;
err = tcf_action_init(net, tp, tb[exts->action],
rate_tlv, NULL, ovr, TCA_ACT_BIND,
- &actions, &attr_size, true,
+ exts->actions, &attr_size, true,
extack);
- if (err)
+ if (err < 0)
return err;
- list_for_each_entry(act, &actions, list)
- exts->actions[i++] = act;
- exts->nr_actions = i;
+ exts->nr_actions = err;
}
exts->net = net;
}
@@ -1505,14 +1499,11 @@ int tcf_exts_dump(struct sk_buff *skb, struct tcf_exts *exts)
* tc data even if iproute2 was newer - jhs
*/
if (exts->type != TCA_OLD_COMPAT) {
- LIST_HEAD(actions);
-
nest = nla_nest_start(skb, exts->action);
if (nest == NULL)
goto nla_put_failure;
- tcf_exts_to_list(exts, &actions);
- if (tcf_action_dump(skb, &actions, 0, 0) < 0)
+ if (tcf_action_dump(skb, exts->actions, 0, 0) < 0)
goto nla_put_failure;
nla_nest_end(skb, nest);
} else if (exts->police) {
--
2.7.5
^ permalink raw reply related
* [PATCH v2 08/11] net: sched: don't release reference on action overwrite
From: Vlad Buslov @ 2018-05-27 16:41 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jiri, ast, daniel, kliteyn,
Vlad Buslov
In-Reply-To: <1527439298-22306-1-git-send-email-vladbu@mellanox.com>
Return from action init function with reference to action taken,
even when overwriting existing action.
Action init API initializes its fourth argument (pointer to pointer to tc
action) to either existing action with same index or newly created action.
In case of existing index(and bind argument is zero), init function returns
without incrementing action reference counter. Caller of action init then
proceeds working with action, without actually holding reference to it.
This means that action could be deleted concurrently.
Change action init behavior to always take reference to action before
returning successfully, in order to protect from concurrent deletion.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
net/sched/act_api.c | 2 --
net/sched/act_bpf.c | 8 ++++----
net/sched/act_connmark.c | 5 +++--
net/sched/act_csum.c | 8 ++++----
net/sched/act_gact.c | 5 +++--
net/sched/act_ife.c | 12 +++++-------
net/sched/act_ipt.c | 5 +++--
net/sched/act_mirred.c | 5 ++---
net/sched/act_nat.c | 5 +++--
net/sched/act_pedit.c | 5 +++--
net/sched/act_police.c | 8 +++-----
net/sched/act_sample.c | 8 +++-----
net/sched/act_simple.c | 5 +++--
net/sched/act_skbedit.c | 5 +++--
net/sched/act_skbmod.c | 8 +++-----
net/sched/act_tunnel_key.c | 8 +++-----
net/sched/act_vlan.c | 8 +++-----
17 files changed, 51 insertions(+), 59 deletions(-)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index a023873db713..f019f0464cec 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -870,8 +870,6 @@ int tcf_action_init(struct net *net, struct tcf_proto *tp, struct nlattr *nla,
}
act->order = i;
sz += tcf_action_fill_size(act);
- if (ovr)
- refcount_inc(&act->tcfa_refcnt);
list_add_tail(&act->list, actions);
}
diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index 7941dd66ff83..d3f4ac6f2c4b 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c
@@ -311,9 +311,10 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
if (bind)
return 0;
- tcf_idr_release(*act, bind);
- if (!replace)
+ if (!replace) {
+ tcf_idr_release(*act, bind);
return -EEXIST;
+ }
}
is_bpf = tb[TCA_ACT_BPF_OPS_LEN] && tb[TCA_ACT_BPF_OPS];
@@ -356,8 +357,7 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
return res;
out:
- if (res == ACT_P_CREATED)
- tcf_idr_release(*act, bind);
+ tcf_idr_release(*act, bind);
return ret;
}
diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
index 143c2d3de723..701e90244eff 100644
--- a/net/sched/act_connmark.c
+++ b/net/sched/act_connmark.c
@@ -135,9 +135,10 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla,
ci = to_connmark(*a);
if (bind)
return 0;
- tcf_idr_release(*a, bind);
- if (!ovr)
+ if (!ovr) {
+ tcf_idr_release(*a, bind);
return -EEXIST;
+ }
/* replacing action and zone */
ci->tcf_action = parm->action;
ci->zone = parm->zone;
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 3768539340e0..5dbee136b0a1 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -76,9 +76,10 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla,
} else {
if (bind)/* dont override defaults */
return 0;
- tcf_idr_release(*a, bind);
- if (!ovr)
+ if (!ovr) {
+ tcf_idr_release(*a, bind);
return -EEXIST;
+ }
}
p = to_tcf_csum(*a);
@@ -86,8 +87,7 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla,
params_new = kzalloc(sizeof(*params_new), GFP_KERNEL);
if (unlikely(!params_new)) {
- if (ret == ACT_P_CREATED)
- tcf_idr_release(*a, bind);
+ tcf_idr_release(*a, bind);
return -ENOMEM;
}
params_old = rtnl_dereference(p->params);
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index a431a711f0dd..11c4de3f344e 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -100,9 +100,10 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
} else {
if (bind)/* dont override defaults */
return 0;
- tcf_idr_release(*a, bind);
- if (!ovr)
+ if (!ovr) {
+ tcf_idr_release(*a, bind);
return -EEXIST;
+ }
}
gact = to_gact(*a);
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 027c305dcb37..3dd3d79c5a4b 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -497,12 +497,10 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
return ret;
}
ret = ACT_P_CREATED;
- } else {
+ } else if (!ovr) {
tcf_idr_release(*a, bind);
- if (!ovr) {
- kfree(p);
- return -EEXIST;
- }
+ kfree(p);
+ return -EEXIST;
}
ife = to_ife(*a);
@@ -544,13 +542,13 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
NULL, NULL);
if (err) {
metadata_parse_err:
- if (exists)
- tcf_idr_release(*a, bind);
if (ret == ACT_P_CREATED)
_tcf_ife_cleanup(*a);
if (exists)
spin_unlock_bh(&ife->tcf_lock);
+ tcf_idr_release(*a, bind);
+
kfree(p);
return err;
}
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index 6c234411c771..85e85dfba401 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -145,10 +145,11 @@ static int __tcf_ipt_init(struct net *net, unsigned int id, struct nlattr *nla,
} else {
if (bind)/* dont override defaults */
return 0;
- tcf_idr_release(*a, bind);
- if (!ovr)
+ if (!ovr) {
+ tcf_idr_release(*a, bind);
return -EEXIST;
+ }
}
hook = nla_get_u32(tb[TCA_IPT_HOOK]);
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index 3d8300bce7e4..e08aed06d7f8 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -132,10 +132,9 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
if (ret)
return ret;
ret = ACT_P_CREATED;
- } else {
+ } else if (!ovr) {
tcf_idr_release(*a, bind);
- if (!ovr)
- return -EEXIST;
+ return -EEXIST;
}
m = to_mirred(*a);
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index 9eb27c89dc46..1f91e8e66c0f 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -66,9 +66,10 @@ static int tcf_nat_init(struct net *net, struct nlattr *nla, struct nlattr *est,
} else {
if (bind)
return 0;
- tcf_idr_release(*a, bind);
- if (!ovr)
+ if (!ovr) {
+ tcf_idr_release(*a, bind);
return -EEXIST;
+ }
}
p = to_tcf_nat(*a);
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index b8857035e3f8..fbf283f2ac34 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -185,9 +185,10 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
} else {
if (bind)
return 0;
- tcf_idr_release(*a, bind);
- if (!ovr)
+ if (!ovr) {
+ tcf_idr_release(*a, bind);
return -EEXIST;
+ }
p = to_pedit(*a);
if (p->tcfp_nkeys && p->tcfp_nkeys != parm->nkeys) {
keys = kmalloc(ksize, GFP_KERNEL);
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index c955fb0d4f3f..99335cca739e 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -111,10 +111,9 @@ static int tcf_act_police_init(struct net *net, struct nlattr *nla,
if (ret)
return ret;
ret = ACT_P_CREATED;
- } else {
+ } else if (!ovr) {
tcf_idr_release(*a, bind);
- if (!ovr)
- return -EEXIST;
+ return -EEXIST;
}
police = to_police(*a);
@@ -195,8 +194,7 @@ static int tcf_act_police_init(struct net *net, struct nlattr *nla,
failure:
qdisc_put_rtab(P_tab);
qdisc_put_rtab(R_tab);
- if (ret == ACT_P_CREATED)
- tcf_idr_release(*a, bind);
+ tcf_idr_release(*a, bind);
return err;
}
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
index 6f79d2afcba2..a8582e1347db 100644
--- a/net/sched/act_sample.c
+++ b/net/sched/act_sample.c
@@ -69,10 +69,9 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
if (ret)
return ret;
ret = ACT_P_CREATED;
- } else {
+ } else if (!ovr) {
tcf_idr_release(*a, bind);
- if (!ovr)
- return -EEXIST;
+ return -EEXIST;
}
s = to_sample(*a);
@@ -81,8 +80,7 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
s->psample_group_num = nla_get_u32(tb[TCA_SAMPLE_PSAMPLE_GROUP]);
psample_group = psample_group_get(net, s->psample_group_num);
if (!psample_group) {
- if (ret == ACT_P_CREATED)
- tcf_idr_release(*a, bind);
+ tcf_idr_release(*a, bind);
return -ENOMEM;
}
RCU_INIT_POINTER(s->psample_group, psample_group);
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index b570e7ca7e33..78fffd329ed9 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -130,9 +130,10 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
} else {
d = to_defact(*a);
- tcf_idr_release(*a, bind);
- if (!ovr)
+ if (!ovr) {
+ tcf_idr_release(*a, bind);
return -EEXIST;
+ }
reset_policy(d, defdata, parm);
}
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index dc0cb350aa45..c0607d1319eb 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -137,9 +137,10 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
ret = ACT_P_CREATED;
} else {
d = to_skbedit(*a);
- tcf_idr_release(*a, bind);
- if (!ovr)
+ if (!ovr) {
+ tcf_idr_release(*a, bind);
return -EEXIST;
+ }
}
spin_lock_bh(&d->tcf_lock);
diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
index 30be3f767495..e844381af066 100644
--- a/net/sched/act_skbmod.c
+++ b/net/sched/act_skbmod.c
@@ -145,10 +145,9 @@ static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
return ret;
ret = ACT_P_CREATED;
- } else {
+ } else if (!ovr) {
tcf_idr_release(*a, bind);
- if (!ovr)
- return -EEXIST;
+ return -EEXIST;
}
d = to_skbmod(*a);
@@ -156,8 +155,7 @@ static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
ASSERT_RTNL();
p = kzalloc(sizeof(struct tcf_skbmod_params), GFP_KERNEL);
if (unlikely(!p)) {
- if (ret == ACT_P_CREATED)
- tcf_idr_release(*a, bind);
+ tcf_idr_release(*a, bind);
return -ENOMEM;
}
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index 4b7f9a3b47d7..bd53f39a345b 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -165,10 +165,9 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
return ret;
ret = ACT_P_CREATED;
- } else {
+ } else if (!ovr) {
tcf_idr_release(*a, bind);
- if (!ovr)
- return -EEXIST;
+ return -EEXIST;
}
t = to_tunnel_key(*a);
@@ -176,8 +175,7 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
ASSERT_RTNL();
params_new = kzalloc(sizeof(*params_new), GFP_KERNEL);
if (unlikely(!params_new)) {
- if (ret == ACT_P_CREATED)
- tcf_idr_release(*a, bind);
+ tcf_idr_release(*a, bind);
return -ENOMEM;
}
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index b44377c951b6..4ac0d565e437 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -185,10 +185,9 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
return ret;
ret = ACT_P_CREATED;
- } else {
+ } else if (!ovr) {
tcf_idr_release(*a, bind);
- if (!ovr)
- return -EEXIST;
+ return -EEXIST;
}
v = to_vlan(*a);
@@ -196,8 +195,7 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
ASSERT_RTNL();
p = kzalloc(sizeof(*p), GFP_KERNEL);
if (!p) {
- if (ret == ACT_P_CREATED)
- tcf_idr_release(*a, bind);
+ tcf_idr_release(*a, bind);
return -ENOMEM;
}
--
2.7.5
^ permalink raw reply related
* [PATCH v2 01/11] net: sched: use rcu for action cookie update
From: Vlad Buslov @ 2018-05-27 16:41 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jiri, ast, daniel, kliteyn,
Vlad Buslov
In-Reply-To: <1527439298-22306-1-git-send-email-vladbu@mellanox.com>
Implement functions to atomically update and free action cookie
using rcu mechanism.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Signed-off-by: Jiri Pirko <jiri@mellanox.com>
---
include/net/act_api.h | 2 +-
include/net/pkt_cls.h | 1 +
net/sched/act_api.c | 44 ++++++++++++++++++++++++++++++--------------
3 files changed, 32 insertions(+), 15 deletions(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index 9e59ebfded62..f7b59ef7303d 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -37,7 +37,7 @@ struct tc_action {
spinlock_t tcfa_lock;
struct gnet_stats_basic_cpu __percpu *cpu_bstats;
struct gnet_stats_queue __percpu *cpu_qstats;
- struct tc_cookie *act_cookie;
+ struct tc_cookie __rcu *act_cookie;
struct tcf_chain *goto_chain;
};
#define tcf_index common.tcfa_index
diff --git a/include/net/pkt_cls.h b/include/net/pkt_cls.h
index e828d31be5da..3068cc8aa0f1 100644
--- a/include/net/pkt_cls.h
+++ b/include/net/pkt_cls.h
@@ -769,6 +769,7 @@ struct tc_mqprio_qopt_offload {
struct tc_cookie {
u8 *data;
u32 len;
+ struct rcu_head rcu;
};
struct tc_qopt_offload_stats {
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 3f4cf930f809..02670c7489e3 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -55,6 +55,24 @@ static void tcf_action_goto_chain_exec(const struct tc_action *a,
res->goto_tp = rcu_dereference_bh(chain->filter_chain);
}
+static void tcf_free_cookie_rcu(struct rcu_head *p)
+{
+ struct tc_cookie *cookie = container_of(p, struct tc_cookie, rcu);
+
+ kfree(cookie->data);
+ kfree(cookie);
+}
+
+static void tcf_set_action_cookie(struct tc_cookie __rcu **old_cookie,
+ struct tc_cookie *new_cookie)
+{
+ struct tc_cookie *old;
+
+ old = xchg(old_cookie, new_cookie);
+ if (old)
+ call_rcu(&old->rcu, tcf_free_cookie_rcu);
+}
+
/* XXX: For standalone actions, we don't need a RCU grace period either, because
* actions are always connected to filters and filters are already destroyed in
* RCU callbacks, so after a RCU grace period actions are already disconnected
@@ -65,10 +83,7 @@ static void free_tcf(struct tc_action *p)
free_percpu(p->cpu_bstats);
free_percpu(p->cpu_qstats);
- if (p->act_cookie) {
- kfree(p->act_cookie->data);
- kfree(p->act_cookie);
- }
+ tcf_set_action_cookie(&p->act_cookie, NULL);
if (p->goto_chain)
tcf_action_goto_chain_fini(p);
@@ -567,16 +582,22 @@ tcf_action_dump_1(struct sk_buff *skb, struct tc_action *a, int bind, int ref)
int err = -EINVAL;
unsigned char *b = skb_tail_pointer(skb);
struct nlattr *nest;
+ struct tc_cookie *cookie;
if (nla_put_string(skb, TCA_KIND, a->ops->kind))
goto nla_put_failure;
if (tcf_action_copy_stats(skb, a, 0))
goto nla_put_failure;
- if (a->act_cookie) {
- if (nla_put(skb, TCA_ACT_COOKIE, a->act_cookie->len,
- a->act_cookie->data))
+
+ rcu_read_lock();
+ cookie = rcu_dereference(a->act_cookie);
+ if (cookie) {
+ if (nla_put(skb, TCA_ACT_COOKIE, cookie->len, cookie->data)) {
+ rcu_read_unlock();
goto nla_put_failure;
+ }
}
+ rcu_read_unlock();
nest = nla_nest_start(skb, TCA_OPTIONS);
if (nest == NULL)
@@ -719,13 +740,8 @@ struct tc_action *tcf_action_init_1(struct net *net, struct tcf_proto *tp,
if (err < 0)
goto err_mod;
- if (name == NULL && tb[TCA_ACT_COOKIE]) {
- if (a->act_cookie) {
- kfree(a->act_cookie->data);
- kfree(a->act_cookie);
- }
- a->act_cookie = cookie;
- }
+ if (!name && tb[TCA_ACT_COOKIE])
+ tcf_set_action_cookie(&a->act_cookie, cookie);
/* module count goes up only when brand new policy is created
* if it exists and is only bound to in a_o->init() then
--
2.7.5
^ permalink raw reply related
* [PATCH v2 10/11] net: sched: atomically check-allocate action
From: Vlad Buslov @ 2018-05-27 16:41 UTC (permalink / raw)
To: netdev; +Cc: davem, jhs, xiyou.wangcong, jiri, ast, daniel, kliteyn,
Vlad Buslov
In-Reply-To: <1527439298-22306-1-git-send-email-vladbu@mellanox.com>
Implement function that atomically checks if action exists and either takes
reference to it, or allocates idr slot for action index to prevent
concurrent allocations of actions with same index. Use EBUSY error pointer
to indicate that idr slot is reserved.
Implement cleanup helper function that removes temporary error pointer from
idr. (in case of error between idr allocation and insertion of newly
created action to specified index)
Refactor all action init functions to insert new action to idr using this
API.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
---
include/net/act_api.h | 3 ++
net/sched/act_api.c | 92 ++++++++++++++++++++++++++++++++++++----------
net/sched/act_bpf.c | 11 ++++--
net/sched/act_connmark.c | 10 +++--
net/sched/act_csum.c | 11 ++++--
net/sched/act_gact.c | 11 ++++--
net/sched/act_ife.c | 6 ++-
net/sched/act_ipt.c | 13 ++++++-
net/sched/act_mirred.c | 16 ++++++--
net/sched/act_nat.c | 11 ++++--
net/sched/act_pedit.c | 15 ++++++--
net/sched/act_police.c | 9 ++++-
net/sched/act_sample.c | 11 ++++--
net/sched/act_simple.c | 11 +++++-
net/sched/act_skbedit.c | 11 +++++-
net/sched/act_skbmod.c | 11 +++++-
net/sched/act_tunnel_key.c | 9 ++++-
net/sched/act_vlan.c | 17 ++++++++-
18 files changed, 218 insertions(+), 60 deletions(-)
diff --git a/include/net/act_api.h b/include/net/act_api.h
index d256e20507b9..cd4547476074 100644
--- a/include/net/act_api.h
+++ b/include/net/act_api.h
@@ -154,6 +154,9 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
int bind, bool cpustats);
void tcf_idr_insert(struct tc_action_net *tn, struct tc_action *a);
+void tcf_idr_cleanup(struct tc_action_net *tn, u32 index);
+int tcf_idr_check_alloc(struct tc_action_net *tn, u32 *index,
+ struct tc_action **a, int bind);
int tcf_idr_delete_index(struct tc_action_net *tn, u32 index);
int __tcf_idr_release(struct tc_action *a, bool bind, bool strict);
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index eefe8c2fe667..9511502e1cbb 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -303,7 +303,9 @@ static bool __tcf_idr_check(struct tc_action_net *tn, u32 index,
spin_lock(&idrinfo->lock);
p = idr_find(&idrinfo->action_idr, index);
- if (p) {
+ if (IS_ERR(p)) {
+ p = NULL;
+ } else if (p) {
refcount_inc(&p->tcfa_refcnt);
if (bind)
atomic_inc(&p->tcfa_bindcnt);
@@ -371,7 +373,6 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
{
struct tc_action *p = kzalloc(ops->size, GFP_KERNEL);
struct tcf_idrinfo *idrinfo = tn->idrinfo;
- struct idr *idr = &idrinfo->action_idr;
int err = -ENOMEM;
if (unlikely(!p))
@@ -389,20 +390,6 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
goto err2;
}
spin_lock_init(&p->tcfa_lock);
- idr_preload(GFP_KERNEL);
- spin_lock(&idrinfo->lock);
- /* user doesn't specify an index */
- if (!index) {
- index = 1;
- err = idr_alloc_u32(idr, NULL, &index, UINT_MAX, GFP_ATOMIC);
- } else {
- err = idr_alloc_u32(idr, NULL, &index, index, GFP_ATOMIC);
- }
- spin_unlock(&idrinfo->lock);
- idr_preload_end();
- if (err)
- goto err3;
-
p->tcfa_index = index;
p->tcfa_tm.install = jiffies;
p->tcfa_tm.lastuse = jiffies;
@@ -412,7 +399,7 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
&p->tcfa_rate_est,
&p->tcfa_lock, NULL, est);
if (err)
- goto err4;
+ goto err3;
}
p->idrinfo = idrinfo;
@@ -420,8 +407,6 @@ int tcf_idr_create(struct tc_action_net *tn, u32 index, struct nlattr *est,
INIT_LIST_HEAD(&p->list);
*a = p;
return 0;
-err4:
- idr_remove(idr, index);
err3:
free_percpu(p->cpu_qstats);
err2:
@@ -437,11 +422,78 @@ void tcf_idr_insert(struct tc_action_net *tn, struct tc_action *a)
struct tcf_idrinfo *idrinfo = tn->idrinfo;
spin_lock(&idrinfo->lock);
- idr_replace(&idrinfo->action_idr, a, a->tcfa_index);
+ /* Replace ERR_PTR(-EBUSY) allocated by tcf_idr_check_alloc */
+ WARN_ON(!IS_ERR(idr_replace(&idrinfo->action_idr, a, a->tcfa_index)));
spin_unlock(&idrinfo->lock);
}
EXPORT_SYMBOL(tcf_idr_insert);
+/* Cleanup idr index that was allocated but not initialized. */
+
+void tcf_idr_cleanup(struct tc_action_net *tn, u32 index)
+{
+ struct tcf_idrinfo *idrinfo = tn->idrinfo;
+
+ spin_lock(&idrinfo->lock);
+ /* Remove ERR_PTR(-EBUSY) allocated by tcf_idr_check_alloc */
+ WARN_ON(!IS_ERR(idr_remove(&idrinfo->action_idr, index)));
+ spin_unlock(&idrinfo->lock);
+}
+EXPORT_SYMBOL(tcf_idr_cleanup);
+
+/* Check if action with specified index exists. If actions is found, increments
+ * its reference and bind counters, and return 1. Otherwise insert temporary
+ * error pointer (to prevent concurrent users from inserting actions with same
+ * index) and return 0.
+ */
+
+int tcf_idr_check_alloc(struct tc_action_net *tn, u32 *index,
+ struct tc_action **a, int bind)
+{
+ struct tcf_idrinfo *idrinfo = tn->idrinfo;
+ struct tc_action *p;
+ int ret;
+
+again:
+ spin_lock(&idrinfo->lock);
+ if (*index) {
+ p = idr_find(&idrinfo->action_idr, *index);
+ if (IS_ERR(p)) {
+ /* This means that another process allocated
+ * index but did not assign the pointer yet.
+ */
+ spin_unlock(&idrinfo->lock);
+ goto again;
+ }
+
+ if (p) {
+ refcount_inc(&p->tcfa_refcnt);
+ if (bind)
+ atomic_inc(&p->tcfa_bindcnt);
+ *a = p;
+ ret = 1;
+ } else {
+ *a = NULL;
+ ret = idr_alloc_u32(&idrinfo->action_idr, NULL, index,
+ *index, GFP_ATOMIC);
+ if (!ret)
+ idr_replace(&idrinfo->action_idr,
+ ERR_PTR(-EBUSY), *index);
+ }
+ } else {
+ *index = 1;
+ *a = NULL;
+ ret = idr_alloc_u32(&idrinfo->action_idr, NULL, index,
+ UINT_MAX, GFP_ATOMIC);
+ if (!ret)
+ idr_replace(&idrinfo->action_idr, ERR_PTR(-EBUSY),
+ *index);
+ }
+ spin_unlock(&idrinfo->lock);
+ return ret;
+}
+EXPORT_SYMBOL(tcf_idr_check_alloc);
+
void tcf_idrinfo_destroy(const struct tc_action_ops *ops,
struct tcf_idrinfo *idrinfo)
{
diff --git a/net/sched/act_bpf.c b/net/sched/act_bpf.c
index d3f4ac6f2c4b..06f743d8ed41 100644
--- a/net/sched/act_bpf.c
+++ b/net/sched/act_bpf.c
@@ -299,14 +299,17 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
parm = nla_data(tb[TCA_ACT_BPF_PARMS]);
- if (!tcf_idr_check(tn, parm->index, act, bind)) {
+ ret = tcf_idr_check_alloc(tn, &parm->index, act, bind);
+ if (!ret) {
ret = tcf_idr_create(tn, parm->index, est, act,
&act_bpf_ops, bind, true);
- if (ret < 0)
+ if (ret < 0) {
+ tcf_idr_cleanup(tn, parm->index);
return ret;
+ }
res = ACT_P_CREATED;
- } else {
+ } else if (ret > 0) {
/* Don't override defaults. */
if (bind)
return 0;
@@ -315,6 +318,8 @@ static int tcf_bpf_init(struct net *net, struct nlattr *nla,
tcf_idr_release(*act, bind);
return -EEXIST;
}
+ } else {
+ return ret;
}
is_bpf = tb[TCA_ACT_BPF_OPS_LEN] && tb[TCA_ACT_BPF_OPS];
diff --git a/net/sched/act_connmark.c b/net/sched/act_connmark.c
index 701e90244eff..1e31f0e448e2 100644
--- a/net/sched/act_connmark.c
+++ b/net/sched/act_connmark.c
@@ -118,11 +118,14 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla,
parm = nla_data(tb[TCA_CONNMARK_PARMS]);
- if (!tcf_idr_check(tn, parm->index, a, bind)) {
+ ret = tcf_idr_check_alloc(tn, &parm->index, a, bind);
+ if (!ret) {
ret = tcf_idr_create(tn, parm->index, est, a,
&act_connmark_ops, bind, false);
- if (ret)
+ if (ret) {
+ tcf_idr_cleanup(tn, parm->index);
return ret;
+ }
ci = to_connmark(*a);
ci->tcf_action = parm->action;
@@ -131,7 +134,7 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla,
tcf_idr_insert(tn, *a);
ret = ACT_P_CREATED;
- } else {
+ } else if (ret > 0) {
ci = to_connmark(*a);
if (bind)
return 0;
@@ -142,6 +145,7 @@ static int tcf_connmark_init(struct net *net, struct nlattr *nla,
/* replacing action and zone */
ci->tcf_action = parm->action;
ci->zone = parm->zone;
+ ret = 0;
}
return ret;
diff --git a/net/sched/act_csum.c b/net/sched/act_csum.c
index 5dbee136b0a1..bd232d3bd022 100644
--- a/net/sched/act_csum.c
+++ b/net/sched/act_csum.c
@@ -67,19 +67,24 @@ static int tcf_csum_init(struct net *net, struct nlattr *nla,
return -EINVAL;
parm = nla_data(tb[TCA_CSUM_PARMS]);
- if (!tcf_idr_check(tn, parm->index, a, bind)) {
+ err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
+ if (!err) {
ret = tcf_idr_create(tn, parm->index, est, a,
&act_csum_ops, bind, true);
- if (ret)
+ if (ret) {
+ tcf_idr_cleanup(tn, parm->index);
return ret;
+ }
ret = ACT_P_CREATED;
- } else {
+ } else if (err > 0) {
if (bind)/* dont override defaults */
return 0;
if (!ovr) {
tcf_idr_release(*a, bind);
return -EEXIST;
}
+ } else {
+ return err;
}
p = to_tcf_csum(*a);
diff --git a/net/sched/act_gact.c b/net/sched/act_gact.c
index 11c4de3f344e..661b72b9147d 100644
--- a/net/sched/act_gact.c
+++ b/net/sched/act_gact.c
@@ -91,19 +91,24 @@ static int tcf_gact_init(struct net *net, struct nlattr *nla,
}
#endif
- if (!tcf_idr_check(tn, parm->index, a, bind)) {
+ err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
+ if (!err) {
ret = tcf_idr_create(tn, parm->index, est, a,
&act_gact_ops, bind, true);
- if (ret)
+ if (ret) {
+ tcf_idr_cleanup(tn, parm->index);
return ret;
+ }
ret = ACT_P_CREATED;
- } else {
+ } else if (err > 0) {
if (bind)/* dont override defaults */
return 0;
if (!ovr) {
tcf_idr_release(*a, bind);
return -EEXIST;
}
+ } else {
+ return err;
}
gact = to_gact(*a);
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 3dd3d79c5a4b..5bf0e79796c0 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -483,7 +483,10 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
if (!p)
return -ENOMEM;
- exists = tcf_idr_check(tn, parm->index, a, bind);
+ err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
+ if (err < 0)
+ return err;
+ exists = err;
if (exists && bind) {
kfree(p);
return 0;
@@ -493,6 +496,7 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
ret = tcf_idr_create(tn, parm->index, est, a, &act_ife_ops,
bind, true);
if (ret) {
+ tcf_idr_cleanup(tn, parm->index);
kfree(p);
return ret;
}
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index 85e85dfba401..0dc787a57798 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -119,13 +119,18 @@ static int __tcf_ipt_init(struct net *net, unsigned int id, struct nlattr *nla,
if (tb[TCA_IPT_INDEX] != NULL)
index = nla_get_u32(tb[TCA_IPT_INDEX]);
- exists = tcf_idr_check(tn, index, a, bind);
+ err = tcf_idr_check_alloc(tn, &index, a, bind);
+ if (err < 0)
+ return err;
+ exists = err;
if (exists && bind)
return 0;
if (tb[TCA_IPT_HOOK] == NULL || tb[TCA_IPT_TARG] == NULL) {
if (exists)
tcf_idr_release(*a, bind);
+ else
+ tcf_idr_cleanup(tn, index);
return -EINVAL;
}
@@ -133,14 +138,18 @@ static int __tcf_ipt_init(struct net *net, unsigned int id, struct nlattr *nla,
if (nla_len(tb[TCA_IPT_TARG]) < td->u.target_size) {
if (exists)
tcf_idr_release(*a, bind);
+ else
+ tcf_idr_cleanup(tn, index);
return -EINVAL;
}
if (!exists) {
ret = tcf_idr_create(tn, index, est, a, ops, bind,
false);
- if (ret)
+ if (ret) {
+ tcf_idr_cleanup(tn, index);
return ret;
+ }
ret = ACT_P_CREATED;
} else {
if (bind)/* dont override defaults */
diff --git a/net/sched/act_mirred.c b/net/sched/act_mirred.c
index e08aed06d7f8..6afd89a36c69 100644
--- a/net/sched/act_mirred.c
+++ b/net/sched/act_mirred.c
@@ -79,7 +79,7 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
struct tcf_mirred *m;
struct net_device *dev;
bool exists = false;
- int ret;
+ int ret, err;
if (!nla) {
NL_SET_ERR_MSG_MOD(extack, "Mirred requires attributes to be passed");
@@ -94,7 +94,10 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
}
parm = nla_data(tb[TCA_MIRRED_PARMS]);
- exists = tcf_idr_check(tn, parm->index, a, bind);
+ err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
+ if (err < 0)
+ return err;
+ exists = err;
if (exists && bind)
return 0;
@@ -107,6 +110,8 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
default:
if (exists)
tcf_idr_release(*a, bind);
+ else
+ tcf_idr_cleanup(tn, parm->index);
NL_SET_ERR_MSG_MOD(extack, "Unknown mirred option");
return -EINVAL;
}
@@ -115,6 +120,8 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
if (dev == NULL) {
if (exists)
tcf_idr_release(*a, bind);
+ else
+ tcf_idr_cleanup(tn, parm->index);
return -ENODEV;
}
mac_header_xmit = dev_is_mac_header_xmit(dev);
@@ -124,13 +131,16 @@ static int tcf_mirred_init(struct net *net, struct nlattr *nla,
if (!exists) {
if (!dev) {
+ tcf_idr_cleanup(tn, parm->index);
NL_SET_ERR_MSG_MOD(extack, "Specified device does not exist");
return -EINVAL;
}
ret = tcf_idr_create(tn, parm->index, est, a,
&act_mirred_ops, bind, true);
- if (ret)
+ if (ret) {
+ tcf_idr_cleanup(tn, parm->index);
return ret;
+ }
ret = ACT_P_CREATED;
} else if (!ovr) {
tcf_idr_release(*a, bind);
diff --git a/net/sched/act_nat.c b/net/sched/act_nat.c
index 1f91e8e66c0f..4dd9188a72fd 100644
--- a/net/sched/act_nat.c
+++ b/net/sched/act_nat.c
@@ -57,19 +57,24 @@ static int tcf_nat_init(struct net *net, struct nlattr *nla, struct nlattr *est,
return -EINVAL;
parm = nla_data(tb[TCA_NAT_PARMS]);
- if (!tcf_idr_check(tn, parm->index, a, bind)) {
+ err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
+ if (!err) {
ret = tcf_idr_create(tn, parm->index, est, a,
&act_nat_ops, bind, false);
- if (ret)
+ if (ret) {
+ tcf_idr_cleanup(tn, parm->index);
return ret;
+ }
ret = ACT_P_CREATED;
- } else {
+ } else if (err > 0) {
if (bind)
return 0;
if (!ovr) {
tcf_idr_release(*a, bind);
return -EEXIST;
}
+ } else {
+ return err;
}
p = to_tcf_nat(*a);
diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index fbf283f2ac34..2bd1d3f61488 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -167,13 +167,18 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
if (IS_ERR(keys_ex))
return PTR_ERR(keys_ex);
- if (!tcf_idr_check(tn, parm->index, a, bind)) {
- if (!parm->nkeys)
+ err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
+ if (!err) {
+ if (!parm->nkeys) {
+ tcf_idr_cleanup(tn, parm->index);
return -EINVAL;
+ }
ret = tcf_idr_create(tn, parm->index, est, a,
&act_pedit_ops, bind, false);
- if (ret)
+ if (ret) {
+ tcf_idr_cleanup(tn, parm->index);
return ret;
+ }
p = to_pedit(*a);
keys = kmalloc(ksize, GFP_KERNEL);
if (keys == NULL) {
@@ -182,7 +187,7 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
return -ENOMEM;
}
ret = ACT_P_CREATED;
- } else {
+ } else if (err > 0) {
if (bind)
return 0;
if (!ovr) {
@@ -197,6 +202,8 @@ static int tcf_pedit_init(struct net *net, struct nlattr *nla,
return -ENOMEM;
}
}
+ } else {
+ return err;
}
spin_lock_bh(&p->tcf_lock);
diff --git a/net/sched/act_police.c b/net/sched/act_police.c
index 99335cca739e..1f3192ea8df7 100644
--- a/net/sched/act_police.c
+++ b/net/sched/act_police.c
@@ -101,15 +101,20 @@ static int tcf_act_police_init(struct net *net, struct nlattr *nla,
return -EINVAL;
parm = nla_data(tb[TCA_POLICE_TBF]);
- exists = tcf_idr_check(tn, parm->index, a, bind);
+ err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
+ if (err < 0)
+ return err;
+ exists = err;
if (exists && bind)
return 0;
if (!exists) {
ret = tcf_idr_create(tn, parm->index, NULL, a,
&act_police_ops, bind, false);
- if (ret)
+ if (ret) {
+ tcf_idr_cleanup(tn, parm->index);
return ret;
+ }
ret = ACT_P_CREATED;
} else if (!ovr) {
tcf_idr_release(*a, bind);
diff --git a/net/sched/act_sample.c b/net/sched/act_sample.c
index a8582e1347db..3079e7be5bde 100644
--- a/net/sched/act_sample.c
+++ b/net/sched/act_sample.c
@@ -46,7 +46,7 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
struct tc_sample *parm;
struct tcf_sample *s;
bool exists = false;
- int ret;
+ int ret, err;
if (!nla)
return -EINVAL;
@@ -59,15 +59,20 @@ static int tcf_sample_init(struct net *net, struct nlattr *nla,
parm = nla_data(tb[TCA_SAMPLE_PARMS]);
- exists = tcf_idr_check(tn, parm->index, a, bind);
+ err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
+ if (err < 0)
+ return err;
+ exists = err;
if (exists && bind)
return 0;
if (!exists) {
ret = tcf_idr_create(tn, parm->index, est, a,
&act_sample_ops, bind, false);
- if (ret)
+ if (ret) {
+ tcf_idr_cleanup(tn, parm->index);
return ret;
+ }
ret = ACT_P_CREATED;
} else if (!ovr) {
tcf_idr_release(*a, bind);
diff --git a/net/sched/act_simple.c b/net/sched/act_simple.c
index 78fffd329ed9..2cc874f791df 100644
--- a/net/sched/act_simple.c
+++ b/net/sched/act_simple.c
@@ -101,13 +101,18 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
return -EINVAL;
parm = nla_data(tb[TCA_DEF_PARMS]);
- exists = tcf_idr_check(tn, parm->index, a, bind);
+ err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
+ if (err < 0)
+ return err;
+ exists = err;
if (exists && bind)
return 0;
if (tb[TCA_DEF_DATA] == NULL) {
if (exists)
tcf_idr_release(*a, bind);
+ else
+ tcf_idr_cleanup(tn, parm->index);
return -EINVAL;
}
@@ -116,8 +121,10 @@ static int tcf_simp_init(struct net *net, struct nlattr *nla,
if (!exists) {
ret = tcf_idr_create(tn, parm->index, est, a,
&act_simp_ops, bind, false);
- if (ret)
+ if (ret) {
+ tcf_idr_cleanup(tn, parm->index);
return ret;
+ }
d = to_defact(*a);
ret = alloc_defdata(d, defdata);
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index c0607d1319eb..29a15172a99d 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -117,21 +117,28 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
parm = nla_data(tb[TCA_SKBEDIT_PARMS]);
- exists = tcf_idr_check(tn, parm->index, a, bind);
+ err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
+ if (err < 0)
+ return err;
+ exists = err;
if (exists && bind)
return 0;
if (!flags) {
if (exists)
tcf_idr_release(*a, bind);
+ else
+ tcf_idr_cleanup(tn, parm->index);
return -EINVAL;
}
if (!exists) {
ret = tcf_idr_create(tn, parm->index, est, a,
&act_skbedit_ops, bind, false);
- if (ret)
+ if (ret) {
+ tcf_idr_cleanup(tn, parm->index);
return ret;
+ }
d = to_skbedit(*a);
ret = ACT_P_CREATED;
diff --git a/net/sched/act_skbmod.c b/net/sched/act_skbmod.c
index e844381af066..cdc6bacfb190 100644
--- a/net/sched/act_skbmod.c
+++ b/net/sched/act_skbmod.c
@@ -128,21 +128,28 @@ static int tcf_skbmod_init(struct net *net, struct nlattr *nla,
if (parm->flags & SKBMOD_F_SWAPMAC)
lflags = SKBMOD_F_SWAPMAC;
- exists = tcf_idr_check(tn, parm->index, a, bind);
+ err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
+ if (err < 0)
+ return err;
+ exists = err;
if (exists && bind)
return 0;
if (!lflags) {
if (exists)
tcf_idr_release(*a, bind);
+ else
+ tcf_idr_cleanup(tn, parm->index);
return -EINVAL;
}
if (!exists) {
ret = tcf_idr_create(tn, parm->index, est, a,
&act_skbmod_ops, bind, true);
- if (ret)
+ if (ret) {
+ tcf_idr_cleanup(tn, parm->index);
return ret;
+ }
ret = ACT_P_CREATED;
} else if (!ovr) {
diff --git a/net/sched/act_tunnel_key.c b/net/sched/act_tunnel_key.c
index bd53f39a345b..4679b620af12 100644
--- a/net/sched/act_tunnel_key.c
+++ b/net/sched/act_tunnel_key.c
@@ -99,7 +99,10 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
return -EINVAL;
parm = nla_data(tb[TCA_TUNNEL_KEY_PARMS]);
- exists = tcf_idr_check(tn, parm->index, a, bind);
+ err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
+ if (err < 0)
+ return err;
+ exists = err;
if (exists && bind)
return 0;
@@ -162,7 +165,7 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
ret = tcf_idr_create(tn, parm->index, est, a,
&act_tunnel_key_ops, bind, true);
if (ret)
- return ret;
+ goto err_out;
ret = ACT_P_CREATED;
} else if (!ovr) {
@@ -198,6 +201,8 @@ static int tunnel_key_init(struct net *net, struct nlattr *nla,
err_out:
if (exists)
tcf_idr_release(*a, bind);
+ else
+ tcf_idr_cleanup(tn, parm->index);
return ret;
}
diff --git a/net/sched/act_vlan.c b/net/sched/act_vlan.c
index 4ac0d565e437..bae9822837d7 100644
--- a/net/sched/act_vlan.c
+++ b/net/sched/act_vlan.c
@@ -134,7 +134,10 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
if (!tb[TCA_VLAN_PARMS])
return -EINVAL;
parm = nla_data(tb[TCA_VLAN_PARMS]);
- exists = tcf_idr_check(tn, parm->index, a, bind);
+ err = tcf_idr_check_alloc(tn, &parm->index, a, bind);
+ if (err < 0)
+ return err;
+ exists = err;
if (exists && bind)
return 0;
@@ -146,12 +149,16 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
if (!tb[TCA_VLAN_PUSH_VLAN_ID]) {
if (exists)
tcf_idr_release(*a, bind);
+ else
+ tcf_idr_cleanup(tn, parm->index);
return -EINVAL;
}
push_vid = nla_get_u16(tb[TCA_VLAN_PUSH_VLAN_ID]);
if (push_vid >= VLAN_VID_MASK) {
if (exists)
tcf_idr_release(*a, bind);
+ else
+ tcf_idr_cleanup(tn, parm->index);
return -ERANGE;
}
@@ -162,6 +169,8 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
case htons(ETH_P_8021AD):
break;
default:
+ if (!exists)
+ tcf_idr_cleanup(tn, parm->index);
return -EPROTONOSUPPORT;
}
} else {
@@ -174,6 +183,8 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
default:
if (exists)
tcf_idr_release(*a, bind);
+ else
+ tcf_idr_cleanup(tn, parm->index);
return -EINVAL;
}
action = parm->v_action;
@@ -181,8 +192,10 @@ static int tcf_vlan_init(struct net *net, struct nlattr *nla,
if (!exists) {
ret = tcf_idr_create(tn, parm->index, est, a,
&act_vlan_ops, bind, true);
- if (ret)
+ if (ret) {
+ tcf_idr_cleanup(tn, parm->index);
return ret;
+ }
ret = ACT_P_CREATED;
} else if (!ovr) {
--
2.7.5
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox