* [PATCH net-next 1/5] ipv6: addrconf: cleanup locking in ipv6_add_addr
From: David Ahern @ 2017-10-13 23:02 UTC (permalink / raw)
To: netdev; +Cc: jiri, idosch, kjlx, davem, yoshfuji, David Ahern
In-Reply-To: <1507935733-18950-1-git-send-email-dsahern@gmail.com>
ipv6_add_addr is called in process context with rtnl lock held
(e.g., manual config of an address) or during softirq processing
(e.g., autoconf and address from a router advertisement).
Currently, ipv6_add_addr calls rcu_read_lock_bh shortly after entry
and does not call unlock until exit, minus the call around the address
validator notifier. Similarly, addrconf_hash_lock is taken after the
validator notifier and held until exit. This forces the allocation of
inet6_ifaddr to always be atomic.
Refactor ipv6_add_addr as follows:
1. add an input boolean to discriminate the call path (process context
or softirq). This new flag controls whether the alloc can be done
with GFP_KERNEL or GFP_ATOMIC.
2. Move the rcu_read_lock_bh and unlock calls only around functions that
do rcu updates.
3. Remove the in6_dev_hold and put added by 3ad7d2468f79f ("Ipvlan should
return an error when an address is already in use."). This was done
presumably because rcu_read_unlock_bh needs to be called before calling
the validator. Since rcu_read_lock is not needed before the validator
runs revert the hold and put added by 3ad7d2468f79f and only do the
hold when setting ifp->idev.
4. move duplicate address check and insertion of new address in the global
address hash into a helper. The helper is called after an ifa is
allocated and filled in.
This allows the ifa for manually configured addresses to be done with
GFP_KERNEL and reduces the overall amount of time with rcu_read_lock held
and hash table spinlock held.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/ipv6/addrconf.c | 97 +++++++++++++++++++++++++++++------------------------
1 file changed, 54 insertions(+), 43 deletions(-)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 4603aa488f4f..80f5fc74f0c4 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -957,18 +957,43 @@ static u32 inet6_addr_hash(const struct in6_addr *addr)
return hash_32(ipv6_addr_hash(addr), IN6_ADDR_HSIZE_SHIFT);
}
+static int ipv6_add_addr_hash(struct net_device *dev, struct inet6_ifaddr *ifa)
+{
+ unsigned int hash;
+ int err = 0;
+
+ spin_lock(&addrconf_hash_lock);
+
+ /* Ignore adding duplicate addresses on an interface */
+ if (ipv6_chk_same_addr(dev_net(dev), &ifa->addr, dev)) {
+ ADBG("ipv6_add_addr: already assigned\n");
+ err = -EEXIST;
+ goto out;
+ }
+
+ /* Add to big hash table */
+ hash = inet6_addr_hash(&ifa->addr);
+ hlist_add_head_rcu(&ifa->addr_lst, &inet6_addr_lst[hash]);
+
+out:
+ spin_unlock(&addrconf_hash_lock);
+
+ return err;
+}
+
/* 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)
+ int scope, u32 flags, u32 valid_lft, u32 prefered_lft,
+ bool can_block)
{
+ gfp_t gfp_flags = can_block ? GFP_KERNEL : GFP_ATOMIC;
struct net *net = dev_net(idev->dev);
struct inet6_ifaddr *ifa = NULL;
- struct rt6_info *rt;
+ struct rt6_info *rt = NULL;
struct in6_validator_info i6vi;
- unsigned int hash;
int err = 0;
int addr_type = ipv6_addr_type(addr);
@@ -978,42 +1003,24 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
addr_type & IPV6_ADDR_LOOPBACK))
return ERR_PTR(-EADDRNOTAVAIL);
- rcu_read_lock_bh();
-
- in6_dev_hold(idev);
-
if (idev->dead) {
err = -ENODEV; /*XXX*/
- goto out2;
+ goto out;
}
if (idev->cnf.disable_ipv6) {
err = -EACCES;
- goto out2;
+ goto out;
}
i6vi.i6vi_addr = *addr;
i6vi.i6vi_dev = idev;
- rcu_read_unlock_bh();
-
err = inet6addr_validator_notifier_call_chain(NETDEV_UP, &i6vi);
-
- rcu_read_lock_bh();
err = notifier_to_errno(err);
- if (err)
- goto out2;
-
- spin_lock(&addrconf_hash_lock);
-
- /* Ignore adding duplicate addresses on an interface */
- if (ipv6_chk_same_addr(dev_net(idev->dev), addr, idev->dev)) {
- ADBG("ipv6_add_addr: already assigned\n");
- err = -EEXIST;
+ if (err < 0)
goto out;
- }
-
- ifa = kzalloc(sizeof(struct inet6_ifaddr), GFP_ATOMIC);
+ ifa = kzalloc(sizeof(*ifa), gfp_flags);
if (!ifa) {
ADBG("ipv6_add_addr: malloc failed\n");
err = -ENOBUFS;
@@ -1053,16 +1060,21 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
ifa->rt = rt;
ifa->idev = idev;
+ in6_dev_hold(idev);
+
/* For caller */
refcount_set(&ifa->refcnt, 1);
- /* Add to big hash table */
- hash = inet6_addr_hash(addr);
+ rcu_read_lock_bh();
- hlist_add_head_rcu(&ifa->addr_lst, &inet6_addr_lst[hash]);
- spin_unlock(&addrconf_hash_lock);
+ err = ipv6_add_addr_hash(idev->dev, ifa);
+ if (err < 0) {
+ rcu_read_unlock_bh();
+ goto out;
+ }
write_lock(&idev->lock);
+
/* Add to inet6_dev unicast addr list. */
ipv6_link_dev_addr(idev, ifa);
@@ -1073,21 +1085,19 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
in6_ifa_hold(ifa);
write_unlock(&idev->lock);
-out2:
+
rcu_read_unlock_bh();
- if (likely(err == 0))
- inet6addr_notifier_call_chain(NETDEV_UP, ifa);
- else {
+ inet6addr_notifier_call_chain(NETDEV_UP, ifa);
+out:
+ if (unlikely(err < 0)) {
+ if (rt)
+ ip6_rt_put(rt);
kfree(ifa);
- in6_dev_put(idev);
ifa = ERR_PTR(err);
}
return ifa;
-out:
- spin_unlock(&addrconf_hash_lock);
- goto out2;
}
enum cleanup_prefix_rt_t {
@@ -1334,7 +1344,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *i
ift = ipv6_add_addr(idev, &addr, NULL, tmp_plen,
ipv6_addr_scope(&addr), addr_flags,
- tmp_valid_lft, tmp_prefered_lft);
+ tmp_valid_lft, tmp_prefered_lft, true);
if (IS_ERR(ift)) {
in6_ifa_put(ifp);
in6_dev_put(idev);
@@ -2018,7 +2028,7 @@ void addrconf_dad_failure(struct inet6_ifaddr *ifp)
ifp2 = ipv6_add_addr(idev, &new_addr, NULL, pfxlen,
scope, flags, valid_lft,
- preferred_lft);
+ preferred_lft, false);
if (IS_ERR(ifp2))
goto lock_errdad;
@@ -2476,7 +2486,7 @@ int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
pinfo->prefix_len,
addr_type&IPV6_ADDR_SCOPE_MASK,
addr_flags, valid_lft,
- prefered_lft);
+ prefered_lft, false);
if (IS_ERR_OR_NULL(ifp))
return -1;
@@ -2845,7 +2855,7 @@ static int inet6_addr_add(struct net *net, int ifindex,
}
ifp = ipv6_add_addr(idev, pfx, peer_pfx, plen, scope, ifa_flags,
- valid_lft, prefered_lft);
+ valid_lft, prefered_lft, true);
if (!IS_ERR(ifp)) {
if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
@@ -2960,7 +2970,8 @@ static void add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
ifp = ipv6_add_addr(idev, addr, NULL, plen,
scope, IFA_F_PERMANENT,
- INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
+ INFINITY_LIFE_TIME, INFINITY_LIFE_TIME,
+ true);
if (!IS_ERR(ifp)) {
spin_lock_bh(&ifp->lock);
ifp->flags &= ~IFA_F_TENTATIVE;
@@ -3060,7 +3071,7 @@ void addrconf_add_linklocal(struct inet6_dev *idev,
#endif
ifp = ipv6_add_addr(idev, addr, NULL, 64, IFA_LINK, addr_flags,
- INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
+ INFINITY_LIFE_TIME, INFINITY_LIFE_TIME, true);
if (!IS_ERR(ifp)) {
addrconf_prefix_route(&ifp->addr, ifp->prefix_len, idev->dev, 0, 0);
addrconf_dad_start(ifp);
--
2.1.4
^ permalink raw reply related
* [PATCH net-next 2/5] net: ipv6: Make inet6addr_validator a blocking notifier
From: David Ahern @ 2017-10-13 23:02 UTC (permalink / raw)
To: netdev; +Cc: jiri, idosch, kjlx, davem, yoshfuji, David Ahern
In-Reply-To: <1507935733-18950-1-git-send-email-dsahern@gmail.com>
inet6addr_validator chain was added by commit 3ad7d2468f79f ("Ipvlan
should return an error when an address is already in use") to allow
address validation before changes are committed and to be able to
fail the address change with an error back to the user. The address
validation is not done for addresses received from router
advertisements.
Handling RAs in softirq context is the only reason for the notifier
chain to be atomic versus blocking. Since the only current user, ipvlan,
of the validator chain ignores softirq context, the notifier can be made
blocking and simply not invoked for softirq path.
The blocking option is needed by spectrum for example to validate
resources for an adding an address to an interface.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
drivers/net/ipvlan/ipvlan_main.c | 4 ----
net/ipv6/addrconf.c | 21 ++++++++++++++-------
net/ipv6/addrconf_core.c | 9 +++++----
3 files changed, 19 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 3cf67db513e2..6842739b6679 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -808,10 +808,6 @@ static int ipvlan_addr6_event(struct notifier_block *unused,
struct net_device *dev = (struct net_device *)if6->idev->dev;
struct ipvl_dev *ipvlan = netdev_priv(dev);
- /* FIXME IPv6 autoconf calls us from bh without RTNL */
- if (in_softirq())
- return NOTIFY_DONE;
-
if (!netif_is_ipvlan(dev))
return NOTIFY_DONE;
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 80f5fc74f0c4..31ff12277bcf 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -993,7 +993,6 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
struct net *net = dev_net(idev->dev);
struct inet6_ifaddr *ifa = NULL;
struct rt6_info *rt = NULL;
- struct in6_validator_info i6vi;
int err = 0;
int addr_type = ipv6_addr_type(addr);
@@ -1013,12 +1012,20 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
goto out;
}
- i6vi.i6vi_addr = *addr;
- i6vi.i6vi_dev = idev;
- err = inet6addr_validator_notifier_call_chain(NETDEV_UP, &i6vi);
- err = notifier_to_errno(err);
- if (err < 0)
- goto out;
+ /* validator notifier needs to be blocking;
+ * do not call in atomic context
+ */
+ if (can_block) {
+ struct in6_validator_info i6vi = {
+ .i6vi_addr = *addr,
+ .i6vi_dev = idev,
+ };
+
+ err = inet6addr_validator_notifier_call_chain(NETDEV_UP, &i6vi);
+ err = notifier_to_errno(err);
+ if (err < 0)
+ goto out;
+ }
ifa = kzalloc(sizeof(*ifa), gfp_flags);
if (!ifa) {
diff --git a/net/ipv6/addrconf_core.c b/net/ipv6/addrconf_core.c
index 9e3488d50b15..32b564dfd02a 100644
--- a/net/ipv6/addrconf_core.c
+++ b/net/ipv6/addrconf_core.c
@@ -88,7 +88,7 @@ int __ipv6_addr_type(const struct in6_addr *addr)
EXPORT_SYMBOL(__ipv6_addr_type);
static ATOMIC_NOTIFIER_HEAD(inet6addr_chain);
-static ATOMIC_NOTIFIER_HEAD(inet6addr_validator_chain);
+static BLOCKING_NOTIFIER_HEAD(inet6addr_validator_chain);
int register_inet6addr_notifier(struct notifier_block *nb)
{
@@ -110,19 +110,20 @@ EXPORT_SYMBOL(inet6addr_notifier_call_chain);
int register_inet6addr_validator_notifier(struct notifier_block *nb)
{
- return atomic_notifier_chain_register(&inet6addr_validator_chain, nb);
+ return blocking_notifier_chain_register(&inet6addr_validator_chain, nb);
}
EXPORT_SYMBOL(register_inet6addr_validator_notifier);
int unregister_inet6addr_validator_notifier(struct notifier_block *nb)
{
- return atomic_notifier_chain_unregister(&inet6addr_validator_chain, nb);
+ return blocking_notifier_chain_unregister(&inet6addr_validator_chain,
+ nb);
}
EXPORT_SYMBOL(unregister_inet6addr_validator_notifier);
int inet6addr_validator_notifier_call_chain(unsigned long val, void *v)
{
- return atomic_notifier_call_chain(&inet6addr_validator_chain, val, v);
+ return blocking_notifier_call_chain(&inet6addr_validator_chain, val, v);
}
EXPORT_SYMBOL(inet6addr_validator_notifier_call_chain);
--
2.1.4
^ permalink raw reply related
* [PATCH net-next 3/5] net: Add extack to validator_info structs used for address notifier
From: David Ahern @ 2017-10-13 23:02 UTC (permalink / raw)
To: netdev; +Cc: jiri, idosch, kjlx, davem, yoshfuji, David Ahern
In-Reply-To: <1507935733-18950-1-git-send-email-dsahern@gmail.com>
Add extack to in_validator_info and in6_validator_info. Update the one
user of each, ipvlan, to return an error message for failures.
Only manual configuration of an address is plumbed in the IPv6 code path.
Signed-off-by: David Ahern <dsahern@gmail.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
drivers/net/ipvlan/ipvlan_main.c | 10 ++++++++--
include/linux/inetdevice.h | 1 +
include/net/addrconf.h | 1 +
net/ipv4/devinet.c | 8 +++++---
net/ipv6/addrconf.c | 22 ++++++++++++----------
5 files changed, 27 insertions(+), 15 deletions(-)
diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index 6842739b6679..f0ab55df57f1 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -847,8 +847,11 @@ static int ipvlan_addr6_validator_event(struct notifier_block *unused,
switch (event) {
case NETDEV_UP:
- if (ipvlan_addr_busy(ipvlan->port, &i6vi->i6vi_addr, true))
+ if (ipvlan_addr_busy(ipvlan->port, &i6vi->i6vi_addr, true)) {
+ NL_SET_ERR_MSG(i6vi->extack,
+ "Address already assigned to an ipvlan device");
return notifier_from_errno(-EADDRINUSE);
+ }
break;
}
@@ -917,8 +920,11 @@ static int ipvlan_addr4_validator_event(struct notifier_block *unused,
switch (event) {
case NETDEV_UP:
- if (ipvlan_addr_busy(ipvlan->port, &ivi->ivi_addr, false))
+ if (ipvlan_addr_busy(ipvlan->port, &ivi->ivi_addr, false)) {
+ NL_SET_ERR_MSG(ivi->extack,
+ "Address already assigned to an ipvlan device");
return notifier_from_errno(-EADDRINUSE);
+ }
break;
}
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index 751d051f0bc7..681dff30940b 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -154,6 +154,7 @@ struct in_ifaddr {
struct in_validator_info {
__be32 ivi_addr;
struct in_device *ivi_dev;
+ struct netlink_ext_ack *extack;
};
int register_inetaddr_notifier(struct notifier_block *nb);
diff --git a/include/net/addrconf.h b/include/net/addrconf.h
index 87981cd63180..b8b16437c6d5 100644
--- a/include/net/addrconf.h
+++ b/include/net/addrconf.h
@@ -55,6 +55,7 @@ struct prefix_info {
struct in6_validator_info {
struct in6_addr i6vi_addr;
struct inet6_dev *i6vi_dev;
+ struct netlink_ext_ack *extack;
};
#define IN6_ADDR_HSIZE_SHIFT 4
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index 7ce22a2c07ce..93773e5a80c7 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -444,7 +444,7 @@ static void check_lifetime(struct work_struct *work);
static DECLARE_DELAYED_WORK(check_lifetime_work, check_lifetime);
static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
- u32 portid)
+ u32 portid, struct netlink_ext_ack *extack)
{
struct in_device *in_dev = ifa->ifa_dev;
struct in_ifaddr *ifa1, **ifap, **last_primary;
@@ -489,6 +489,7 @@ static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
*/
ivi.ivi_addr = ifa->ifa_address;
ivi.ivi_dev = ifa->ifa_dev;
+ ivi.extack = extack;
ret = blocking_notifier_call_chain(&inetaddr_validator_chain,
NETDEV_UP, &ivi);
ret = notifier_to_errno(ret);
@@ -521,7 +522,7 @@ static int __inet_insert_ifa(struct in_ifaddr *ifa, struct nlmsghdr *nlh,
static int inet_insert_ifa(struct in_ifaddr *ifa)
{
- return __inet_insert_ifa(ifa, NULL, 0);
+ return __inet_insert_ifa(ifa, NULL, 0, NULL);
}
static int inet_set_ifa(struct net_device *dev, struct in_ifaddr *ifa)
@@ -902,7 +903,8 @@ static int inet_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
return ret;
}
}
- return __inet_insert_ifa(ifa, nlh, NETLINK_CB(skb).portid);
+ return __inet_insert_ifa(ifa, nlh, NETLINK_CB(skb).portid,
+ extack);
} else {
inet_free_ifa(ifa);
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 31ff12277bcf..0075dd3fdc57 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -987,7 +987,7 @@ 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,
- bool can_block)
+ bool can_block, struct netlink_ext_ack *extack)
{
gfp_t gfp_flags = can_block ? GFP_KERNEL : GFP_ATOMIC;
struct net *net = dev_net(idev->dev);
@@ -1019,6 +1019,7 @@ ipv6_add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
struct in6_validator_info i6vi = {
.i6vi_addr = *addr,
.i6vi_dev = idev,
+ .extack = extack,
};
err = inet6addr_validator_notifier_call_chain(NETDEV_UP, &i6vi);
@@ -1351,7 +1352,7 @@ static int ipv6_create_tempaddr(struct inet6_ifaddr *ifp, struct inet6_ifaddr *i
ift = ipv6_add_addr(idev, &addr, NULL, tmp_plen,
ipv6_addr_scope(&addr), addr_flags,
- tmp_valid_lft, tmp_prefered_lft, true);
+ tmp_valid_lft, tmp_prefered_lft, true, NULL);
if (IS_ERR(ift)) {
in6_ifa_put(ifp);
in6_dev_put(idev);
@@ -2035,7 +2036,7 @@ void addrconf_dad_failure(struct inet6_ifaddr *ifp)
ifp2 = ipv6_add_addr(idev, &new_addr, NULL, pfxlen,
scope, flags, valid_lft,
- preferred_lft, false);
+ preferred_lft, false, NULL);
if (IS_ERR(ifp2))
goto lock_errdad;
@@ -2493,7 +2494,7 @@ int addrconf_prefix_rcv_add_addr(struct net *net, struct net_device *dev,
pinfo->prefix_len,
addr_type&IPV6_ADDR_SCOPE_MASK,
addr_flags, valid_lft,
- prefered_lft, false);
+ prefered_lft, false, NULL);
if (IS_ERR_OR_NULL(ifp))
return -1;
@@ -2803,7 +2804,8 @@ static int inet6_addr_add(struct net *net, int ifindex,
const struct in6_addr *pfx,
const struct in6_addr *peer_pfx,
unsigned int plen, __u32 ifa_flags,
- __u32 prefered_lft, __u32 valid_lft)
+ __u32 prefered_lft, __u32 valid_lft,
+ struct netlink_ext_ack *extack)
{
struct inet6_ifaddr *ifp;
struct inet6_dev *idev;
@@ -2862,7 +2864,7 @@ static int inet6_addr_add(struct net *net, int ifindex,
}
ifp = ipv6_add_addr(idev, pfx, peer_pfx, plen, scope, ifa_flags,
- valid_lft, prefered_lft, true);
+ valid_lft, prefered_lft, true, extack);
if (!IS_ERR(ifp)) {
if (!(ifa_flags & IFA_F_NOPREFIXROUTE)) {
@@ -2947,7 +2949,7 @@ int addrconf_add_ifaddr(struct net *net, void __user *arg)
rtnl_lock();
err = inet6_addr_add(net, ireq.ifr6_ifindex, &ireq.ifr6_addr, NULL,
ireq.ifr6_prefixlen, IFA_F_PERMANENT,
- INFINITY_LIFE_TIME, INFINITY_LIFE_TIME);
+ INFINITY_LIFE_TIME, INFINITY_LIFE_TIME, NULL);
rtnl_unlock();
return err;
}
@@ -2978,7 +2980,7 @@ static void add_addr(struct inet6_dev *idev, const struct in6_addr *addr,
ifp = ipv6_add_addr(idev, addr, NULL, plen,
scope, IFA_F_PERMANENT,
INFINITY_LIFE_TIME, INFINITY_LIFE_TIME,
- true);
+ true, NULL);
if (!IS_ERR(ifp)) {
spin_lock_bh(&ifp->lock);
ifp->flags &= ~IFA_F_TENTATIVE;
@@ -3078,7 +3080,7 @@ void addrconf_add_linklocal(struct inet6_dev *idev,
#endif
ifp = ipv6_add_addr(idev, addr, NULL, 64, IFA_LINK, addr_flags,
- INFINITY_LIFE_TIME, INFINITY_LIFE_TIME, true);
+ INFINITY_LIFE_TIME, INFINITY_LIFE_TIME, true, NULL);
if (!IS_ERR(ifp)) {
addrconf_prefix_route(&ifp->addr, ifp->prefix_len, idev->dev, 0, 0);
addrconf_dad_start(ifp);
@@ -4581,7 +4583,7 @@ inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh,
*/
return inet6_addr_add(net, ifm->ifa_index, pfx, peer_pfx,
ifm->ifa_prefixlen, ifa_flags,
- preferred_lft, valid_lft);
+ preferred_lft, valid_lft, extack);
}
if (nlh->nlmsg_flags & NLM_F_EXCL ||
--
2.1.4
^ permalink raw reply related
* [PATCH net-next 4/5] mlxsw: spectrum: router: Add support for address validator notifier
From: David Ahern @ 2017-10-13 23:02 UTC (permalink / raw)
To: netdev; +Cc: jiri, idosch, kjlx, davem, yoshfuji, David Ahern
In-Reply-To: <1507935733-18950-1-git-send-email-dsahern@gmail.com>
Add support for inetaddr_validator and inet6addr_validator. The
notifiers provide a means for validating ipv4 and ipv6 addresses
before the addresses are installed and on failure the error
is propagated back to the user.
Signed-off-by: David Ahern <dsahern@gmail.com>
---
drivers/net/ethernet/mellanox/mlxsw/spectrum.c | 15 ++++++-
drivers/net/ethernet/mellanox/mlxsw/spectrum.h | 4 ++
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 52 ++++++++++++++++++++++
3 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
index 321988ac57cc..d51402f98f97 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.c
@@ -4505,9 +4505,16 @@ static struct notifier_block mlxsw_sp_netdevice_nb __read_mostly = {
.notifier_call = mlxsw_sp_netdevice_event,
};
+static struct notifier_block mlxsw_sp_inetaddr_valid_nb __read_mostly = {
+ .notifier_call = mlxsw_sp_inetaddr_valid_event,
+};
+
static struct notifier_block mlxsw_sp_inetaddr_nb __read_mostly = {
.notifier_call = mlxsw_sp_inetaddr_event,
- .priority = 10, /* Must be called before FIB notifier block */
+};
+
+static struct notifier_block mlxsw_sp_inet6addr_valid_nb __read_mostly = {
+ .notifier_call = mlxsw_sp_inet6addr_valid_event,
};
static struct notifier_block mlxsw_sp_inet6addr_nb __read_mostly = {
@@ -4533,7 +4540,9 @@ static int __init mlxsw_sp_module_init(void)
int err;
register_netdevice_notifier(&mlxsw_sp_netdevice_nb);
+ register_inetaddr_validator_notifier(&mlxsw_sp_inetaddr_valid_nb);
register_inetaddr_notifier(&mlxsw_sp_inetaddr_nb);
+ register_inet6addr_validator_notifier(&mlxsw_sp_inet6addr_valid_nb);
register_inet6addr_notifier(&mlxsw_sp_inet6addr_nb);
register_netevent_notifier(&mlxsw_sp_router_netevent_nb);
@@ -4552,7 +4561,9 @@ static int __init mlxsw_sp_module_init(void)
err_core_driver_register:
unregister_netevent_notifier(&mlxsw_sp_router_netevent_nb);
unregister_inet6addr_notifier(&mlxsw_sp_inet6addr_nb);
+ unregister_inet6addr_validator_notifier(&mlxsw_sp_inet6addr_valid_nb);
unregister_inetaddr_notifier(&mlxsw_sp_inetaddr_nb);
+ unregister_inetaddr_validator_notifier(&mlxsw_sp_inetaddr_valid_nb);
unregister_netdevice_notifier(&mlxsw_sp_netdevice_nb);
return err;
}
@@ -4563,7 +4574,9 @@ static void __exit mlxsw_sp_module_exit(void)
mlxsw_core_driver_unregister(&mlxsw_sp_driver);
unregister_netevent_notifier(&mlxsw_sp_router_netevent_nb);
unregister_inet6addr_notifier(&mlxsw_sp_inet6addr_nb);
+ unregister_inet6addr_validator_notifier(&mlxsw_sp_inet6addr_valid_nb);
unregister_inetaddr_notifier(&mlxsw_sp_inetaddr_nb);
+ unregister_inetaddr_validator_notifier(&mlxsw_sp_inetaddr_valid_nb);
unregister_netdevice_notifier(&mlxsw_sp_netdevice_nb);
}
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
index 8e45183dc9bb..4865a6f58c83 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum.h
@@ -390,8 +390,12 @@ int mlxsw_sp_router_netevent_event(struct notifier_block *unused,
int mlxsw_sp_netdevice_router_port_event(struct net_device *dev);
int mlxsw_sp_inetaddr_event(struct notifier_block *unused,
unsigned long event, void *ptr);
+int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
+ unsigned long event, void *ptr);
int mlxsw_sp_inet6addr_event(struct notifier_block *unused,
unsigned long event, void *ptr);
+int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
+ unsigned long event, void *ptr);
int mlxsw_sp_netdevice_vrf_event(struct net_device *l3_dev, unsigned long event,
struct netdev_notifier_changeupper_info *info);
void
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 6a356f4b99a3..2a7f066dfab5 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -5656,6 +5656,32 @@ int mlxsw_sp_inetaddr_event(struct notifier_block *unused,
struct mlxsw_sp_rif *rif;
int err = 0;
+ /* NETDEV_UP event is handled by mlxsw_sp_inetaddr_valid_event */
+ if (event == NETDEV_UP)
+ goto out;
+
+ mlxsw_sp = mlxsw_sp_lower_get(dev);
+ if (!mlxsw_sp)
+ goto out;
+
+ rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
+ if (!mlxsw_sp_rif_should_config(rif, dev, event))
+ goto out;
+
+ err = __mlxsw_sp_inetaddr_event(dev, event);
+out:
+ return notifier_from_errno(err);
+}
+
+int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
+ unsigned long event, void *ptr)
+{
+ struct in_validator_info *ivi = (struct in_validator_info *) ptr;
+ struct net_device *dev = ivi->ivi_dev->dev;
+ struct mlxsw_sp *mlxsw_sp;
+ struct mlxsw_sp_rif *rif;
+ int err = 0;
+
mlxsw_sp = mlxsw_sp_lower_get(dev);
if (!mlxsw_sp)
goto out;
@@ -5708,6 +5734,10 @@ int mlxsw_sp_inet6addr_event(struct notifier_block *unused,
struct mlxsw_sp_inet6addr_event_work *inet6addr_work;
struct net_device *dev = if6->idev->dev;
+ /* NETDEV_UP event is handled by mlxsw_sp_inet6addr_valid_event */
+ if (event == NETDEV_UP)
+ return NOTIFY_DONE;
+
if (!mlxsw_sp_port_dev_lower_find_rcu(dev))
return NOTIFY_DONE;
@@ -5724,6 +5754,28 @@ int mlxsw_sp_inet6addr_event(struct notifier_block *unused,
return NOTIFY_DONE;
}
+int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
+ unsigned long event, void *ptr)
+{
+ struct in6_validator_info *i6vi = (struct in6_validator_info *) ptr;
+ struct net_device *dev = i6vi->i6vi_dev->dev;
+ struct mlxsw_sp *mlxsw_sp;
+ struct mlxsw_sp_rif *rif;
+ int err = 0;
+
+ mlxsw_sp = mlxsw_sp_lower_get(dev);
+ if (!mlxsw_sp)
+ goto out;
+
+ rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, dev);
+ if (!mlxsw_sp_rif_should_config(rif, dev, event))
+ goto out;
+
+ err = __mlxsw_sp_inetaddr_event(dev, event);
+out:
+ return notifier_from_errno(err);
+}
+
static int mlxsw_sp_rif_edit(struct mlxsw_sp *mlxsw_sp, u16 rif_index,
const char *mac, int mtu)
{
--
2.1.4
^ permalink raw reply related
* [PATCH net-next 5/5] mlxsw: spectrum_router: Add extack message for RIF and VRF overflow
From: David Ahern @ 2017-10-13 23:02 UTC (permalink / raw)
To: netdev; +Cc: jiri, idosch, kjlx, davem, yoshfuji, David Ahern
In-Reply-To: <1507935733-18950-1-git-send-email-dsahern@gmail.com>
Add extack argument down to mlxsw_sp_rif_create and mlxsw_sp_vr_create
to set an error message on RIF or VR overflow. Now on overflow of
either resource the user gets an informative message as opposed to
failing with EBUSY.
Signed-off-by: David Ahern <dsahern@gmail.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
---
.../net/ethernet/mellanox/mlxsw/spectrum_router.c | 114 +++++++++++++--------
1 file changed, 69 insertions(+), 45 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
index 2a7f066dfab5..9e0b46513ca7 100644
--- a/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
+++ b/drivers/net/ethernet/mellanox/mlxsw/spectrum_router.c
@@ -731,14 +731,17 @@ static struct mlxsw_sp_fib *mlxsw_sp_vr_fib(const struct mlxsw_sp_vr *vr,
}
static struct mlxsw_sp_vr *mlxsw_sp_vr_create(struct mlxsw_sp *mlxsw_sp,
- u32 tb_id)
+ u32 tb_id,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp_vr *vr;
int err;
vr = mlxsw_sp_vr_find_unused(mlxsw_sp);
- if (!vr)
+ if (!vr) {
+ NL_SET_ERR_MSG(extack, "spectrum: Exceeded number of supported virtual routers");
return ERR_PTR(-EBUSY);
+ }
vr->fib4 = mlxsw_sp_fib_create(vr, MLXSW_SP_L3_PROTO_IPV4);
if (IS_ERR(vr->fib4))
return ERR_CAST(vr->fib4);
@@ -775,14 +778,15 @@ static void mlxsw_sp_vr_destroy(struct mlxsw_sp_vr *vr)
vr->fib4 = NULL;
}
-static struct mlxsw_sp_vr *mlxsw_sp_vr_get(struct mlxsw_sp *mlxsw_sp, u32 tb_id)
+static struct mlxsw_sp_vr *mlxsw_sp_vr_get(struct mlxsw_sp *mlxsw_sp, u32 tb_id,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp_vr *vr;
tb_id = mlxsw_sp_fix_tb_id(tb_id);
vr = mlxsw_sp_vr_find(mlxsw_sp, tb_id);
if (!vr)
- vr = mlxsw_sp_vr_create(mlxsw_sp, tb_id);
+ vr = mlxsw_sp_vr_create(mlxsw_sp, tb_id, extack);
return vr;
}
@@ -948,7 +952,8 @@ static u32 mlxsw_sp_ipip_dev_ul_tb_id(const struct net_device *ol_dev)
static struct mlxsw_sp_rif *
mlxsw_sp_rif_create(struct mlxsw_sp *mlxsw_sp,
- const struct mlxsw_sp_rif_params *params);
+ const struct mlxsw_sp_rif_params *params,
+ struct netlink_ext_ack *extack);
static struct mlxsw_sp_rif_ipip_lb *
mlxsw_sp_ipip_ol_ipip_lb_create(struct mlxsw_sp *mlxsw_sp,
@@ -966,7 +971,7 @@ mlxsw_sp_ipip_ol_ipip_lb_create(struct mlxsw_sp *mlxsw_sp,
.lb_config = ipip_ops->ol_loopback_config(mlxsw_sp, ol_dev),
};
- rif = mlxsw_sp_rif_create(mlxsw_sp, &lb_params.common);
+ rif = mlxsw_sp_rif_create(mlxsw_sp, &lb_params.common, NULL);
if (IS_ERR(rif))
return ERR_CAST(rif);
return container_of(rif, struct mlxsw_sp_rif_ipip_lb, common);
@@ -3711,7 +3716,7 @@ mlxsw_sp_fib_node_get(struct mlxsw_sp *mlxsw_sp, u32 tb_id, const void *addr,
struct mlxsw_sp_vr *vr;
int err;
- vr = mlxsw_sp_vr_get(mlxsw_sp, tb_id);
+ vr = mlxsw_sp_vr_get(mlxsw_sp, tb_id, NULL);
if (IS_ERR(vr))
return ERR_CAST(vr);
fib = mlxsw_sp_vr_fib(vr, proto);
@@ -4750,7 +4755,7 @@ static int mlxsw_sp_router_fibmr_add(struct mlxsw_sp *mlxsw_sp,
if (mlxsw_sp->router->aborted)
return 0;
- vr = mlxsw_sp_vr_get(mlxsw_sp, men_info->tb_id);
+ vr = mlxsw_sp_vr_get(mlxsw_sp, men_info->tb_id, NULL);
if (IS_ERR(vr))
return PTR_ERR(vr);
@@ -4783,7 +4788,7 @@ mlxsw_sp_router_fibmr_vif_add(struct mlxsw_sp *mlxsw_sp,
if (mlxsw_sp->router->aborted)
return 0;
- vr = mlxsw_sp_vr_get(mlxsw_sp, ven_info->tb_id);
+ vr = mlxsw_sp_vr_get(mlxsw_sp, ven_info->tb_id, NULL);
if (IS_ERR(vr))
return PTR_ERR(vr);
@@ -5346,7 +5351,8 @@ const struct net_device *mlxsw_sp_rif_dev(const struct mlxsw_sp_rif *rif)
static struct mlxsw_sp_rif *
mlxsw_sp_rif_create(struct mlxsw_sp *mlxsw_sp,
- const struct mlxsw_sp_rif_params *params)
+ const struct mlxsw_sp_rif_params *params,
+ struct netlink_ext_ack *extack)
{
u32 tb_id = l3mdev_fib_table(params->dev);
const struct mlxsw_sp_rif_ops *ops;
@@ -5360,14 +5366,16 @@ mlxsw_sp_rif_create(struct mlxsw_sp *mlxsw_sp,
type = mlxsw_sp_dev_rif_type(mlxsw_sp, params->dev);
ops = mlxsw_sp->router->rif_ops_arr[type];
- vr = mlxsw_sp_vr_get(mlxsw_sp, tb_id ? : RT_TABLE_MAIN);
+ vr = mlxsw_sp_vr_get(mlxsw_sp, tb_id ? : RT_TABLE_MAIN, extack);
if (IS_ERR(vr))
return ERR_CAST(vr);
vr->rif_count++;
err = mlxsw_sp_rif_index_alloc(mlxsw_sp, &rif_index);
- if (err)
+ if (err) {
+ NL_SET_ERR_MSG(extack, "spectrum: Exceeded number of supported router interfaces");
goto err_rif_index_alloc;
+ }
rif = mlxsw_sp_rif_alloc(ops->rif_size, rif_index, vr->id, params->dev);
if (!rif) {
@@ -5454,7 +5462,8 @@ mlxsw_sp_rif_subport_params_init(struct mlxsw_sp_rif_params *params,
static int
mlxsw_sp_port_vlan_router_join(struct mlxsw_sp_port_vlan *mlxsw_sp_port_vlan,
- struct net_device *l3_dev)
+ struct net_device *l3_dev,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp_port *mlxsw_sp_port = mlxsw_sp_port_vlan->mlxsw_sp_port;
struct mlxsw_sp *mlxsw_sp = mlxsw_sp_port->mlxsw_sp;
@@ -5470,7 +5479,7 @@ mlxsw_sp_port_vlan_router_join(struct mlxsw_sp_port_vlan *mlxsw_sp_port_vlan,
};
mlxsw_sp_rif_subport_params_init(¶ms, mlxsw_sp_port_vlan);
- rif = mlxsw_sp_rif_create(mlxsw_sp, ¶ms);
+ rif = mlxsw_sp_rif_create(mlxsw_sp, ¶ms, extack);
if (IS_ERR(rif))
return PTR_ERR(rif);
}
@@ -5525,7 +5534,8 @@ mlxsw_sp_port_vlan_router_leave(struct mlxsw_sp_port_vlan *mlxsw_sp_port_vlan)
static int mlxsw_sp_inetaddr_port_vlan_event(struct net_device *l3_dev,
struct net_device *port_dev,
- unsigned long event, u16 vid)
+ unsigned long event, u16 vid,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp_port *mlxsw_sp_port = netdev_priv(port_dev);
struct mlxsw_sp_port_vlan *mlxsw_sp_port_vlan;
@@ -5537,7 +5547,7 @@ static int mlxsw_sp_inetaddr_port_vlan_event(struct net_device *l3_dev,
switch (event) {
case NETDEV_UP:
return mlxsw_sp_port_vlan_router_join(mlxsw_sp_port_vlan,
- l3_dev);
+ l3_dev, extack);
case NETDEV_DOWN:
mlxsw_sp_port_vlan_router_leave(mlxsw_sp_port_vlan);
break;
@@ -5547,19 +5557,22 @@ static int mlxsw_sp_inetaddr_port_vlan_event(struct net_device *l3_dev,
}
static int mlxsw_sp_inetaddr_port_event(struct net_device *port_dev,
- unsigned long event)
+ unsigned long event,
+ struct netlink_ext_ack *extack)
{
if (netif_is_bridge_port(port_dev) ||
netif_is_lag_port(port_dev) ||
netif_is_ovs_port(port_dev))
return 0;
- return mlxsw_sp_inetaddr_port_vlan_event(port_dev, port_dev, event, 1);
+ return mlxsw_sp_inetaddr_port_vlan_event(port_dev, port_dev, event, 1,
+ extack);
}
static int __mlxsw_sp_inetaddr_lag_event(struct net_device *l3_dev,
struct net_device *lag_dev,
- unsigned long event, u16 vid)
+ unsigned long event, u16 vid,
+ struct netlink_ext_ack *extack)
{
struct net_device *port_dev;
struct list_head *iter;
@@ -5569,7 +5582,8 @@ static int __mlxsw_sp_inetaddr_lag_event(struct net_device *l3_dev,
if (mlxsw_sp_port_dev_check(port_dev)) {
err = mlxsw_sp_inetaddr_port_vlan_event(l3_dev,
port_dev,
- event, vid);
+ event, vid,
+ extack);
if (err)
return err;
}
@@ -5579,16 +5593,19 @@ static int __mlxsw_sp_inetaddr_lag_event(struct net_device *l3_dev,
}
static int mlxsw_sp_inetaddr_lag_event(struct net_device *lag_dev,
- unsigned long event)
+ unsigned long event,
+ struct netlink_ext_ack *extack)
{
if (netif_is_bridge_port(lag_dev))
return 0;
- return __mlxsw_sp_inetaddr_lag_event(lag_dev, lag_dev, event, 1);
+ return __mlxsw_sp_inetaddr_lag_event(lag_dev, lag_dev, event, 1,
+ extack);
}
static int mlxsw_sp_inetaddr_bridge_event(struct net_device *l3_dev,
- unsigned long event)
+ unsigned long event,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp *mlxsw_sp = mlxsw_sp_lower_get(l3_dev);
struct mlxsw_sp_rif_params params = {
@@ -5598,7 +5615,7 @@ static int mlxsw_sp_inetaddr_bridge_event(struct net_device *l3_dev,
switch (event) {
case NETDEV_UP:
- rif = mlxsw_sp_rif_create(mlxsw_sp, ¶ms);
+ rif = mlxsw_sp_rif_create(mlxsw_sp, ¶ms, extack);
if (IS_ERR(rif))
return PTR_ERR(rif);
break;
@@ -5612,7 +5629,8 @@ static int mlxsw_sp_inetaddr_bridge_event(struct net_device *l3_dev,
}
static int mlxsw_sp_inetaddr_vlan_event(struct net_device *vlan_dev,
- unsigned long event)
+ unsigned long event,
+ struct netlink_ext_ack *extack)
{
struct net_device *real_dev = vlan_dev_real_dev(vlan_dev);
u16 vid = vlan_dev_vlan_id(vlan_dev);
@@ -5622,27 +5640,28 @@ static int mlxsw_sp_inetaddr_vlan_event(struct net_device *vlan_dev,
if (mlxsw_sp_port_dev_check(real_dev))
return mlxsw_sp_inetaddr_port_vlan_event(vlan_dev, real_dev,
- event, vid);
+ event, vid, extack);
else if (netif_is_lag_master(real_dev))
return __mlxsw_sp_inetaddr_lag_event(vlan_dev, real_dev, event,
- vid);
+ vid, extack);
else if (netif_is_bridge_master(real_dev) && br_vlan_enabled(real_dev))
- return mlxsw_sp_inetaddr_bridge_event(vlan_dev, event);
+ return mlxsw_sp_inetaddr_bridge_event(vlan_dev, event, extack);
return 0;
}
static int __mlxsw_sp_inetaddr_event(struct net_device *dev,
- unsigned long event)
+ unsigned long event,
+ struct netlink_ext_ack *extack)
{
if (mlxsw_sp_port_dev_check(dev))
- return mlxsw_sp_inetaddr_port_event(dev, event);
+ return mlxsw_sp_inetaddr_port_event(dev, event, extack);
else if (netif_is_lag_master(dev))
- return mlxsw_sp_inetaddr_lag_event(dev, event);
+ return mlxsw_sp_inetaddr_lag_event(dev, event, extack);
else if (netif_is_bridge_master(dev))
- return mlxsw_sp_inetaddr_bridge_event(dev, event);
+ return mlxsw_sp_inetaddr_bridge_event(dev, event, extack);
else if (is_vlan_dev(dev))
- return mlxsw_sp_inetaddr_vlan_event(dev, event);
+ return mlxsw_sp_inetaddr_vlan_event(dev, event, extack);
else
return 0;
}
@@ -5668,7 +5687,7 @@ int mlxsw_sp_inetaddr_event(struct notifier_block *unused,
if (!mlxsw_sp_rif_should_config(rif, dev, event))
goto out;
- err = __mlxsw_sp_inetaddr_event(dev, event);
+ err = __mlxsw_sp_inetaddr_event(dev, event, NULL);
out:
return notifier_from_errno(err);
}
@@ -5690,7 +5709,7 @@ int mlxsw_sp_inetaddr_valid_event(struct notifier_block *unused,
if (!mlxsw_sp_rif_should_config(rif, dev, event))
goto out;
- err = __mlxsw_sp_inetaddr_event(dev, event);
+ err = __mlxsw_sp_inetaddr_event(dev, event, ivi->extack);
out:
return notifier_from_errno(err);
}
@@ -5719,7 +5738,7 @@ static void mlxsw_sp_inet6addr_event_work(struct work_struct *work)
if (!mlxsw_sp_rif_should_config(rif, dev, event))
goto out;
- __mlxsw_sp_inetaddr_event(dev, event);
+ __mlxsw_sp_inetaddr_event(dev, event, NULL);
out:
rtnl_unlock();
dev_put(dev);
@@ -5771,7 +5790,7 @@ int mlxsw_sp_inet6addr_valid_event(struct notifier_block *unused,
if (!mlxsw_sp_rif_should_config(rif, dev, event))
goto out;
- err = __mlxsw_sp_inetaddr_event(dev, event);
+ err = __mlxsw_sp_inetaddr_event(dev, event, i6vi->extack);
out:
return notifier_from_errno(err);
}
@@ -5848,7 +5867,8 @@ int mlxsw_sp_netdevice_router_port_event(struct net_device *dev)
}
static int mlxsw_sp_port_vrf_join(struct mlxsw_sp *mlxsw_sp,
- struct net_device *l3_dev)
+ struct net_device *l3_dev,
+ struct netlink_ext_ack *extack)
{
struct mlxsw_sp_rif *rif;
@@ -5857,9 +5877,9 @@ static int mlxsw_sp_port_vrf_join(struct mlxsw_sp *mlxsw_sp,
*/
rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, l3_dev);
if (rif)
- __mlxsw_sp_inetaddr_event(l3_dev, NETDEV_DOWN);
+ __mlxsw_sp_inetaddr_event(l3_dev, NETDEV_DOWN, extack);
- return __mlxsw_sp_inetaddr_event(l3_dev, NETDEV_UP);
+ return __mlxsw_sp_inetaddr_event(l3_dev, NETDEV_UP, extack);
}
static void mlxsw_sp_port_vrf_leave(struct mlxsw_sp *mlxsw_sp,
@@ -5870,7 +5890,7 @@ static void mlxsw_sp_port_vrf_leave(struct mlxsw_sp *mlxsw_sp,
rif = mlxsw_sp_rif_find_by_dev(mlxsw_sp, l3_dev);
if (!rif)
return;
- __mlxsw_sp_inetaddr_event(l3_dev, NETDEV_DOWN);
+ __mlxsw_sp_inetaddr_event(l3_dev, NETDEV_DOWN, NULL);
}
int mlxsw_sp_netdevice_vrf_event(struct net_device *l3_dev, unsigned long event,
@@ -5886,10 +5906,14 @@ int mlxsw_sp_netdevice_vrf_event(struct net_device *l3_dev, unsigned long event,
case NETDEV_PRECHANGEUPPER:
return 0;
case NETDEV_CHANGEUPPER:
- if (info->linking)
- err = mlxsw_sp_port_vrf_join(mlxsw_sp, l3_dev);
- else
+ if (info->linking) {
+ struct netlink_ext_ack *extack;
+
+ extack = netdev_notifier_info_to_extack(&info->info);
+ err = mlxsw_sp_port_vrf_join(mlxsw_sp, l3_dev, extack);
+ } else {
mlxsw_sp_port_vrf_leave(mlxsw_sp, l3_dev);
+ }
break;
}
@@ -6196,7 +6220,7 @@ mlxsw_sp_rif_ipip_lb_configure(struct mlxsw_sp_rif *rif)
struct mlxsw_sp_vr *ul_vr;
int err;
- ul_vr = mlxsw_sp_vr_get(mlxsw_sp, ul_tb_id);
+ ul_vr = mlxsw_sp_vr_get(mlxsw_sp, ul_tb_id, NULL);
if (IS_ERR(ul_vr))
return PTR_ERR(ul_vr);
--
2.1.4
^ permalink raw reply related
* Re: [Patch net-next v3] tcp: add a tracepoint for tcp retransmission
From: David Ahern @ 2017-10-13 23:13 UTC (permalink / raw)
To: Cong Wang, netdev
Cc: Eric Dumazet, Alexei Starovoitov, Hannes Frederic Sowa,
Brendan Gregg, Neal Cardwell
In-Reply-To: <20171013200316.28561-1-xiyou.wangcong@gmail.com>
On 10/13/17 2:03 PM, Cong Wang wrote:
> diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
> new file mode 100644
> index 000000000000..3d1cbd072b7e
> --- /dev/null
> +++ b/include/trace/events/tcp.h
> @@ -0,0 +1,68 @@
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM tcp
> +
> +#if !defined(_TRACE_TCP_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_TCP_H
> +
> +#include <linux/ipv6.h>
> +#include <linux/tcp.h>
> +#include <linux/tracepoint.h>
> +#include <net/ipv6.h>
> +
> +TRACE_EVENT(tcp_retransmit_skb,
> +
> + TP_PROTO(struct sock *sk, struct sk_buff *skb),
> +
> + TP_ARGS(sk, skb),
> +
> + TP_STRUCT__entry(
> + __field(void *, skbaddr)
> + __field(void *, skaddr)
> + __field(__u16, sport)
> + __field(__u16, dport)
> + __array(__u8, saddr, 4)
> + __array(__u8, daddr, 4)
> + __array(__u8, saddr_v6, 16)
> + __array(__u8, daddr_v6, 16)
> + ),
> +
> + TP_fast_assign(
> + struct ipv6_pinfo *np = inet6_sk(sk);
> + struct inet_sock *inet = inet_sk(sk);
> + struct in6_addr *pin6;
> + __be32 *p32;
> +
> + __entry->skbaddr = skb;
> + __entry->skaddr = sk;
> +
> + __entry->sport = ntohs(inet->inet_sport);
> + __entry->dport = ntohs(inet->inet_dport);
> +
> + p32 = (__be32 *) __entry->saddr;
> + *p32 = inet->inet_saddr;
> +
> + p32 = (__be32 *) __entry->daddr;
> + *p32 = inet->inet_daddr;
> +
> + if (np) {
> + pin6 = (struct in6_addr *)__entry->saddr_v6;
> + *pin6 = np->saddr;
> + pin6 = (struct in6_addr *)__entry->daddr_v6;
> + *pin6 = *(np->daddr_cache);
> + } else {
> + pin6 = (struct in6_addr *)__entry->saddr_v6;
> + ipv6_addr_set_v4mapped(inet->inet_saddr, pin6);
> + pin6 = (struct in6_addr *)__entry->daddr_v6;
> + ipv6_addr_set_v4mapped(inet->inet_daddr, pin6);
> + }
> + ),
> +
> + TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6 daddrv6=%pI6",
%pI6c is more user friendly for IPv6 addresses.
^ permalink raw reply
* Re: [Patch net-next v3] tcp: add a tracepoint for tcp retransmission
From: Brendan Gregg @ 2017-10-13 23:58 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Cong Wang, netdev, Eric Dumazet, Hannes Frederic Sowa,
Neal Cardwell
In-Reply-To: <20171013220937.qoyu5heijjsr6z6p@ast-mbp>
On Fri, Oct 13, 2017 at 3:09 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Fri, Oct 13, 2017 at 01:50:44PM -0700, Brendan Gregg wrote:
>> On Fri, Oct 13, 2017 at 1:03 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> > We need a real-time notification for tcp retransmission
>> > for monitoring.
>> >
>> > Of course we could use ftrace to dynamically instrument this
>> > kernel function too, however we can't retrieve the connection
>> > information at the same time, for example perf-tools [1] reads
>> > /proc/net/tcp for socket details, which is slow when we have
>> > a lots of connections.
>> >
>> > Therefore, this patch adds a tracepoint for __tcp_retransmit_skb()
>> > and exposes src/dst IP addresses and ports of the connection.
>> > This also makes it easier to integrate into perf.
>> >
>> > Note, I expose both IPv4 and IPv6 addresses at the same time:
>> > for a IPv4 socket, v4 mapped address is used as IPv6 addresses,
>> > for a IPv6 socket, LOOPBACK4_IPV6 is already filled by kernel.
>> > Also, add sk and skb pointers as they are useful for BPF.
>>
>> Thanks, a TCP retransmit tracepoint would be great. (tcp_set_state
>> would be highly useful too, which Alexei already has in his list).
>>
>> Should skp->__sk_common.skc_state be included in the format string, so
>> we don't have to always dig it out of the skaddr? For retransmits I
>> always want to know the TCP state, to determine if it is ESTABLISHED
>> (packet drop) or SYN_SENT (backlog full) or something else.
>
> let's not expose internal socket fields into tp fields.
> Few people still believe that tp fields are abi, so to be safe
> no such fields should be exposed.
> It's trivial enough to read sk_state from bpf program
> with bpf_probe_read().
Ah, right, the number mapping for TCP_ESTABLISHED and friends is a
Linux implementation detail, and not from the RFCs. Ok, I can dig it
from the skp instead.
>
>> We probably need a tracepoint for tcp_send_loss_probe() (TLP) as well,
>> for tracing at the same time as retransmits (like my tools do), but
>> that can be added later.
>
> hmm. why?
> This single tracepoint will cover both cases of retransmits.
I don't think tcp_send_loss_probe() TLP goes through
__tcp_retransmit_skb(): look at the path to bumping
LINUX_MIB_TCPLOSSPROBES. I was thinking that later on we might want to
add a tcp:tcp_send_tlp tracepoint, in addition to this
tcp:tcp_retransmit_skb tracepoint, for investigating the same kind of
issues: packet loss. This existing tcp:tcp_retransmit_skb tracepoint
patch is ok.
Acked-by: Brendan Gregg <bgregg@netflix.com>
(with or without %pI6c)
Brendan
^ permalink raw reply
* Re: [PATCH net-next] ipv6: only update __use and lastusetime once per jiffy at most
From: Martin KaFai Lau @ 2017-10-14 0:09 UTC (permalink / raw)
To: Wei Wang; +Cc: David Miller, netdev, Eric Dumazet, Paolo Abeni
In-Reply-To: <20171013220807.90366-1-tracywwnj@gmail.com>
On Fri, Oct 13, 2017 at 10:08:07PM +0000, Wei Wang wrote:
> From: Wei Wang <weiwan@google.com>
>
> In order to not dirty the cacheline too often, we try to only update
> dst->__use and dst->lastusetime at most once per jiffy.
> As dst->lastusetime is only used by ipv6 garbage collector, it should
> be good enough time resolution.
Make sense.
> And __use is only used in ipv6_route_seq_show() to show how many times a
> dst has been used. And as __use is not atomic_t right now, it does not
> show the precise number of usage times anyway. So we think it should be
> OK to only update it at most once per jiffy.
If __use is only bumped HZ number of times per second and we can do ~3Mpps now,
would __use be way off?
^ permalink raw reply
* Re: [net-next RFC 0/4] Openvswitch meter action
From: Pravin Shelar @ 2017-10-14 0:09 UTC (permalink / raw)
To: Andy Zhou; +Cc: Linux Kernel Network Developers, Joe Stringer, Greg Rose
In-Reply-To: <1507847923-13612-1-git-send-email-azhou@ovn.org>
On Thu, Oct 12, 2017 at 3:38 PM, Andy Zhou <azhou@ovn.org> wrote:
> This patch series is the first attempt to add openvswitch
> meter support. We have previously experimented with adding
> metering support in nftables. However 1) It was not clear
> how to expose a named nftables object cleanly, and 2)
> the logic that implements metering is quite small, < 100 lines
> of code.
>
> With those two observations, it seems cleaner to add meter
> support in the openvswitch module directly.
>
>
Thanks for working on this feature. It looks good to me. I have couple
of comments inlined.
> Andy Zhou (4):
> openvswitch: Add meter netlink definitions
> openvswitch: export get_dp() API.
> openvswitch: Add meter infrastructure
> openvswitch: Add meter action support
>
> include/uapi/linux/openvswitch.h | 52 ++++
> net/openvswitch/Makefile | 1 +
> net/openvswitch/actions.c | 12 +
> net/openvswitch/datapath.c | 43 +--
> net/openvswitch/datapath.h | 35 +++
> net/openvswitch/flow_netlink.c | 6 +
> net/openvswitch/meter.c | 611 +++++++++++++++++++++++++++++++++++++++
> net/openvswitch/meter.h | 54 ++++
> 8 files changed, 783 insertions(+), 31 deletions(-)
> create mode 100644 net/openvswitch/meter.c
> create mode 100644 net/openvswitch/meter.h
>
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: [net-next RFC 3/4] openvswitch: Add meter infrastructure
From: Pravin Shelar @ 2017-10-14 0:12 UTC (permalink / raw)
To: Andy Zhou; +Cc: Linux Kernel Network Developers, Joe Stringer, Greg Rose
In-Reply-To: <1507847923-13612-4-git-send-email-azhou@ovn.org>
On Thu, Oct 12, 2017 at 3:38 PM, Andy Zhou <azhou@ovn.org> wrote:
> OVS kernel datapath so far does not support Openflow meter action.
> This is the first stab at adding kernel datapath meter support.
> This implementation supports only drop band type.
>
> Signed-off-by: Andy Zhou <azhou@ovn.org>
> ---
> net/openvswitch/Makefile | 1 +
> net/openvswitch/datapath.c | 14 +-
> net/openvswitch/datapath.h | 3 +
> net/openvswitch/meter.c | 611 +++++++++++++++++++++++++++++++++++++++++++++
> net/openvswitch/meter.h | 54 ++++
> 5 files changed, 681 insertions(+), 2 deletions(-)
> create mode 100644 net/openvswitch/meter.c
> create mode 100644 net/openvswitch/meter.h
>
...
> diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c
> new file mode 100644
> index 000000000000..f24ebb5f7af4
> --- /dev/null
> +++ b/net/openvswitch/meter.c
....
....
> +static int ovs_meter_cmd_features(struct sk_buff *skb, struct genl_info *info)
> +{
> + struct datapath *dp;
> + struct ovs_header *ovs_header = info->userhdr;
> + struct sk_buff *reply;
> + struct ovs_header *ovs_reply_header;
> + struct nlattr *nla, *band_nla;
> + int err;
> +
> + /* Check that the datapath exists */
> + ovs_lock();
> + dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
> + ovs_unlock();
> + if (!dp)
> + return -ENODEV;
> +
why dp check is required for this API?
> + reply = ovs_meter_cmd_reply_start(info, OVS_METER_CMD_FEATURES,
> + &ovs_reply_header);
> + if (!reply)
> + return PTR_ERR(reply);
> +
> + if (nla_put_u32(reply, OVS_METER_ATTR_MAX_METERS, U32_MAX) ||
> + nla_put_u32(reply, OVS_METER_ATTR_MAX_BANDS, DP_MAX_BANDS))
> + goto nla_put_failure;
> +
> + nla = nla_nest_start(reply, OVS_METER_ATTR_BANDS);
> + if (!nla)
> + goto nla_put_failure;
> +
> + band_nla = nla_nest_start(reply, OVS_BAND_ATTR_UNSPEC);
> + if (!band_nla)
> + goto nla_put_failure;
> + /* Currently only DROP band type is supported. */
> + if (nla_put_u32(reply, OVS_BAND_ATTR_TYPE, OVS_METER_BAND_TYPE_DROP))
> + goto nla_put_failure;
> + nla_nest_end(reply, band_nla);
> + nla_nest_end(reply, nla);
> +
> + genlmsg_end(reply, ovs_reply_header);
> + return genlmsg_reply(reply, info);
> +
> +nla_put_failure:
> + nlmsg_free(reply);
> + err = -EMSGSIZE;
> + return err;
> +}
> +
....
> +static int ovs_meter_cmd_set(struct sk_buff *skb, struct genl_info *info)
> +{
> + struct nlattr **a = info->attrs;
> + struct dp_meter *meter, *old_meter;
> + struct sk_buff *reply;
> + struct ovs_header *ovs_reply_header;
> + struct ovs_header *ovs_header = info->userhdr;
> + struct datapath *dp;
> + int err;
> + u32 meter_id;
> + bool failed;
> +
> + meter = dp_meter_create(a);
> + if (IS_ERR_OR_NULL(meter))
> + return PTR_ERR(meter);
> +
> + reply = ovs_meter_cmd_reply_start(info, OVS_METER_CMD_SET,
> + &ovs_reply_header);
> + if (IS_ERR(reply)) {
> + err = PTR_ERR(reply);
> + goto exit_free_meter;
> + }
> +
> + ovs_lock();
> + dp = get_dp(sock_net(skb->sk), ovs_header->dp_ifindex);
> + if (!dp) {
> + err = -ENODEV;
> + goto exit_unlock;
> + }
> +
> + if (!a[OVS_METER_ATTR_ID]) {
> + err = -ENODEV;
> + goto exit_unlock;
> + }
> +
> + meter_id = nla_get_u32(a[OVS_METER_ATTR_ID]);
> +
> + /* Cannot fail after this. */
> + old_meter = lookup_meter(dp, meter_id);
> + attach_meter(dp, meter);
> + ovs_unlock();
> +
After the unlock, it is not safe to keep the ref to old_meter. better
to release lock at the end. we could optimize it later if required.
> + /* Build response with the meter_id and stats from
> + * the old meter, if any.
> + */
> + failed = nla_put_u32(reply, OVS_METER_ATTR_ID, meter_id);
> + WARN_ON(failed);
> + if (old_meter) {
> + spin_lock_bh(&old_meter->lock);
> + if (old_meter->keep_stats) {
> + err = ovs_meter_cmd_reply_stats(reply, meter_id,
> + old_meter);
> + WARN_ON(err);
> + }
> + spin_unlock_bh(&old_meter->lock);
> + ovs_meter_free(old_meter);
> + }
> +
> + genlmsg_end(reply, ovs_reply_header);
> + return genlmsg_reply(reply, info);
> +
> +exit_unlock:
> + ovs_unlock();
> + nlmsg_free(reply);
> +exit_free_meter:
> + kfree(meter);
> + return err;
> +}
> +
....
> +bool ovs_meter_execute(struct datapath *dp, struct sk_buff *skb,
> + struct sw_flow_key *key, u32 meter_id)
> +{
> + struct dp_meter *meter;
> + struct dp_meter_band *band;
> + long long int now_ms = ktime_get_ns() / 1000 / 1000;
> + long long int long_delta_ms;
> + u32 delta_ms;
> + u32 cost;
> + int i, band_exceeded_max = -1;
> + u32 band_exceeded_rate = 0;
> +
> + meter = lookup_meter(dp, meter_id);
> + /* Do not drop the packet when there is no meter. */
> + if (!meter)
> + return false;
> +
> + /* Lock the meter while using it. */
> + spin_lock(&meter->lock);
> +
> + long_delta_ms = (now_ms - meter->used); /* ms */
> +
> + /* Make sure delta_ms will not be too large, so that bucket will not
> + * wrap around below.
> + */
> + delta_ms = (long_delta_ms > (long long int)meter->max_delta_t)
> + ? meter->max_delta_t : (u32)long_delta_ms;
> +
> + /* Update meter statistics.
> + */
> + meter->used = now_ms;
> + meter->stats.n_packets += 1;
> + meter->stats.n_bytes += skb->len;
> +
> + /* Bucket rate is either in kilobits per second, or in packets per
> + * second. We maintain the bucket in the units of either bits or
> + * 1/1000th of a packet, correspondingly.
> + * Then, when rate is multiplied with milliseconds, we get the
> + * bucket units:
> + * msec * kbps = bits, and
> + * msec * packets/sec = 1/1000 packets.
> + *
> + * 'cost' is the number of bucket units in this packet.
> + */
> + cost = (meter->kbps) ? skb->len * 8 : 1000;
> +
> + /* Update all bands and find the one hit with the highest rate. */
> + for (i = 0; i < meter->n_bands; ++i) {
> + long long int max_bucket_size;
> +
> + band = &meter->bands[i];
> + max_bucket_size = (band->burst_size + band->rate) * 1000;
> +
> + band->bucket += delta_ms * band->rate;
> + if (band->bucket > max_bucket_size)
> + band->bucket = max_bucket_size;
> +
> + if (band->bucket >= cost) {
> + band->bucket -= cost;
> + } else if (band->rate > band_exceeded_rate) {
> + band_exceeded_rate = band->rate;
> + band_exceeded_max = i;
> + }
> + }
> +
> + spin_unlock(&meter->lock);
> +
> + if (band_exceeded_max >= 0) {
> + /* Update band statistics. */
> + band = &meter->bands[band_exceeded_max];
> + band->stats.n_packets += 1;
> + band->stats.n_bytes += skb->len;
> +
Is it safe to do outside of the sipinlock?
> + /* Drop band triggered, let the caller drop the 'skb'. */
> + if (band->type == OVS_METER_BAND_TYPE_DROP)
> + return true;
> + }
> +
> + return false;
> +}
> +
^ permalink raw reply
* Re: [net-next RFC 4/4] openvswitch: Add meter action support
From: Pravin Shelar @ 2017-10-14 0:13 UTC (permalink / raw)
To: Andy Zhou; +Cc: Linux Kernel Network Developers, Joe Stringer, Greg Rose
In-Reply-To: <1507847923-13612-5-git-send-email-azhou@ovn.org>
On Thu, Oct 12, 2017 at 3:38 PM, Andy Zhou <azhou@ovn.org> wrote:
> Implements OVS kernel meter action support.
>
> Signed-off-by: Andy Zhou <azhou@ovn.org>
> ---
> include/uapi/linux/openvswitch.h | 1 +
> net/openvswitch/actions.c | 12 ++++++++++++
> net/openvswitch/datapath.h | 1 +
> net/openvswitch/flow_netlink.c | 6 ++++++
> 4 files changed, 20 insertions(+)
>
> diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h
> index 325049a129e4..11fe1a06cdd6 100644
> --- a/include/uapi/linux/openvswitch.h
> +++ b/include/uapi/linux/openvswitch.h
> @@ -835,6 +835,7 @@ enum ovs_action_attr {
> OVS_ACTION_ATTR_TRUNC, /* u32 struct ovs_action_trunc. */
> OVS_ACTION_ATTR_PUSH_ETH, /* struct ovs_action_push_eth. */
> OVS_ACTION_ATTR_POP_ETH, /* No argument. */
> + OVS_ACTION_ATTR_METER, /* u32 meter ID. */
>
> __OVS_ACTION_ATTR_MAX, /* Nothing past this will be accepted
> * from userspace. */
> diff --git a/net/openvswitch/actions.c b/net/openvswitch/actions.c
> index a54a556fcdb5..4eb160ac5a27 100644
> --- a/net/openvswitch/actions.c
> +++ b/net/openvswitch/actions.c
> @@ -1210,6 +1210,12 @@ static int do_execute_actions(struct datapath *dp, struct sk_buff *skb,
> case OVS_ACTION_ATTR_POP_ETH:
> err = pop_eth(skb, key);
> break;
> +
> + case OVS_ACTION_ATTR_METER:
> + if (ovs_meter_execute(dp, skb, key, nla_get_u32(a))) {
> + consume_skb(skb);
> + return 0;
> + }
> }
>
> if (unlikely(err)) {
> @@ -1341,6 +1347,12 @@ int ovs_execute_actions(struct datapath *dp, struct sk_buff *skb,
> err = do_execute_actions(dp, skb, key,
> acts->actions, acts->actions_len);
>
> + /* OVS action has dropped the packet, do not expose it
> + * to the user.
> + */
> + if (err == -ENODATA)
> + err = 0;
> +
I am not sure who is returning this error code?
^ permalink raw reply
* [next-queue PATCH v8 0/6] TSN: Add qdisc based config interface for CBS
From: Vinicius Costa Gomes @ 2017-10-14 0:25 UTC (permalink / raw)
To: netdev, intel-wired-lan
Cc: Vinicius Costa Gomes, jhs, xiyou.wangcong, jiri, andre.guedes,
ivan.briano, jesus.sanchez-palencia, boon.leong.ong,
richardcochran, henrik, levipearson, rodney.cummings
Hi,
Changes from v8:
- Fixed comments from Eric Dumazet and Ivan Khoronzhuk;
Changes since v6:
- Fixed compilation for 32bit arches;
- Aligned the behaviour of .select_queue() of the mq qdisc to be the
same as mqprio;
Changes since v5:
- Fixed comments from Jiri Pirko;
Changes since v4:
- Added a software implementation of the CBS algorithm;
Changes since v3:
- None, only a clean patchset without old patches;
Changes since v2:
- squashed the patch introducing the userspace API into the patch
implementing CBS;
Changes since v1:
- Solved the mqprio dependency;
- Fixed a mqprio bug, that caused the inner qdisc to have a wrong
dev_queue associated with it;
Changes from the RFC:
- Fixed comments from Henrik Austad;
- Simplified the Qdisc, using the generic implementation of callbacks
where possible;
- Small refactor on the driver (igb) code;
This patchset is a proposal of how the Traffic Control subsystem can
be used to offload the configuration of the Credit Based Shaper
(defined in the IEEE 802.1Q-2014 Section 8.6.8.2) into supported
network devices.
As part of this work, we've assessed previous public discussions
related to TSN enabling: patches from Henrik Austad (Cisco), the
presentation from Eric Mann at Linux Plumbers 2012, patches from
Gangfeng Huang (National Instruments) and the current state of the
OpenAVNU project (https://github.com/AVnu/OpenAvnu/).
Overview
========
Time-sensitive Networking (TSN) is a set of standards that aim to
address resources availability for providing bandwidth reservation and
bounded latency on Ethernet based LANs. The proposal described here
aims to cover mainly what is needed to enable the following standards:
802.1Qat and 802.1Qav.
The initial target of this work is the Intel i210 NIC, but other
controllers' datasheet were also taken into account, like the Renesas
RZ/A1H RZ/A1M group and the Synopsis DesignWare Ethernet QoS
controller.
Proposal
========
Feature-wise, what is covered here is the configuration interfaces for
HW implementations of the Credit-Based shaper (CBS, 802.1Qav). CBS is
a per-queue shaper. Given that this feature is related to traffic
shaping, and that the traffic control subsystem already provides a
queueing discipline that offloads config into the device driver (i.e.
mqprio), designing a new qdisc for the specific purpose of offloading
the config for the CBS shaper seemed like a good fit.
For steering traffic into the correct queues, we use the socket option
SO_PRIORITY and then a mechanism to map priority to traffic classes /
Tx queues. The qdisc mqprio is currently used in our tests.
As for the CBS config interface, this patchset is proposing a new
qdisc called 'cbs'. Its 'tc' cmd line is:
$ tc qdisc add dev IFACE parent ID cbs locredit N hicredit M sendslope S \
idleslope I
Note that the parameters for this qdisc are the ones defined by the
802.1Q-2014 spec, so no hardware specific functionality is exposed here.
Per-stream shaping, as defined by IEEE 802.1Q-2014 Section 34.6.1, is
not yet covered by this proposal.
Testing this RFC
================
Attached to this cover letter are:
- calculate_cbs_params.py: A Python script to calculate the
parameters to the CBS queueing discipline;
- tsn-talker.c: A sample C implementation of the talker side of a stream;
- tsn-listener.c: A sample C implementation of the listener side of a
stream;
For testing the patches of this series, you may want to use the
attached samples to this cover letter and use the 'mqprio' qdisc to
setup the priorities to Tx queues mapping, together with the 'cbs'
qdisc to configure the HW shaper of the i210 controller:
1) Setup priorities to traffic classes to hardware queues mapping
$ tc qdisc replace dev ens4 handle 100: parent root mqprio num_tc 3 \
map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 queues 1@0 1@1 2@2 hw 0
For a more detailed explanation, see mqprio(8), in short, this command
will map traffic with priority 3 to the hardware queue 0, traffic with
priority 2 to hardware queue 1, and the rest will be mapped to
hardware queues 2 and 3.
2) Check scheme. You want to get the inner qdiscs ID from the bottom up
$ tc -g class show dev ens4
Ex.:
+---(100:3) mqprio
| +---(100:6) mqprio
| +---(100:7) mqprio
|
+---(100:2) mqprio
| +---(100:5) mqprio
|
+---(100:1) mqprio
+---(100:4) mqprio
* Here '100:4' is Tx Queue #0 and '100:5' is Tx Queue #1.
3) Calculate CBS parameters for classes A and B. i.e. BW for A is 20Mbps and
for B is 10Mbps:
$ calc_cbs_params.py -A 20000 -a 1500 -B 10000 -b 1500
4) Configure CBS for traffic class A (priority 3) as provided by the script:
$ tc qdisc replace dev ens4 parent 100:4 cbs locredit -1470 \
hicredit 30 sendslope -980000 idleslope 20000
5) Configure CBS for traffic class B (priority 2):
$ tc qdisc replace dev ens4 parent 100:5 cbs \
locredit -1485 hicredit 31 sendslope -990000 idleslope 10000
6) Run Listener:
$ ./tsn-listener -d 01:AA:AA:AA:AA:AA -i ens4 -s 1500
7) Run Talker for class A (prio 3 here), compiled from samples/tsn/talker.c
$ ./tsn-talker -d 01:AA:AA:AA:AA:AA -i ens4 -p 3 -s 1500
* The bandwidth displayed on the listener output at this stage should be very
close to the one configured for class A.
8) You can also run a Talker for class B (prio 2 here and using a
different address):
$ ./tsn-talker -d 01:BB:BB:BB:BB:BB -i ens4 -p 2 -s 1500
Authors
=======
- Andre Guedes <andre.guedes@intel.com>
- Ivan Briano <ivan.briano@intel.com>
- Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
- Vinicius Gomes <vinicius.gomes@intel.com>
Andre Guedes (1):
igb: Add support for CBS offload
Jesus Sanchez-Palencia (3):
net/sched: Check for null dev_queue on create flow
net/sched: Change behavior of mq select_queue()
net/sched: Add select_queue() class_ops for mqprio
Vinicius Costa Gomes (2):
net/sched: Introduce Credit Based Shaper (CBS) qdisc
net/sched: Add support for HW offloading for CBS
drivers/net/ethernet/intel/igb/e1000_defines.h | 23 ++
drivers/net/ethernet/intel/igb/e1000_regs.h | 8 +
drivers/net/ethernet/intel/igb/igb.h | 6 +
drivers/net/ethernet/intel/igb/igb_main.c | 347 +++++++++++++++++++++++
include/linux/netdevice.h | 1 +
include/net/pkt_sched.h | 9 +
include/uapi/linux/pkt_sched.h | 18 ++
net/sched/Kconfig | 11 +
net/sched/Makefile | 1 +
net/sched/sch_cbs.c | 373 +++++++++++++++++++++++++
net/sched/sch_generic.c | 8 +-
net/sched/sch_mq.c | 10 +-
net/sched/sch_mqprio.c | 7 +
13 files changed, 812 insertions(+), 10 deletions(-)
create mode 100644 net/sched/sch_cbs.c
Annex: Sample files
===================
calc_cbs_params.py
--8<---------------cut here---------------start------------->8---
#!/usr/bin/env python
#
# Copyright (c) 2017, Intel Corporation
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are met:
#
# * Redistributions of source code must retain the above copyright notice,
# this list of conditions and the following disclaimer.
# * Redistributions in binary form must reproduce the above copyright
# notice, this list of conditions and the following disclaimer in the
# documentation and/or other materials provided with the distribution.
# * Neither the name of Intel Corporation nor the names of its contributors
# may be used to endorse or promote products derived from this software
# without specific prior written permission.
#
# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
# DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE
# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
# SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
# CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
# OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
import argparse
import math
def print_cbs_params_for_class_a(args):
idleslope = args.idleslope_a
sendslope = idleslope - args.link_speed
# According to 802.1Q-2014 spec, Annex L, hiCredit and
# loCredit for SR class A are calculated following the
# equations L-10 and L-12, respectively.
hicredit = math.ceil(idleslope * args.frame_non_sr / args.link_speed)
locredit = math.ceil(sendslope * args.frame_a / args.link_speed)
print("tc qdisc add dev <IFNAME> parent <QDISC-ID> cbs idleslope %d sendslope %d hicredit %d locredit %d" % \
(idleslope, sendslope, hicredit, locredit))
def print_cbs_params_for_class_b(args):
idleslope = args.idleslope_b
sendslope = idleslope - args.link_speed
# Annex L doesn't present a straightforward equation to
# calculate hiCredit for Class B so we have to derive it
# based on generic equations presented in that Annex.
#
# L-3 is the primary equation to calculate hiCredit. Section
# L.2 states that the 'maxInterferenceSize' for SR class B
# is the maximum burst size for SR class A plus the
# maxInterferenceSize from SR class A (which is equal to the
# maximum frame from non-SR traffic).
#
# The maximum burst size for SR class A equation is shown in
# L-16. Merging L-16 into L-3 we get the resulting equation
# which calculates hiCredit B (refer to section L.3 in case
# you're not familiar with the legend):
#
# hiCredit B = Rb * ( Mo Ma )
# ---------- + ------
# Ro - Ra Ro
#
hicredit = math.ceil(idleslope * \
((args.frame_non_sr / (args.link_speed - args.idleslope_a)) + \
(args.frame_a / args.link_speed)))
# loCredit B is calculated following equation L-2.
locredit = math.ceil(sendslope * args.frame_b / args.link_speed)
print("tc qdisc add dev <IFNAME> parent <QDISC-ID> cbs idleslope %d sendslope %d hicredit %d locredit %d" % \
(idleslope, sendslope, hicredit, locredit))
def main():
parser = argparse.ArgumentParser()
parser.add_argument('-S', dest='link_speed', default=1000000.0, type=float,
help='Link speed in kbps')
parser.add_argument('-s', dest='frame_non_sr', default=1500.0, type=float,
help='Maximum frame size from non-SR traffic (MTU size'
'usually')
parser.add_argument('-A', dest='idleslope_a', default=0, type=float,
help='Idleslope for SR class A in kbps')
parser.add_argument('-a', dest='frame_a', default=0, type=float,
help='Maximum frame size for SR class A traffic')
parser.add_argument('-B', dest='idleslope_b', default=0, type=float,
help='Idleslope for SR class B in kbps')
parser.add_argument('-b', dest='frame_b', default=0, type=float,
help='Maximum frame size for SR class B traffic')
args = parser.parse_args()
if args.idleslope_a > 0:
print_cbs_params_for_class_a(args)
if args.idleslope_b > 0:
print_cbs_params_for_class_b(args)
if __name__ == "__main__":
main()
--8<---------------cut here---------------end--------------->8---
tsn-talker.c
--8<---------------cut here---------------start------------->8---
/*
* Copyright (c) 2017, Intel Corporation
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
*
* * Redistributions of source code must retain the above copyright notice,
* this list of conditions and the following disclaimer.
* * Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
* * Neither the name of Intel Corporation nor the names of its contributors
* may be used to endorse or promote products derived from this software
* without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
* FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
* COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
* SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
* STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
* OF THE POSSIBILITY OF SUCH DAMAGE.
*/
#include <alloca.h>
#include <argp.h>
#include <arpa/inet.h>
#include <inttypes.h>
#include <linux/if.h>
#include <linux/if_ether.h>
#include <linux/if_packet.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
#include <unistd.h>
#define MAGIC 0xCC
static uint8_t ifname[IFNAMSIZ];
static uint8_t macaddr[ETH_ALEN];
static int priority = -1;
static size_t size = 1500;
static uint64_t seq;
static int delay = -1;
static struct argp_option options[] = {
{"dst-addr", 'd', "MACADDR", 0, "Stream Destination MAC address" },
{"delay", 'D', "NUM", 0, "Delay (in us) between packet transmission" },
{"ifname", 'i', "IFNAME", 0, "Network Interface" },
{"prio", 'p', "NUM", 0, "SO_PRIORITY to be set in socket" },
{"packet-size", 's', "NUM", 0, "Size of packets to be transmitted" },
{ 0 }
};
static error_t parser(int key, char *arg, struct argp_state *state)
{
int res;
switch (key) {
case 'd':
res = sscanf(arg, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
&macaddr[0], &macaddr[1], &macaddr[2],
&macaddr[3], &macaddr[4], &macaddr[5]);
if (res != 6) {
printf("Invalid address\n");
exit(EXIT_FAILURE);
}
break;
case 'D':
delay = atoi(arg);
break;
case 'i':
strncpy(ifname, arg, sizeof(ifname) - 1);
break;
case 'p':
priority = atoi(arg);
break;
case 's':
size = atoi(arg);
break;
}
return 0;
}
static struct argp argp = { options, parser };
int main(int argc, char *argv[])
{
int fd, res;
struct ifreq req;
uint8_t *data;
struct sockaddr_ll sk_addr = {
.sll_family = AF_PACKET,
.sll_protocol = htons(ETH_P_TSN),
.sll_halen = ETH_ALEN,
};
argp_parse(&argp, argc, argv, 0, NULL, NULL);
fd = socket(AF_PACKET, SOCK_DGRAM, htons(ETH_P_TSN));
if (fd < 0) {
perror("Couldn't open socket");
return 1;
}
strncpy(req.ifr_name, ifname, sizeof(req.ifr_name));
res = ioctl(fd, SIOCGIFINDEX, &req);
if (res < 0) {
perror("Couldn't get interface index");
goto err;
}
sk_addr.sll_ifindex = req.ifr_ifindex;
memcpy(&sk_addr.sll_addr, macaddr, ETH_ALEN);
if (priority != -1) {
res = setsockopt(fd, SOL_SOCKET, SO_PRIORITY, &priority,
sizeof(priority));
if (res < 0) {
perror("Couldn't set priority");
goto err;
}
}
data = alloca(size);
memset(data, MAGIC, size);
printf("Sending packets...\n");
while (1) {
uint64_t *seq_ptr = (uint64_t *) &data[0];
ssize_t n;
*seq_ptr = seq++;
n = sendto(fd, data, size, 0, (struct sockaddr *) &sk_addr,
sizeof(sk_addr));
if (n < 0)
perror("Failed to send data");
if (delay > 0)
usleep(delay);
}
close(fd);
return 0;
err:
close(fd);
return 1;
}
--8<---------------cut here---------------end--------------->8---
tsn-listener.c
--8<---------------cut here---------------start------------->8---
/*
* Copyright (c) 2017, Intel Corporation
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are met:
*
* * Redistributions of source code must retain the above copyright notice,
* this list of conditions and the following disclaimer.
* * Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
* * Neither the name of Intel Corporation nor the names of its contributors
* may be used to endorse or promote products derived from this software
* without specific prior written permission.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
* FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
* COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
* (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
* SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
* HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT,
* STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE)
* ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED
* OF THE POSSIBILITY OF SUCH DAMAGE.
*/
#include <alloca.h>
#include <argp.h>
#include <arpa/inet.h>
#include <inttypes.h>
#include <linux/if.h>
#include <linux/if_ether.h>
#include <linux/if_packet.h>
#include <poll.h>
#include <stdbool.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/ioctl.h>
#include <sys/timerfd.h>
#include <unistd.h>
static uint8_t ifname[IFNAMSIZ];
static uint8_t macaddr[ETH_ALEN];
static uint64_t data_count;
static int size = 1500;
static time_t interval = 1;
static bool check_seq = false;
static uint64_t expected_seq;
static struct argp_option options[] = {
{"check-seq", 'c', NULL, 0, "Check sequence number within packet" },
{"dst-addr", 'd', "MACADDR", 0, "Stream Destination MAC address" },
{"ifname", 'i', "IFNAME", 0, "Network Interface" },
{"interval", 'I', "SEC", 0, "Interval between bandwidth reports" },
{"packet-size", 's', "NUM", 0, "Expected packet size" },
{ 0 }
};
static error_t parser(int key, char *arg, struct argp_state *state)
{
int res;
switch (key) {
case 'c':
check_seq = true;
break;
case 'd':
res = sscanf(arg, "%hhx:%hhx:%hhx:%hhx:%hhx:%hhx",
&macaddr[0], &macaddr[1], &macaddr[2],
&macaddr[3], &macaddr[4], &macaddr[5]);
if (res != 6) {
printf("Invalid address\n");
exit(EXIT_FAILURE);
}
break;
case 'i':
strncpy(ifname, arg, sizeof(ifname) - 1);
break;
case 'I':
interval = atoi(arg);
break;
case 's':
size = atoi(arg);
break;
}
return 0;
}
static struct argp argp = { options, parser };
static int setup_timer(void)
{
int fd, res;
struct itimerspec tspec = { 0 };
fd = timerfd_create(CLOCK_MONOTONIC, 0);
if (fd < 0) {
perror("Couldn't create timer");
return -1;
}
tspec.it_value.tv_sec = interval;
tspec.it_interval.tv_sec = interval;
res = timerfd_settime(fd, 0, &tspec, NULL);
if (res < 0) {
perror("Couldn't set timer");
close(fd);
return -1;
}
return fd;
}
static int setup_socket(void)
{
int fd, res;
struct sockaddr_ll sk_addr = {
.sll_family = AF_PACKET,
.sll_protocol = htons(ETH_P_TSN),
};
fd = socket(AF_PACKET, SOCK_DGRAM, htons(ETH_P_TSN));
if (fd < 0) {
perror("Couldn't open socket");
return -1;
}
/* If user provided a network interface, bind() to it. */
if (ifname[0] != '\0') {
struct ifreq req;
strncpy(req.ifr_name, ifname, sizeof(req.ifr_name));
res = ioctl(fd, SIOCGIFINDEX, &req);
if (res < 0) {
perror("Couldn't get interface index");
goto err;
}
sk_addr.sll_ifindex = req.ifr_ifindex;
res = bind(fd, (struct sockaddr *) &sk_addr, sizeof(sk_addr));
if (res < 0) {
perror("Couldn't bind() to interface");
goto err;
}
}
/* If user provided the stream destination address, set it as multicast
* address.
*/
if (macaddr[0] != '\0') {
struct packet_mreq mreq;
mreq.mr_ifindex = sk_addr.sll_ifindex;
mreq.mr_type = PACKET_MR_MULTICAST;
mreq.mr_alen = ETH_ALEN;
memcpy(&mreq.mr_address, macaddr, ETH_ALEN);
res = setsockopt(fd, SOL_PACKET, PACKET_ADD_MEMBERSHIP,
&mreq, sizeof(struct packet_mreq));
if (res < 0) {
perror("Couldn't set PACKET_ADD_MEMBERSHIP");
goto err;
}
}
return fd;
err:
close(fd);
return -1;
}
static void recv_packet(int fd)
{
uint8_t *data = alloca(size);
ssize_t n = recv(fd, data, size, 0);
if (n < 0) {
perror("Failed to receive data");
return;
}
if (n != size)
printf("Size mismatch: expected %d, got %d\n", size, n);
if (check_seq) {
uint64_t *seq = (uint64_t *) &data[0];
/* If 'expected_seq' is equal to zero, it means this is the
* first packet we received so we don't know what sequence
* number to expect.
*/
if (expected_seq == 0)
expected_seq = *seq;
if (*seq != expected_seq) {
printf("Sequence mismatch: expected %llu, got %llu\n",
expected_seq, *seq);
expected_seq = *seq;
}
expected_seq++;
}
data_count += n;
}
static void report_bw(int fd)
{
uint64_t expirations;
ssize_t n = read(fd, &expirations, sizeof(uint64_t));
if (n < 0) {
perror("Couldn't read timerfd");
return;
}
if (expirations != 1)
printf("Some went wrong with timerfd\n");
printf("Receiving data rate: %llu kbps\n", (data_count * 8) / (1000 * interval));
data_count = 0;
}
int main(int argc, char *argv[])
{
int sk_fd, timer_fd, res;
struct pollfd fds[2];
argp_parse(&argp, argc, argv, 0, NULL, NULL);
sk_fd = setup_socket();
if (sk_fd < 0)
return 1;
timer_fd = setup_timer();
if (timer_fd < 0) {
close(sk_fd);
return 1;
}
fds[0].fd = sk_fd;
fds[0].events = POLLIN;
fds[1].fd = timer_fd;
fds[1].events = POLLIN;
printf("Waiting for packets...\n");
while (1) {
res = poll(fds, 2, -1);
if (res < 0) {
perror("Error on poll()");
goto err;
}
if (fds[0].revents & POLLIN)
recv_packet(fds[0].fd);
if (fds[1].revents & POLLIN) {
report_bw(fds[1].fd);
}
}
close(timer_fd);
close(sk_fd);
return 0;
err:
close(timer_fd);
close(sk_fd);
return 1;
}
--8<---------------cut here---------------end--------------->8---
^ permalink raw reply
* [next-queue PATCH v8 2/6] net/sched: Change behavior of mq select_queue()
From: Vinicius Costa Gomes @ 2017-10-14 0:25 UTC (permalink / raw)
To: netdev, intel-wired-lan
Cc: Jesus Sanchez-Palencia, jhs, xiyou.wangcong, jiri, andre.guedes,
ivan.briano, boon.leong.ong, richardcochran, henrik, levipearson,
rodney.cummings
In-Reply-To: <20171014002534.19896-1-vinicius.gomes@intel.com>
From: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
Currently, the class_ops select_queue() implementation on sch_mq
returns a pointer to netdev_queue #0 when it receives and invalid
qdisc id. That can be misleading since all of mq's inner qdiscs are
attached to a valid netdev_queue.
Here we fix that by returning NULL when a qdisc id is invalid. This is
aligned with how select_queue() is implemented for sch_mqprio in the
next patch on this series, keeping a consistent behavior between these
two qdiscs.
Signed-off-by: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
---
net/sched/sch_mq.c | 10 +---------
1 file changed, 1 insertion(+), 9 deletions(-)
diff --git a/net/sched/sch_mq.c b/net/sched/sch_mq.c
index f3a3e507422b..213b586a06a0 100644
--- a/net/sched/sch_mq.c
+++ b/net/sched/sch_mq.c
@@ -130,15 +130,7 @@ static struct netdev_queue *mq_queue_get(struct Qdisc *sch, unsigned long cl)
static struct netdev_queue *mq_select_queue(struct Qdisc *sch,
struct tcmsg *tcm)
{
- unsigned int ntx = TC_H_MIN(tcm->tcm_parent);
- struct netdev_queue *dev_queue = mq_queue_get(sch, ntx);
-
- if (!dev_queue) {
- struct net_device *dev = qdisc_dev(sch);
-
- return netdev_get_tx_queue(dev, 0);
- }
- return dev_queue;
+ return mq_queue_get(sch, TC_H_MIN(tcm->tcm_parent));
}
static int mq_graft(struct Qdisc *sch, unsigned long cl, struct Qdisc *new,
--
2.14.2
^ permalink raw reply related
* [next-queue PATCH v8 1/6] net/sched: Check for null dev_queue on create flow
From: Vinicius Costa Gomes @ 2017-10-14 0:25 UTC (permalink / raw)
To: netdev, intel-wired-lan
Cc: Jesus Sanchez-Palencia, jhs, xiyou.wangcong, jiri, andre.guedes,
ivan.briano, boon.leong.ong, richardcochran, henrik, levipearson,
rodney.cummings
In-Reply-To: <20171014002534.19896-1-vinicius.gomes@intel.com>
From: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
In qdisc_alloc() the dev_queue pointer was used without any checks
being performed. If qdisc_create() gets a null dev_queue pointer, it
just passes it along to qdisc_alloc(), leading to a crash. That
happens if a root qdisc implements select_queue() and returns a null
dev_queue pointer for an "invalid handle", for example, or if the
dev_queue associated with the parent qdisc is null.
This patch is in preparation for the next in this series, where
select_queue() is being added to mqprio and as it may return a null
dev_queue.
Signed-off-by: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
---
net/sched/sch_generic.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/net/sched/sch_generic.c b/net/sched/sch_generic.c
index a0a198768aad..de2408f1ccd3 100644
--- a/net/sched/sch_generic.c
+++ b/net/sched/sch_generic.c
@@ -603,8 +603,14 @@ struct Qdisc *qdisc_alloc(struct netdev_queue *dev_queue,
struct Qdisc *sch;
unsigned int size = QDISC_ALIGN(sizeof(*sch)) + ops->priv_size;
int err = -ENOBUFS;
- struct net_device *dev = dev_queue->dev;
+ struct net_device *dev;
+
+ if (!dev_queue) {
+ err = -EINVAL;
+ goto errout;
+ }
+ dev = dev_queue->dev;
p = kzalloc_node(size, GFP_KERNEL,
netdev_queue_numa_node_read(dev_queue));
--
2.14.2
^ permalink raw reply related
* [next-queue PATCH v8 3/6] net/sched: Add select_queue() class_ops for mqprio
From: Vinicius Costa Gomes @ 2017-10-14 0:25 UTC (permalink / raw)
To: netdev, intel-wired-lan
Cc: Jesus Sanchez-Palencia, jhs, xiyou.wangcong, jiri, andre.guedes,
ivan.briano, boon.leong.ong, richardcochran, henrik, levipearson,
rodney.cummings
In-Reply-To: <20171014002534.19896-1-vinicius.gomes@intel.com>
From: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
When replacing a child qdisc from mqprio, tc_modify_qdisc() must fetch
the netdev_queue pointer that the current child qdisc is associated
with before creating the new qdisc.
Currently, when using mqprio as root qdisc, the kernel will end up
getting the queue #0 pointer from the mqprio (root qdisc), which leaves
any new child qdisc with a possibly wrong netdev_queue pointer.
Implementing the Qdisc_class_ops select_queue() on mqprio fixes this
issue and avoid an inconsistent state when child qdiscs are replaced.
Signed-off-by: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
---
net/sched/sch_mqprio.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/sched/sch_mqprio.c b/net/sched/sch_mqprio.c
index 6bcdfe6e7b63..8c042ae323e3 100644
--- a/net/sched/sch_mqprio.c
+++ b/net/sched/sch_mqprio.c
@@ -396,6 +396,12 @@ static void mqprio_walk(struct Qdisc *sch, struct qdisc_walker *arg)
}
}
+static struct netdev_queue *mqprio_select_queue(struct Qdisc *sch,
+ struct tcmsg *tcm)
+{
+ return mqprio_queue_get(sch, TC_H_MIN(tcm->tcm_parent));
+}
+
static const struct Qdisc_class_ops mqprio_class_ops = {
.graft = mqprio_graft,
.leaf = mqprio_leaf,
@@ -403,6 +409,7 @@ static const struct Qdisc_class_ops mqprio_class_ops = {
.walk = mqprio_walk,
.dump = mqprio_dump_class,
.dump_stats = mqprio_dump_class_stats,
+ .select_queue = mqprio_select_queue,
};
static struct Qdisc_ops mqprio_qdisc_ops __read_mostly = {
--
2.14.2
^ permalink raw reply related
* [next-queue PATCH v8 6/6] igb: Add support for CBS offload
From: Vinicius Costa Gomes @ 2017-10-14 0:25 UTC (permalink / raw)
To: netdev, intel-wired-lan
Cc: Andre Guedes, jhs, xiyou.wangcong, jiri, ivan.briano,
jesus.sanchez-palencia, boon.leong.ong, richardcochran, henrik,
levipearson, rodney.cummings
In-Reply-To: <20171014002534.19896-1-vinicius.gomes@intel.com>
From: Andre Guedes <andre.guedes@intel.com>
This patch adds support for Credit-Based Shaper (CBS) qdisc offload
from Traffic Control system. This support enable us to leverage the
Forwarding and Queuing for Time-Sensitive Streams (FQTSS) features
from Intel i210 Ethernet Controller. FQTSS is the former 802.1Qav
standard which was merged into 802.1Q in 2014. It enables traffic
prioritization and bandwidth reservation via the Credit-Based Shaper
which is implemented in hardware by i210 controller.
The patch introduces the igb_setup_tc() function which implements the
support for CBS qdisc hardware offload in the IGB driver. CBS offload
is the only traffic control offload supported by the driver at the
moment.
FQTSS transmission mode from i210 controller is automatically enabled
by the IGB driver when the CBS is enabled for the first hardware
queue. Likewise, FQTSS mode is automatically disabled when CBS is
disabled for the last hardware queue. Changing FQTSS mode requires NIC
reset.
FQTSS feature is supported by i210 controller only.
Signed-off-by: Andre Guedes <andre.guedes@intel.com>
---
drivers/net/ethernet/intel/igb/e1000_defines.h | 23 ++
drivers/net/ethernet/intel/igb/e1000_regs.h | 8 +
drivers/net/ethernet/intel/igb/igb.h | 6 +
drivers/net/ethernet/intel/igb/igb_main.c | 347 +++++++++++++++++++++++++
4 files changed, 384 insertions(+)
diff --git a/drivers/net/ethernet/intel/igb/e1000_defines.h b/drivers/net/ethernet/intel/igb/e1000_defines.h
index 1de82f247312..83cabff1e0ab 100644
--- a/drivers/net/ethernet/intel/igb/e1000_defines.h
+++ b/drivers/net/ethernet/intel/igb/e1000_defines.h
@@ -353,7 +353,18 @@
#define E1000_RXPBS_CFG_TS_EN 0x80000000
#define I210_RXPBSIZE_DEFAULT 0x000000A2 /* RXPBSIZE default */
+#define I210_RXPBSIZE_MASK 0x0000003F
+#define I210_RXPBSIZE_PB_32KB 0x00000020
#define I210_TXPBSIZE_DEFAULT 0x04000014 /* TXPBSIZE default */
+#define I210_TXPBSIZE_MASK 0xC0FFFFFF
+#define I210_TXPBSIZE_PB0_8KB (8 << 0)
+#define I210_TXPBSIZE_PB1_8KB (8 << 6)
+#define I210_TXPBSIZE_PB2_4KB (4 << 12)
+#define I210_TXPBSIZE_PB3_4KB (4 << 18)
+
+#define I210_DTXMXPKTSZ_DEFAULT 0x00000098
+
+#define I210_SR_QUEUES_NUM 2
/* SerDes Control */
#define E1000_SCTL_DISABLE_SERDES_LOOPBACK 0x0400
@@ -1051,4 +1062,16 @@
#define E1000_VLAPQF_P_VALID(_n) (0x1 << (3 + (_n) * 4))
#define E1000_VLAPQF_QUEUE_MASK 0x03
+/* TX Qav Control fields */
+#define E1000_TQAVCTRL_XMIT_MODE BIT(0)
+#define E1000_TQAVCTRL_DATAFETCHARB BIT(4)
+#define E1000_TQAVCTRL_DATATRANARB BIT(8)
+
+/* TX Qav Credit Control fields */
+#define E1000_TQAVCC_IDLESLOPE_MASK 0xFFFF
+#define E1000_TQAVCC_QUEUEMODE BIT(31)
+
+/* Transmit Descriptor Control fields */
+#define E1000_TXDCTL_PRIORITY BIT(27)
+
#endif
diff --git a/drivers/net/ethernet/intel/igb/e1000_regs.h b/drivers/net/ethernet/intel/igb/e1000_regs.h
index 58adbf234e07..8eee081d395f 100644
--- a/drivers/net/ethernet/intel/igb/e1000_regs.h
+++ b/drivers/net/ethernet/intel/igb/e1000_regs.h
@@ -421,6 +421,14 @@ do { \
#define E1000_I210_FLA 0x1201C
+#define E1000_I210_DTXMXPKTSZ 0x355C
+
+#define E1000_I210_TXDCTL(_n) (0x0E028 + ((_n) * 0x40))
+
+#define E1000_I210_TQAVCTRL 0x3570
+#define E1000_I210_TQAVCC(_n) (0x3004 + ((_n) * 0x40))
+#define E1000_I210_TQAVHC(_n) (0x300C + ((_n) * 0x40))
+
#define E1000_INVM_DATA_REG(_n) (0x12120 + 4*(_n))
#define E1000_INVM_SIZE 64 /* Number of INVM Data Registers */
diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 06ffb2bc713e..92845692087a 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -281,6 +281,11 @@ struct igb_ring {
u16 count; /* number of desc. in the ring */
u8 queue_index; /* logical index of the ring*/
u8 reg_idx; /* physical index of the ring */
+ bool cbs_enable; /* indicates if CBS is enabled */
+ s32 idleslope; /* idleSlope in kbps */
+ s32 sendslope; /* sendSlope in kbps */
+ s32 hicredit; /* hiCredit in bytes */
+ s32 locredit; /* loCredit in bytes */
/* everything past this point are written often */
u16 next_to_clean;
@@ -621,6 +626,7 @@ struct igb_adapter {
#define IGB_FLAG_EEE BIT(14)
#define IGB_FLAG_VLAN_PROMISC BIT(15)
#define IGB_FLAG_RX_LEGACY BIT(16)
+#define IGB_FLAG_FQTSS BIT(17)
/* Media Auto Sense */
#define IGB_MAS_ENABLE_0 0X0001
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 837d9b46a390..be2cf263efa9 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -34,6 +34,7 @@
#include <linux/slab.h>
#include <net/checksum.h>
#include <net/ip6_checksum.h>
+#include <net/pkt_sched.h>
#include <linux/net_tstamp.h>
#include <linux/mii.h>
#include <linux/ethtool.h>
@@ -62,6 +63,17 @@
#define BUILD 0
#define DRV_VERSION __stringify(MAJ) "." __stringify(MIN) "." \
__stringify(BUILD) "-k"
+
+enum queue_mode {
+ QUEUE_MODE_STRICT_PRIORITY,
+ QUEUE_MODE_STREAM_RESERVATION,
+};
+
+enum tx_queue_prio {
+ TX_QUEUE_PRIO_HIGH,
+ TX_QUEUE_PRIO_LOW,
+};
+
char igb_driver_name[] = "igb";
char igb_driver_version[] = DRV_VERSION;
static const char igb_driver_string[] =
@@ -1271,6 +1283,12 @@ static int igb_alloc_q_vector(struct igb_adapter *adapter,
ring->count = adapter->tx_ring_count;
ring->queue_index = txr_idx;
+ ring->cbs_enable = false;
+ ring->idleslope = 0;
+ ring->sendslope = 0;
+ ring->hicredit = 0;
+ ring->locredit = 0;
+
u64_stats_init(&ring->tx_syncp);
u64_stats_init(&ring->tx_syncp2);
@@ -1598,6 +1616,284 @@ static void igb_get_hw_control(struct igb_adapter *adapter)
ctrl_ext | E1000_CTRL_EXT_DRV_LOAD);
}
+static void enable_fqtss(struct igb_adapter *adapter, bool enable)
+{
+ struct net_device *netdev = adapter->netdev;
+ struct e1000_hw *hw = &adapter->hw;
+
+ WARN_ON(hw->mac.type != e1000_i210);
+
+ if (enable)
+ adapter->flags |= IGB_FLAG_FQTSS;
+ else
+ adapter->flags &= ~IGB_FLAG_FQTSS;
+
+ if (netif_running(netdev))
+ schedule_work(&adapter->reset_task);
+}
+
+static bool is_fqtss_enabled(struct igb_adapter *adapter)
+{
+ return (adapter->flags & IGB_FLAG_FQTSS) ? true : false;
+}
+
+static void set_tx_desc_fetch_prio(struct e1000_hw *hw, int queue,
+ enum tx_queue_prio prio)
+{
+ u32 val;
+
+ WARN_ON(hw->mac.type != e1000_i210);
+ WARN_ON(queue < 0 || queue > 4);
+
+ val = rd32(E1000_I210_TXDCTL(queue));
+
+ if (prio == TX_QUEUE_PRIO_HIGH)
+ val |= E1000_TXDCTL_PRIORITY;
+ else
+ val &= ~E1000_TXDCTL_PRIORITY;
+
+ wr32(E1000_I210_TXDCTL(queue), val);
+}
+
+static void set_queue_mode(struct e1000_hw *hw, int queue, enum queue_mode mode)
+{
+ u32 val;
+
+ WARN_ON(hw->mac.type != e1000_i210);
+ WARN_ON(queue < 0 || queue > 1);
+
+ val = rd32(E1000_I210_TQAVCC(queue));
+
+ if (mode == QUEUE_MODE_STREAM_RESERVATION)
+ val |= E1000_TQAVCC_QUEUEMODE;
+ else
+ val &= ~E1000_TQAVCC_QUEUEMODE;
+
+ wr32(E1000_I210_TQAVCC(queue), val);
+}
+
+/**
+ * igb_configure_cbs - Configure Credit-Based Shaper (CBS)
+ * @adapter: pointer to adapter struct
+ * @queue: queue number
+ * @enable: true = enable CBS, false = disable CBS
+ * @idleslope: idleSlope in kbps
+ * @sendslope: sendSlope in kbps
+ * @hicredit: hiCredit in bytes
+ * @locredit: loCredit in bytes
+ *
+ * Configure CBS for a given hardware queue. When disabling, idleslope,
+ * sendslope, hicredit, locredit arguments are ignored. Returns 0 if
+ * success. Negative otherwise.
+ **/
+static void igb_configure_cbs(struct igb_adapter *adapter, int queue,
+ bool enable, int idleslope, int sendslope,
+ int hicredit, int locredit)
+{
+ struct net_device *netdev = adapter->netdev;
+ struct e1000_hw *hw = &adapter->hw;
+ u32 tqavcc;
+ u16 value;
+
+ WARN_ON(hw->mac.type != e1000_i210);
+ WARN_ON(queue < 0 || queue > 1);
+
+ if (enable) {
+ set_tx_desc_fetch_prio(hw, queue, TX_QUEUE_PRIO_HIGH);
+ set_queue_mode(hw, queue, QUEUE_MODE_STREAM_RESERVATION);
+
+ /* According to i210 datasheet section 7.2.7.7, we should set
+ * the 'idleSlope' field from TQAVCC register following the
+ * equation:
+ *
+ * For 100 Mbps link speed:
+ *
+ * value = BW * 0x7735 * 0.2 (E1)
+ *
+ * For 1000Mbps link speed:
+ *
+ * value = BW * 0x7735 * 2 (E2)
+ *
+ * E1 and E2 can be merged into one equation as shown below.
+ * Note that 'link-speed' is in Mbps.
+ *
+ * value = BW * 0x7735 * 2 * link-speed
+ * -------------- (E3)
+ * 1000
+ *
+ * 'BW' is the percentage bandwidth out of full link speed
+ * which can be found with the following equation. Note that
+ * idleSlope here is the parameter from this function which
+ * is in kbps.
+ *
+ * BW = idleSlope
+ * ----------------- (E4)
+ * link-speed * 1000
+ *
+ * That said, we can come up with a generic equation to
+ * calculate the value we should set it TQAVCC register by
+ * replacing 'BW' in E3 by E4. The resulting equation is:
+ *
+ * value = idleSlope * 0x7735 * 2 * link-speed
+ * ----------------- -------------- (E5)
+ * link-speed * 1000 1000
+ *
+ * 'link-speed' is present in both sides of the fraction so
+ * it is canceled out. The final equation is the following:
+ *
+ * value = idleSlope * 61034
+ * ----------------- (E6)
+ * 1000000
+ */
+ value = DIV_ROUND_UP_ULL(idleslope * 61034ULL, 1000000);
+
+ tqavcc = rd32(E1000_I210_TQAVCC(queue));
+ tqavcc &= ~E1000_TQAVCC_IDLESLOPE_MASK;
+ tqavcc |= value;
+ wr32(E1000_I210_TQAVCC(queue), tqavcc);
+
+ wr32(E1000_I210_TQAVHC(queue), 0x80000000 + hicredit * 0x7735);
+ } else {
+ set_tx_desc_fetch_prio(hw, queue, TX_QUEUE_PRIO_LOW);
+ set_queue_mode(hw, queue, QUEUE_MODE_STRICT_PRIORITY);
+
+ /* Set idleSlope to zero. */
+ tqavcc = rd32(E1000_I210_TQAVCC(queue));
+ tqavcc &= ~E1000_TQAVCC_IDLESLOPE_MASK;
+ wr32(E1000_I210_TQAVCC(queue), tqavcc);
+
+ /* Set hiCredit to zero. */
+ wr32(E1000_I210_TQAVHC(queue), 0);
+ }
+
+ /* XXX: In i210 controller the sendSlope and loCredit parameters from
+ * CBS are not configurable by software so we don't do any 'controller
+ * configuration' in respect to these parameters.
+ */
+
+ netdev_dbg(netdev, "CBS %s: queue %d idleslope %d sendslope %d hiCredit %d locredit %d\n",
+ (enable) ? "enabled" : "disabled", queue,
+ idleslope, sendslope, hicredit, locredit);
+}
+
+static int igb_save_cbs_params(struct igb_adapter *adapter, int queue,
+ bool enable, int idleslope, int sendslope,
+ int hicredit, int locredit)
+{
+ struct igb_ring *ring;
+
+ if (queue < 0 || queue > adapter->num_tx_queues)
+ return -EINVAL;
+
+ ring = adapter->tx_ring[queue];
+
+ ring->cbs_enable = enable;
+ ring->idleslope = idleslope;
+ ring->sendslope = sendslope;
+ ring->hicredit = hicredit;
+ ring->locredit = locredit;
+
+ return 0;
+}
+
+static bool is_any_cbs_enabled(struct igb_adapter *adapter)
+{
+ struct igb_ring *ring;
+ int i;
+
+ for (i = 0; i < adapter->num_tx_queues; i++) {
+ ring = adapter->tx_ring[i];
+
+ if (ring->cbs_enable)
+ return true;
+ }
+
+ return false;
+}
+
+static void igb_setup_tx_mode(struct igb_adapter *adapter)
+{
+ struct net_device *netdev = adapter->netdev;
+ struct e1000_hw *hw = &adapter->hw;
+ u32 val;
+
+ /* Only i210 controller supports changing the transmission mode. */
+ if (hw->mac.type != e1000_i210)
+ return;
+
+ if (is_fqtss_enabled(adapter)) {
+ int i, max_queue;
+
+ /* Configure TQAVCTRL register: set transmit mode to 'Qav',
+ * set data fetch arbitration to 'round robin' and set data
+ * transfer arbitration to 'credit shaper algorithm.
+ */
+ val = rd32(E1000_I210_TQAVCTRL);
+ val |= E1000_TQAVCTRL_XMIT_MODE | E1000_TQAVCTRL_DATATRANARB;
+ val &= ~E1000_TQAVCTRL_DATAFETCHARB;
+ wr32(E1000_I210_TQAVCTRL, val);
+
+ /* Configure Tx and Rx packet buffers sizes as described in
+ * i210 datasheet section 7.2.7.7.
+ */
+ val = rd32(E1000_TXPBS);
+ val &= ~I210_TXPBSIZE_MASK;
+ val |= I210_TXPBSIZE_PB0_8KB | I210_TXPBSIZE_PB1_8KB |
+ I210_TXPBSIZE_PB2_4KB | I210_TXPBSIZE_PB3_4KB;
+ wr32(E1000_TXPBS, val);
+
+ val = rd32(E1000_RXPBS);
+ val &= ~I210_RXPBSIZE_MASK;
+ val |= I210_RXPBSIZE_PB_32KB;
+ wr32(E1000_RXPBS, val);
+
+ /* Section 8.12.9 states that MAX_TPKT_SIZE from DTXMXPKTSZ
+ * register should not exceed the buffer size programmed in
+ * TXPBS. The smallest buffer size programmed in TXPBS is 4kB
+ * so according to the datasheet we should set MAX_TPKT_SIZE to
+ * 4kB / 64.
+ *
+ * However, when we do so, no frame from queue 2 and 3 are
+ * transmitted. It seems the MAX_TPKT_SIZE should not be great
+ * or _equal_ to the buffer size programmed in TXPBS. For this
+ * reason, we set set MAX_ TPKT_SIZE to (4kB - 1) / 64.
+ */
+ val = (4096 - 1) / 64;
+ wr32(E1000_I210_DTXMXPKTSZ, val);
+
+ /* Since FQTSS mode is enabled, apply any CBS configuration
+ * previously set. If no previous CBS configuration has been
+ * done, then the initial configuration is applied, which means
+ * CBS is disabled.
+ */
+ max_queue = (adapter->num_tx_queues < I210_SR_QUEUES_NUM) ?
+ adapter->num_tx_queues : I210_SR_QUEUES_NUM;
+
+ for (i = 0; i < max_queue; i++) {
+ struct igb_ring *ring = adapter->tx_ring[i];
+
+ igb_configure_cbs(adapter, i, ring->cbs_enable,
+ ring->idleslope, ring->sendslope,
+ ring->hicredit, ring->locredit);
+ }
+ } else {
+ wr32(E1000_RXPBS, I210_RXPBSIZE_DEFAULT);
+ wr32(E1000_TXPBS, I210_TXPBSIZE_DEFAULT);
+ wr32(E1000_I210_DTXMXPKTSZ, I210_DTXMXPKTSZ_DEFAULT);
+
+ val = rd32(E1000_I210_TQAVCTRL);
+ /* According to Section 8.12.21, the other flags we've set when
+ * enabling FQTSS are not relevant when disabling FQTSS so we
+ * don't set they here.
+ */
+ val &= ~E1000_TQAVCTRL_XMIT_MODE;
+ wr32(E1000_I210_TQAVCTRL, val);
+ }
+
+ netdev_dbg(netdev, "FQTSS %s\n", (is_fqtss_enabled(adapter)) ?
+ "enabled" : "disabled");
+}
+
/**
* igb_configure - configure the hardware for RX and TX
* @adapter: private board structure
@@ -1609,6 +1905,7 @@ static void igb_configure(struct igb_adapter *adapter)
igb_get_hw_control(adapter);
igb_set_rx_mode(netdev);
+ igb_setup_tx_mode(adapter);
igb_restore_vlan(adapter);
@@ -2150,6 +2447,55 @@ igb_features_check(struct sk_buff *skb, struct net_device *dev,
return features;
}
+static int igb_offload_cbs(struct igb_adapter *adapter,
+ struct tc_cbs_qopt_offload *qopt)
+{
+ struct e1000_hw *hw = &adapter->hw;
+ int err;
+
+ /* CBS offloading is only supported by i210 controller. */
+ if (hw->mac.type != e1000_i210)
+ return -EOPNOTSUPP;
+
+ /* CBS offloading is only supported by queue 0 and queue 1. */
+ if (qopt->queue < 0 || qopt->queue > 1)
+ return -EINVAL;
+
+ err = igb_save_cbs_params(adapter, qopt->queue, qopt->enable,
+ qopt->idleslope, qopt->sendslope,
+ qopt->hicredit, qopt->locredit);
+ if (err)
+ return err;
+
+ if (is_fqtss_enabled(adapter)) {
+ igb_configure_cbs(adapter, qopt->queue, qopt->enable,
+ qopt->idleslope, qopt->sendslope,
+ qopt->hicredit, qopt->locredit);
+
+ if (!is_any_cbs_enabled(adapter))
+ enable_fqtss(adapter, false);
+
+ } else {
+ enable_fqtss(adapter, true);
+ }
+
+ return 0;
+}
+
+static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
+ void *type_data)
+{
+ struct igb_adapter *adapter = netdev_priv(dev);
+
+ switch (type) {
+ case TC_SETUP_CBS:
+ return igb_offload_cbs(adapter, type_data);
+
+ default:
+ return -EOPNOTSUPP;
+ }
+}
+
static const struct net_device_ops igb_netdev_ops = {
.ndo_open = igb_open,
.ndo_stop = igb_close,
@@ -2175,6 +2521,7 @@ static const struct net_device_ops igb_netdev_ops = {
.ndo_set_features = igb_set_features,
.ndo_fdb_add = igb_ndo_fdb_add,
.ndo_features_check = igb_features_check,
+ .ndo_setup_tc = igb_setup_tc,
};
/**
--
2.14.2
^ permalink raw reply related
* [next-queue PATCH v8 4/6] net/sched: Introduce Credit Based Shaper (CBS) qdisc
From: Vinicius Costa Gomes @ 2017-10-14 0:25 UTC (permalink / raw)
To: netdev, intel-wired-lan
Cc: Vinicius Costa Gomes, jhs, xiyou.wangcong, jiri, andre.guedes,
ivan.briano, jesus.sanchez-palencia, boon.leong.ong,
richardcochran, henrik, levipearson, rodney.cummings
In-Reply-To: <20171014002534.19896-1-vinicius.gomes@intel.com>
This queueing discipline implements the shaper algorithm defined by
the 802.1Q-2014 Section 8.6.8.2 and detailed in Annex L.
It's primary usage is to apply some bandwidth reservation to user
defined traffic classes, which are mapped to different queues via the
mqprio qdisc.
Only a simple software implementation is added for now.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
Signed-off-by: Jesus Sanchez-Palencia <jesus.sanchez-palencia@intel.com>
---
include/uapi/linux/pkt_sched.h | 18 +++
net/sched/Kconfig | 11 ++
net/sched/Makefile | 1 +
net/sched/sch_cbs.c | 293 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 323 insertions(+)
create mode 100644 net/sched/sch_cbs.c
diff --git a/include/uapi/linux/pkt_sched.h b/include/uapi/linux/pkt_sched.h
index 099bf5528fed..41e349df4bf4 100644
--- a/include/uapi/linux/pkt_sched.h
+++ b/include/uapi/linux/pkt_sched.h
@@ -871,4 +871,22 @@ struct tc_pie_xstats {
__u32 maxq; /* maximum queue size */
__u32 ecn_mark; /* packets marked with ecn*/
};
+
+/* CBS */
+struct tc_cbs_qopt {
+ __u8 offload;
+ __s32 hicredit;
+ __s32 locredit;
+ __s32 idleslope;
+ __s32 sendslope;
+};
+
+enum {
+ TCA_CBS_UNSPEC,
+ TCA_CBS_PARMS,
+ __TCA_CBS_MAX,
+};
+
+#define TCA_CBS_MAX (__TCA_CBS_MAX - 1)
+
#endif
diff --git a/net/sched/Kconfig b/net/sched/Kconfig
index e70ed26485a2..c03d86a7775e 100644
--- a/net/sched/Kconfig
+++ b/net/sched/Kconfig
@@ -172,6 +172,17 @@ config NET_SCH_TBF
To compile this code as a module, choose M here: the
module will be called sch_tbf.
+config NET_SCH_CBS
+ tristate "Credit Based Shaper (CBS)"
+ ---help---
+ Say Y here if you want to use the Credit Based Shaper (CBS) packet
+ scheduling algorithm.
+
+ See the top of <file:net/sched/sch_cbs.c> for more details.
+
+ To compile this code as a module, choose M here: the
+ module will be called sch_cbs.
+
config NET_SCH_GRED
tristate "Generic Random Early Detection (GRED)"
---help---
diff --git a/net/sched/Makefile b/net/sched/Makefile
index 7b915d226de7..80c8f92d162d 100644
--- a/net/sched/Makefile
+++ b/net/sched/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_NET_SCH_FQ_CODEL) += sch_fq_codel.o
obj-$(CONFIG_NET_SCH_FQ) += sch_fq.o
obj-$(CONFIG_NET_SCH_HHF) += sch_hhf.o
obj-$(CONFIG_NET_SCH_PIE) += sch_pie.o
+obj-$(CONFIG_NET_SCH_CBS) += sch_cbs.o
obj-$(CONFIG_NET_CLS_U32) += cls_u32.o
obj-$(CONFIG_NET_CLS_ROUTE4) += cls_route.o
diff --git a/net/sched/sch_cbs.c b/net/sched/sch_cbs.c
new file mode 100644
index 000000000000..c0102b589494
--- /dev/null
+++ b/net/sched/sch_cbs.c
@@ -0,0 +1,293 @@
+/*
+ * net/sched/sch_cbs.c Credit Based Shaper
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version
+ * 2 of the License, or (at your option) any later version.
+ *
+ * Authors: Vinicius Costa Gomes <vinicius.gomes@intel.com>
+ *
+ */
+
+/* Credit Based Shaper (CBS)
+ * =========================
+ *
+ * This is a simple rate-limiting shaper aimed at TSN applications on
+ * systems with known traffic workloads.
+ *
+ * Its algorithm is defined by the IEEE 802.1Q-2014 Specification,
+ * Section 8.6.8.2, and explained in more detail in the Annex L of the
+ * same specification.
+ *
+ * There are four tunables to be considered:
+ *
+ * 'idleslope': Idleslope is the rate of credits that is
+ * accumulated (in kilobits per second) when there is at least
+ * one packet waiting for transmission. Packets are transmitted
+ * when the current value of credits is equal or greater than
+ * zero. When there is no packet to be transmitted the amount of
+ * credits is set to zero. This is the main tunable of the CBS
+ * algorithm.
+ *
+ * 'sendslope':
+ * Sendslope is the rate of credits that is depleted (it should be a
+ * negative number of kilobits per second) when a transmission is
+ * ocurring. It can be calculated as follows, (IEEE 802.1Q-2014 Section
+ * 8.6.8.2 item g):
+ *
+ * sendslope = idleslope - port_transmit_rate
+ *
+ * 'hicredit': Hicredit defines the maximum amount of credits (in
+ * bytes) that can be accumulated. Hicredit depends on the
+ * characteristics of interfering traffic,
+ * 'max_interference_size' is the maximum size of any burst of
+ * traffic that can delay the transmission of a frame that is
+ * available for transmission for this traffic class, (IEEE
+ * 802.1Q-2014 Annex L, Equation L-3):
+ *
+ * hicredit = max_interference_size * (idleslope / port_transmit_rate)
+ *
+ * 'locredit': Locredit is the minimum amount of credits that can
+ * be reached. It is a function of the traffic flowing through
+ * this qdisc (IEEE 802.1Q-2014 Annex L, Equation L-2):
+ *
+ * locredit = max_frame_size * (sendslope / port_transmit_rate)
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include <linux/errno.h>
+#include <linux/skbuff.h>
+#include <net/netlink.h>
+#include <net/sch_generic.h>
+#include <net/pkt_sched.h>
+
+#define BYTES_PER_KBIT (1000LL / 8)
+
+struct cbs_sched_data {
+ s64 port_rate; /* in bytes/s */
+ s64 last; /* timestamp in ns */
+ s64 credits; /* in bytes */
+ s32 locredit; /* in bytes */
+ s32 hicredit; /* in bytes */
+ s64 sendslope; /* in bytes/s */
+ s64 idleslope; /* in bytes/s */
+ struct qdisc_watchdog watchdog;
+ int (*enqueue)(struct sk_buff *skb, struct Qdisc *sch);
+ struct sk_buff *(*dequeue)(struct Qdisc *sch);
+};
+
+static int cbs_enqueue_soft(struct sk_buff *skb, struct Qdisc *sch)
+{
+ struct cbs_sched_data *q = qdisc_priv(sch);
+
+ if (sch->q.qlen == 0 && q->credits > 0) {
+ /* We need to stop accumulating credits when there's
+ * no enqueued packets and q->credits is positive.
+ */
+ q->credits = 0;
+ q->last = ktime_get_ns();
+ }
+
+ return qdisc_enqueue_tail(skb, sch);
+}
+
+static int cbs_enqueue(struct sk_buff *skb, struct Qdisc *sch,
+ struct sk_buff **to_free)
+{
+ struct cbs_sched_data *q = qdisc_priv(sch);
+
+ return q->enqueue(skb, sch);
+}
+
+/* timediff is in ns, slope is in bytes/s */
+static s64 timediff_to_credits(s64 timediff, s64 slope)
+{
+ return div64_s64(timediff * slope, NSEC_PER_SEC);
+}
+
+static s64 delay_from_credits(s64 credits, s64 slope)
+{
+ if (unlikely(slope == 0))
+ return S64_MAX;
+
+ return div64_s64(-credits * NSEC_PER_SEC, slope);
+}
+
+static s64 credits_from_len(unsigned int len, s64 slope, s64 port_rate)
+{
+ if (unlikely(port_rate == 0))
+ return S64_MAX;
+
+ return div64_s64(len * slope, port_rate);
+}
+
+static struct sk_buff *cbs_dequeue_soft(struct Qdisc *sch)
+{
+ struct cbs_sched_data *q = qdisc_priv(sch);
+ s64 now = ktime_get_ns();
+ struct sk_buff *skb;
+ s64 credits;
+ int len;
+
+ if (q->credits < 0) {
+ credits = timediff_to_credits(now - q->last, q->idleslope);
+
+ credits = q->credits + credits;
+ q->credits = min_t(s64, credits, q->hicredit);
+
+ if (q->credits < 0) {
+ s64 delay;
+
+ delay = delay_from_credits(q->credits, q->idleslope);
+ qdisc_watchdog_schedule_ns(&q->watchdog, now + delay);
+
+ q->last = now;
+
+ return NULL;
+ }
+ }
+
+ skb = qdisc_dequeue_head(sch);
+ if (!skb)
+ return NULL;
+
+ len = qdisc_pkt_len(skb);
+
+ /* As sendslope is a negative number, this will decrease the
+ * amount of q->credits.
+ */
+ credits = credits_from_len(len, q->sendslope, q->port_rate);
+ credits += q->credits;
+
+ q->credits = max_t(s64, credits, q->locredit);
+ q->last = now;
+
+ return skb;
+}
+
+static struct sk_buff *cbs_dequeue(struct Qdisc *sch)
+{
+ struct cbs_sched_data *q = qdisc_priv(sch);
+
+ return q->dequeue(sch);
+}
+
+static const struct nla_policy cbs_policy[TCA_CBS_MAX + 1] = {
+ [TCA_CBS_PARMS] = { .len = sizeof(struct tc_cbs_qopt) },
+};
+
+static int cbs_change(struct Qdisc *sch, struct nlattr *opt)
+{
+ struct cbs_sched_data *q = qdisc_priv(sch);
+ struct net_device *dev = qdisc_dev(sch);
+ struct nlattr *tb[TCA_CBS_MAX + 1];
+ struct ethtool_link_ksettings ecmd;
+ struct tc_cbs_qopt *qopt;
+ s64 link_speed;
+ int err;
+
+ err = nla_parse_nested(tb, TCA_CBS_MAX, opt, cbs_policy, NULL);
+ if (err < 0)
+ return err;
+
+ if (!tb[TCA_CBS_PARMS])
+ return -EINVAL;
+
+ qopt = nla_data(tb[TCA_CBS_PARMS]);
+
+ if (qopt->offload)
+ return -EOPNOTSUPP;
+
+ if (!__ethtool_get_link_ksettings(dev, &ecmd))
+ link_speed = ecmd.base.speed;
+ else
+ link_speed = SPEED_1000;
+
+ q->port_rate = link_speed * 1000 * BYTES_PER_KBIT;
+
+ q->enqueue = cbs_enqueue_soft;
+ q->dequeue = cbs_dequeue_soft;
+
+ q->hicredit = qopt->hicredit;
+ q->locredit = qopt->locredit;
+ q->idleslope = qopt->idleslope * BYTES_PER_KBIT;
+ q->sendslope = qopt->sendslope * BYTES_PER_KBIT;
+
+ return 0;
+}
+
+static int cbs_init(struct Qdisc *sch, struct nlattr *opt)
+{
+ struct cbs_sched_data *q = qdisc_priv(sch);
+
+ if (!opt)
+ return -EINVAL;
+
+ qdisc_watchdog_init(&q->watchdog, sch);
+
+ return cbs_change(sch, opt);
+}
+
+static void cbs_destroy(struct Qdisc *sch)
+{
+ struct cbs_sched_data *q = qdisc_priv(sch);
+
+ qdisc_watchdog_cancel(&q->watchdog);
+}
+
+static int cbs_dump(struct Qdisc *sch, struct sk_buff *skb)
+{
+ struct cbs_sched_data *q = qdisc_priv(sch);
+ struct tc_cbs_qopt opt = { };
+ struct nlattr *nest;
+
+ nest = nla_nest_start(skb, TCA_OPTIONS);
+ if (!nest)
+ goto nla_put_failure;
+
+ opt.hicredit = q->hicredit;
+ opt.locredit = q->locredit;
+ opt.sendslope = q->sendslope / BYTES_PER_KBIT;
+ opt.idleslope = q->idleslope / BYTES_PER_KBIT;
+ opt.offload = 0;
+
+ if (nla_put(skb, TCA_CBS_PARMS, sizeof(opt), &opt))
+ goto nla_put_failure;
+
+ return nla_nest_end(skb, nest);
+
+nla_put_failure:
+ nla_nest_cancel(skb, nest);
+ return -1;
+}
+
+static struct Qdisc_ops cbs_qdisc_ops __read_mostly = {
+ .id = "cbs",
+ .priv_size = sizeof(struct cbs_sched_data),
+ .enqueue = cbs_enqueue,
+ .dequeue = cbs_dequeue,
+ .peek = qdisc_peek_dequeued,
+ .init = cbs_init,
+ .reset = qdisc_reset_queue,
+ .destroy = cbs_destroy,
+ .change = cbs_change,
+ .dump = cbs_dump,
+ .owner = THIS_MODULE,
+};
+
+static int __init cbs_module_init(void)
+{
+ return register_qdisc(&cbs_qdisc_ops);
+}
+
+static void __exit cbs_module_exit(void)
+{
+ unregister_qdisc(&cbs_qdisc_ops);
+}
+module_init(cbs_module_init)
+module_exit(cbs_module_exit)
+MODULE_LICENSE("GPL");
--
2.14.2
^ permalink raw reply related
* [next-queue PATCH v8 5/6] net/sched: Add support for HW offloading for CBS
From: Vinicius Costa Gomes @ 2017-10-14 0:25 UTC (permalink / raw)
To: netdev, intel-wired-lan
Cc: Vinicius Costa Gomes, jhs, xiyou.wangcong, jiri, andre.guedes,
ivan.briano, jesus.sanchez-palencia, boon.leong.ong,
richardcochran, henrik, levipearson, rodney.cummings
In-Reply-To: <20171014002534.19896-1-vinicius.gomes@intel.com>
This adds support for offloading the CBS algorithm to the controller,
if supported. Drivers wanting to support CBS offload must implement
the .ndo_setup_tc callback and handle the TC_SETUP_CBS (introduced
here) type.
Signed-off-by: Vinicius Costa Gomes <vinicius.gomes@intel.com>
---
include/linux/netdevice.h | 1 +
include/net/pkt_sched.h | 9 ++++
net/sched/sch_cbs.c | 104 ++++++++++++++++++++++++++++++++++++++++------
3 files changed, 102 insertions(+), 12 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 31bb3010c69b..1f6c44ef5b21 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -775,6 +775,7 @@ enum tc_setup_type {
TC_SETUP_CLSFLOWER,
TC_SETUP_CLSMATCHALL,
TC_SETUP_CLSBPF,
+ TC_SETUP_CBS,
};
/* These structures hold the attributes of xdp state that are being passed
diff --git a/include/net/pkt_sched.h b/include/net/pkt_sched.h
index 259bc191ba59..7c597b050b36 100644
--- a/include/net/pkt_sched.h
+++ b/include/net/pkt_sched.h
@@ -146,4 +146,13 @@ static inline bool is_classid_clsact_egress(u32 classid)
TC_H_MIN(classid) == TC_H_MIN(TC_H_MIN_EGRESS);
}
+struct tc_cbs_qopt_offload {
+ u8 enable;
+ s32 queue;
+ s32 hicredit;
+ s32 locredit;
+ s32 idleslope;
+ s32 sendslope;
+};
+
#endif
diff --git a/net/sched/sch_cbs.c b/net/sched/sch_cbs.c
index c0102b589494..cae021c642e5 100644
--- a/net/sched/sch_cbs.c
+++ b/net/sched/sch_cbs.c
@@ -68,6 +68,8 @@
#define BYTES_PER_KBIT (1000LL / 8)
struct cbs_sched_data {
+ bool offload;
+ int queue;
s64 port_rate; /* in bytes/s */
s64 last; /* timestamp in ns */
s64 credits; /* in bytes */
@@ -80,6 +82,11 @@ struct cbs_sched_data {
struct sk_buff *(*dequeue)(struct Qdisc *sch);
};
+static int cbs_enqueue_offload(struct sk_buff *skb, struct Qdisc *sch)
+{
+ return qdisc_enqueue_tail(skb, sch);
+}
+
static int cbs_enqueue_soft(struct sk_buff *skb, struct Qdisc *sch)
{
struct cbs_sched_data *q = qdisc_priv(sch);
@@ -169,6 +176,11 @@ static struct sk_buff *cbs_dequeue_soft(struct Qdisc *sch)
return skb;
}
+static struct sk_buff *cbs_dequeue_offload(struct Qdisc *sch)
+{
+ return qdisc_dequeue_head(sch);
+}
+
static struct sk_buff *cbs_dequeue(struct Qdisc *sch)
{
struct cbs_sched_data *q = qdisc_priv(sch);
@@ -180,14 +192,66 @@ static const struct nla_policy cbs_policy[TCA_CBS_MAX + 1] = {
[TCA_CBS_PARMS] = { .len = sizeof(struct tc_cbs_qopt) },
};
+static void cbs_disable_offload(struct net_device *dev,
+ struct cbs_sched_data *q)
+{
+ struct tc_cbs_qopt_offload cbs = { };
+ const struct net_device_ops *ops;
+ int err;
+
+ if (!q->offload)
+ return;
+
+ q->enqueue = cbs_enqueue_soft;
+ q->dequeue = cbs_dequeue_soft;
+
+ ops = dev->netdev_ops;
+ if (!ops->ndo_setup_tc)
+ return;
+
+ cbs.queue = q->queue;
+ cbs.enable = 0;
+
+ err = ops->ndo_setup_tc(dev, TC_SETUP_CBS, &cbs);
+ if (err < 0)
+ pr_warn("Couldn't disable CBS offload for queue %d\n",
+ cbs.queue);
+}
+
+static int cbs_enable_offload(struct net_device *dev, struct cbs_sched_data *q,
+ const struct tc_cbs_qopt *opt)
+{
+ const struct net_device_ops *ops = dev->netdev_ops;
+ struct tc_cbs_qopt_offload cbs = { };
+ int err;
+
+ if (!ops->ndo_setup_tc)
+ return -EOPNOTSUPP;
+
+ cbs.queue = q->queue;
+
+ cbs.enable = 1;
+ cbs.hicredit = opt->hicredit;
+ cbs.locredit = opt->locredit;
+ cbs.idleslope = opt->idleslope;
+ cbs.sendslope = opt->sendslope;
+
+ err = ops->ndo_setup_tc(dev, TC_SETUP_CBS, &cbs);
+ if (err < 0)
+ return err;
+
+ q->enqueue = cbs_enqueue_offload;
+ q->dequeue = cbs_dequeue_offload;
+
+ return 0;
+}
+
static int cbs_change(struct Qdisc *sch, struct nlattr *opt)
{
struct cbs_sched_data *q = qdisc_priv(sch);
struct net_device *dev = qdisc_dev(sch);
struct nlattr *tb[TCA_CBS_MAX + 1];
- struct ethtool_link_ksettings ecmd;
struct tc_cbs_qopt *qopt;
- s64 link_speed;
int err;
err = nla_parse_nested(tb, TCA_CBS_MAX, opt, cbs_policy, NULL);
@@ -199,23 +263,30 @@ static int cbs_change(struct Qdisc *sch, struct nlattr *opt)
qopt = nla_data(tb[TCA_CBS_PARMS]);
- if (qopt->offload)
- return -EOPNOTSUPP;
+ if (!qopt->offload) {
+ struct ethtool_link_ksettings ecmd;
+ s64 link_speed;
- if (!__ethtool_get_link_ksettings(dev, &ecmd))
- link_speed = ecmd.base.speed;
- else
- link_speed = SPEED_1000;
+ if (!__ethtool_get_link_ksettings(dev, &ecmd))
+ link_speed = ecmd.base.speed;
+ else
+ link_speed = SPEED_1000;
- q->port_rate = link_speed * 1000 * BYTES_PER_KBIT;
+ q->port_rate = link_speed * 1000 * BYTES_PER_KBIT;
- q->enqueue = cbs_enqueue_soft;
- q->dequeue = cbs_dequeue_soft;
+ cbs_disable_offload(dev, q);
+ } else {
+ err = cbs_enable_offload(dev, q, qopt);
+ if (err < 0)
+ return err;
+ }
+ /* Everything went OK, save the parameters used. */
q->hicredit = qopt->hicredit;
q->locredit = qopt->locredit;
q->idleslope = qopt->idleslope * BYTES_PER_KBIT;
q->sendslope = qopt->sendslope * BYTES_PER_KBIT;
+ q->offload = qopt->offload;
return 0;
}
@@ -223,10 +294,16 @@ static int cbs_change(struct Qdisc *sch, struct nlattr *opt)
static int cbs_init(struct Qdisc *sch, struct nlattr *opt)
{
struct cbs_sched_data *q = qdisc_priv(sch);
+ struct net_device *dev = qdisc_dev(sch);
if (!opt)
return -EINVAL;
+ q->queue = sch->dev_queue - netdev_get_tx_queue(dev, 0);
+
+ q->enqueue = cbs_enqueue_soft;
+ q->dequeue = cbs_dequeue_soft;
+
qdisc_watchdog_init(&q->watchdog, sch);
return cbs_change(sch, opt);
@@ -235,8 +312,11 @@ static int cbs_init(struct Qdisc *sch, struct nlattr *opt)
static void cbs_destroy(struct Qdisc *sch)
{
struct cbs_sched_data *q = qdisc_priv(sch);
+ struct net_device *dev = qdisc_dev(sch);
qdisc_watchdog_cancel(&q->watchdog);
+
+ cbs_disable_offload(dev, q);
}
static int cbs_dump(struct Qdisc *sch, struct sk_buff *skb)
@@ -253,7 +333,7 @@ static int cbs_dump(struct Qdisc *sch, struct sk_buff *skb)
opt.locredit = q->locredit;
opt.sendslope = q->sendslope / BYTES_PER_KBIT;
opt.idleslope = q->idleslope / BYTES_PER_KBIT;
- opt.offload = 0;
+ opt.offload = q->offload;
if (nla_put(skb, TCA_CBS_PARMS, sizeof(opt), &opt))
goto nla_put_failure;
--
2.14.2
^ permalink raw reply related
* Re: [net-next V7 PATCH 1/5] bpf: introduce new bpf cpu map type BPF_MAP_TYPE_CPUMAP
From: kbuild test robot @ 2017-10-14 0:36 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: kbuild-all, netdev, jakub.kicinski, Michael S. Tsirkin,
pavel.odintsov, Jason Wang, mchan, John Fastabend,
peter.waskiewicz.jr, Jesper Dangaard Brouer, ast, Daniel Borkmann,
Alexei Starovoitov, Andy Gospodarek
In-Reply-To: <150781120970.9409.4248519763583438653.stgit@firesoul>
[-- Attachment #1: Type: text/plain, Size: 3066 bytes --]
Hi Jesper,
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Jesper-Dangaard-Brouer/New-bpf-cpumap-type-for-XDP_REDIRECT/20171014-061849
config: blackfin-allyesconfig (attached as .config)
compiler: bfin-uclinux-gcc (GCC) 6.2.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=blackfin
Note: the linux-review/Jesper-Dangaard-Brouer/New-bpf-cpumap-type-for-XDP_REDIRECT/20171014-061849 HEAD 000080e4de4026e29bcabecba678bc91f4a9688c builds fine.
It only hurts bisectibility.
All errors (new ones prefixed by >>):
kernel//bpf/cpumap.c: In function '__cpu_map_entry_alloc':
>> kernel//bpf/cpumap.c:242:2: error: expected ';' before 'rcpu'
rcpu->kthread = kthread_create_on_node(cpu_map_kthread_run, rcpu, numa,
^~~~
At top level:
kernel//bpf/cpumap.c:184:12: warning: 'cpu_map_kthread_run' defined but not used [-Wunused-function]
static int cpu_map_kthread_run(void *data)
^~~~~~~~~~~~~~~~~~~
vim +242 kernel//bpf/cpumap.c
211
212 struct bpf_cpu_map_entry *__cpu_map_entry_alloc(u32 qsize, u32 cpu, int map_id)
213 {
214 gfp_t gfp = GFP_ATOMIC|__GFP_NOWARN;
215 struct bpf_cpu_map_entry *rcpu;
216 int numa, err;
217
218 /* Have map->numa_node, but choose node of redirect target CPU */
219 numa = cpu_to_node(cpu);
220
221 rcpu = kzalloc_node(sizeof(*rcpu), gfp, numa);
222 if (!rcpu)
223 return NULL;
224
225 /* Alloc percpu bulkq */
226 rcpu->bulkq = __alloc_percpu_gfp(sizeof(*rcpu->bulkq),
227 sizeof(void *), gfp);
228 if (!rcpu->bulkq)
229 goto free_rcu;
230
231 /* Alloc queue */
232 rcpu->queue = kzalloc_node(sizeof(*rcpu->queue), gfp, numa);
233 if (!rcpu->queue)
234 goto free_bulkq;
235
236 err = ptr_ring_init(rcpu->queue, qsize, gfp);
237 if (err)
238 goto free_queue;
239 rcpu->qsize = qsize
240
241 /* Setup kthread */
> 242 rcpu->kthread = kthread_create_on_node(cpu_map_kthread_run, rcpu, numa,
243 "cpumap/%d/map:%d", cpu, map_id);
244 if (IS_ERR(rcpu->kthread))
245 goto free_ptr_ring;
246
247 get_cpu_map_entry(rcpu); /* 1-refcnt for being in cmap->cpu_map[] */
248 get_cpu_map_entry(rcpu); /* 1-refcnt for kthread */
249
250 /* Make sure kthread runs on a single CPU */
251 kthread_bind(rcpu->kthread, cpu);
252 wake_up_process(rcpu->kthread);
253
254 return rcpu;
255
256 free_ptr_ring:
257 ptr_ring_cleanup(rcpu->queue, NULL);
258 free_queue:
259 kfree(rcpu->queue);
260 free_bulkq:
261 free_percpu(rcpu->bulkq);
262 free_rcu:
263 kfree(rcpu);
264 return NULL;
265 }
266
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 46057 bytes --]
^ permalink raw reply
* Re: [PATCH net-next] ipv6: only update __use and lastusetime once per jiffy at most
From: Eric Dumazet @ 2017-10-14 0:26 UTC (permalink / raw)
To: Martin KaFai Lau; +Cc: Wei Wang, David Miller, netdev, Paolo Abeni
In-Reply-To: <20171014000924.dcfuqxwlalaigqdq@kafai-mbp.dhcp.thefacebook.com>
On Fri, Oct 13, 2017 at 5:09 PM, Martin KaFai Lau <kafai@fb.com> wrote:
> On Fri, Oct 13, 2017 at 10:08:07PM +0000, Wei Wang wrote:
>> From: Wei Wang <weiwan@google.com>
>>
>> In order to not dirty the cacheline too often, we try to only update
>> dst->__use and dst->lastusetime at most once per jiffy.
>
>
>> As dst->lastusetime is only used by ipv6 garbage collector, it should
>> be good enough time resolution.
> Make sense.
>
>> And __use is only used in ipv6_route_seq_show() to show how many times a
>> dst has been used. And as __use is not atomic_t right now, it does not
>> show the precise number of usage times anyway. So we think it should be
>> OK to only update it at most once per jiffy.
> If __use is only bumped HZ number of times per second and we can do ~3Mpps now,
> would __use be way off?
It is not used in the kernel, and is not even reported by user space
(iproute2) currently.
With the percpu stuff, we never did the sum anyway.
I believe we should be fine by being very lazy on this field.
If really someones complain, we will see, but insuring ~one update per
HZ seems fine.
^ permalink raw reply
* [PATCH net 0/6] bnxt_en: bug fixes.
From: Michael Chan @ 2017-10-14 1:09 UTC (permalink / raw)
To: davem; +Cc: netdev
Various bug fixes for the VF/PF link change logic, VF resource checking,
potential firmware response corruption on NVRAM and DCB parameters,
and reading the wrong register for PCIe link speed on the VF.
Michael Chan (4):
bnxt_en: Improve VF/PF link change logic.
bnxt_en: Don't use rtnl lock to protect link change logic in
workqueue.
bnxt_en: Fix VF resource checking.
bnxt_en: Fix possible corrupted NVRAM parameters from firmware
response.
Sankar Patchineelam (1):
bnxt_en: Fix possible corruption in DCB parameters from firmware.
Vasundhara Volam (1):
bnxt_en: Fix VF PCIe link speed and width logic.
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 99 +++++++++++++++++------
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 5 ++
drivers/net/ethernet/broadcom/bnxt/bnxt_dcb.c | 23 ++++--
drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 8 +-
drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c | 11 ++-
5 files changed, 112 insertions(+), 34 deletions(-)
--
1.8.3.1
^ permalink raw reply
* [PATCH net 1/6] bnxt_en: Improve VF/PF link change logic.
From: Michael Chan @ 2017-10-14 1:09 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <1507943374-13308-1-git-send-email-michael.chan@broadcom.com>
Link status query firmware messages originating from the VFs are forwarded
to the PF. The driver handles these interactions in a workqueue for the
VF and PF. The VF driver waits for the response from the PF in the
workqueue. If the PF and VF driver are running on the same host and the
work for both PF and VF are queued on the same workqueue, the VF driver
may not get the response if the PF work item is queued behind it on the
same workqueue. This will lead to the VF link query message timing out.
To prevent this, we create a private workqueue for PFs instead of using
the common workqueue. The VF query and PF response will never be on
the same workqueue.
Fixes: c0c050c58d84 ("bnxt_en: New Broadcom ethernet driver.")
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 66 +++++++++++++++++++++++++------
1 file changed, 53 insertions(+), 13 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index aacec8b..7906153 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -214,6 +214,8 @@ enum board_idx {
ASYNC_EVENT_CMPL_EVENT_ID_LINK_SPEED_CFG_CHANGE,
};
+static struct workqueue_struct *bnxt_pf_wq;
+
static bool bnxt_vf_pciid(enum board_idx idx)
{
return (idx == NETXTREME_C_VF || idx == NETXTREME_E_VF);
@@ -1024,12 +1026,28 @@ static int bnxt_discard_rx(struct bnxt *bp, struct bnxt_napi *bnapi,
return 0;
}
+static void bnxt_queue_sp_work(struct bnxt *bp)
+{
+ if (BNXT_PF(bp))
+ queue_work(bnxt_pf_wq, &bp->sp_task);
+ else
+ schedule_work(&bp->sp_task);
+}
+
+static void bnxt_cancel_sp_work(struct bnxt *bp)
+{
+ if (BNXT_PF(bp))
+ flush_workqueue(bnxt_pf_wq);
+ else
+ cancel_work_sync(&bp->sp_task);
+}
+
static void bnxt_sched_reset(struct bnxt *bp, struct bnxt_rx_ring_info *rxr)
{
if (!rxr->bnapi->in_reset) {
rxr->bnapi->in_reset = true;
set_bit(BNXT_RESET_TASK_SP_EVENT, &bp->sp_event);
- schedule_work(&bp->sp_task);
+ bnxt_queue_sp_work(bp);
}
rxr->rx_next_cons = 0xffff;
}
@@ -1717,7 +1735,7 @@ static int bnxt_async_event_process(struct bnxt *bp,
default:
goto async_event_process_exit;
}
- schedule_work(&bp->sp_task);
+ bnxt_queue_sp_work(bp);
async_event_process_exit:
bnxt_ulp_async_events(bp, cmpl);
return 0;
@@ -1751,7 +1769,7 @@ static int bnxt_hwrm_handler(struct bnxt *bp, struct tx_cmp *txcmp)
set_bit(vf_id - bp->pf.first_vf_id, bp->pf.vf_event_bmap);
set_bit(BNXT_HWRM_EXEC_FWD_REQ_SP_EVENT, &bp->sp_event);
- schedule_work(&bp->sp_task);
+ bnxt_queue_sp_work(bp);
break;
case CMPL_BASE_TYPE_HWRM_ASYNC_EVENT:
@@ -6647,7 +6665,7 @@ static void bnxt_set_rx_mode(struct net_device *dev)
vnic->rx_mask = mask;
set_bit(BNXT_RX_MASK_SP_EVENT, &bp->sp_event);
- schedule_work(&bp->sp_task);
+ bnxt_queue_sp_work(bp);
}
}
@@ -6920,7 +6938,7 @@ static void bnxt_tx_timeout(struct net_device *dev)
netdev_err(bp->dev, "TX timeout detected, starting reset task!\n");
set_bit(BNXT_RESET_TASK_SP_EVENT, &bp->sp_event);
- schedule_work(&bp->sp_task);
+ bnxt_queue_sp_work(bp);
}
#ifdef CONFIG_NET_POLL_CONTROLLER
@@ -6952,7 +6970,7 @@ static void bnxt_timer(unsigned long data)
if (bp->link_info.link_up && (bp->flags & BNXT_FLAG_PORT_STATS) &&
bp->stats_coal_ticks) {
set_bit(BNXT_PERIODIC_STATS_SP_EVENT, &bp->sp_event);
- schedule_work(&bp->sp_task);
+ bnxt_queue_sp_work(bp);
}
bnxt_restart_timer:
mod_timer(&bp->timer, jiffies + bp->current_interval);
@@ -7433,7 +7451,7 @@ static int bnxt_rx_flow_steer(struct net_device *dev, const struct sk_buff *skb,
spin_unlock_bh(&bp->ntp_fltr_lock);
set_bit(BNXT_RX_NTP_FLTR_SP_EVENT, &bp->sp_event);
- schedule_work(&bp->sp_task);
+ bnxt_queue_sp_work(bp);
return new_fltr->sw_id;
@@ -7516,7 +7534,7 @@ static void bnxt_udp_tunnel_add(struct net_device *dev,
if (bp->vxlan_port_cnt == 1) {
bp->vxlan_port = ti->port;
set_bit(BNXT_VXLAN_ADD_PORT_SP_EVENT, &bp->sp_event);
- schedule_work(&bp->sp_task);
+ bnxt_queue_sp_work(bp);
}
break;
case UDP_TUNNEL_TYPE_GENEVE:
@@ -7533,7 +7551,7 @@ static void bnxt_udp_tunnel_add(struct net_device *dev,
return;
}
- schedule_work(&bp->sp_task);
+ bnxt_queue_sp_work(bp);
}
static void bnxt_udp_tunnel_del(struct net_device *dev,
@@ -7572,7 +7590,7 @@ static void bnxt_udp_tunnel_del(struct net_device *dev,
return;
}
- schedule_work(&bp->sp_task);
+ bnxt_queue_sp_work(bp);
}
static int bnxt_bridge_getlink(struct sk_buff *skb, u32 pid, u32 seq,
@@ -7720,7 +7738,7 @@ static void bnxt_remove_one(struct pci_dev *pdev)
pci_disable_pcie_error_reporting(pdev);
unregister_netdev(dev);
bnxt_shutdown_tc(bp);
- cancel_work_sync(&bp->sp_task);
+ bnxt_cancel_sp_work(bp);
bp->sp_event = 0;
bnxt_clear_int_mode(bp);
@@ -8138,8 +8156,17 @@ static int bnxt_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
else
device_set_wakeup_capable(&pdev->dev, false);
- if (BNXT_PF(bp))
+ if (BNXT_PF(bp)) {
+ if (!bnxt_pf_wq) {
+ bnxt_pf_wq =
+ create_singlethread_workqueue("bnxt_pf_wq");
+ if (!bnxt_pf_wq) {
+ dev_err(&pdev->dev, "Unable to create workqueue.\n");
+ goto init_err_pci_clean;
+ }
+ }
bnxt_init_tc(bp);
+ }
rc = register_netdev(dev);
if (rc)
@@ -8375,4 +8402,17 @@ static void bnxt_io_resume(struct pci_dev *pdev)
#endif
};
-module_pci_driver(bnxt_pci_driver);
+static int __init bnxt_init(void)
+{
+ return pci_register_driver(&bnxt_pci_driver);
+}
+
+static void __exit bnxt_exit(void)
+{
+ pci_unregister_driver(&bnxt_pci_driver);
+ if (bnxt_pf_wq)
+ destroy_workqueue(bnxt_pf_wq);
+}
+
+module_init(bnxt_init);
+module_exit(bnxt_exit);
--
1.8.3.1
^ permalink raw reply related
* [PATCH net 2/6] bnxt_en: Don't use rtnl lock to protect link change logic in workqueue.
From: Michael Chan @ 2017-10-14 1:09 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <1507943374-13308-1-git-send-email-michael.chan@broadcom.com>
As a further improvement to the PF/VF link change logic, use a private
mutex instead of the rtnl lock to protect link change logic. With the
new mutex, we don't have to take the rtnl lock in the workqueue when
we have to handle link related functions. If the VF and PF drivers
are running on the same host and both take the rtnl lock and one is
waiting for the other, it will cause timeout. This patch fixes these
timeouts.
Fixes: 90c694bb7181 ("bnxt_en: Fix RTNL lock usage on bnxt_update_link().")
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 25 ++++++++++++-----------
drivers/net/ethernet/broadcom/bnxt/bnxt.h | 4 ++++
drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c | 4 ++++
3 files changed, 21 insertions(+), 12 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 7906153..3f596de 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -6345,7 +6345,9 @@ static int __bnxt_open_nic(struct bnxt *bp, bool irq_re_init, bool link_re_init)
}
if (link_re_init) {
+ mutex_lock(&bp->link_lock);
rc = bnxt_update_phy_setting(bp);
+ mutex_unlock(&bp->link_lock);
if (rc)
netdev_warn(bp->dev, "failed to update phy settings\n");
}
@@ -7043,30 +7045,28 @@ static void bnxt_sp_task(struct work_struct *work)
if (test_and_clear_bit(BNXT_PERIODIC_STATS_SP_EVENT, &bp->sp_event))
bnxt_hwrm_port_qstats(bp);
- /* These functions below will clear BNXT_STATE_IN_SP_TASK. They
- * must be the last functions to be called before exiting.
- */
if (test_and_clear_bit(BNXT_LINK_CHNG_SP_EVENT, &bp->sp_event)) {
- int rc = 0;
+ int rc;
+ mutex_lock(&bp->link_lock);
if (test_and_clear_bit(BNXT_LINK_SPEED_CHNG_SP_EVENT,
&bp->sp_event))
bnxt_hwrm_phy_qcaps(bp);
- bnxt_rtnl_lock_sp(bp);
- if (test_bit(BNXT_STATE_OPEN, &bp->state))
- rc = bnxt_update_link(bp, true);
- bnxt_rtnl_unlock_sp(bp);
+ rc = bnxt_update_link(bp, true);
+ mutex_unlock(&bp->link_lock);
if (rc)
netdev_err(bp->dev, "SP task can't update link (rc: %x)\n",
rc);
}
if (test_and_clear_bit(BNXT_HWRM_PORT_MODULE_SP_EVENT, &bp->sp_event)) {
- bnxt_rtnl_lock_sp(bp);
- if (test_bit(BNXT_STATE_OPEN, &bp->state))
- bnxt_get_port_module_status(bp);
- bnxt_rtnl_unlock_sp(bp);
+ mutex_lock(&bp->link_lock);
+ bnxt_get_port_module_status(bp);
+ mutex_unlock(&bp->link_lock);
}
+ /* These functions below will clear BNXT_STATE_IN_SP_TASK. They
+ * must be the last functions to be called before exiting.
+ */
if (test_and_clear_bit(BNXT_RESET_TASK_SP_EVENT, &bp->sp_event))
bnxt_reset(bp, false);
@@ -7766,6 +7766,7 @@ static int bnxt_probe_phy(struct bnxt *bp)
rc);
return rc;
}
+ mutex_init(&bp->link_lock);
rc = bnxt_update_link(bp, false);
if (rc) {
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.h b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
index 7b888d4..d2925c0 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.h
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.h
@@ -1290,6 +1290,10 @@ struct bnxt {
unsigned long *ntp_fltr_bmap;
int ntp_fltr_count;
+ /* To protect link related settings during link changes and
+ * ethtool settings changes.
+ */
+ struct mutex link_lock;
struct bnxt_link_info link_info;
struct ethtool_eee eee;
u32 lpi_tmr_lo;
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
index 8eff05a..b2cbc97 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_ethtool.c
@@ -1052,6 +1052,7 @@ static int bnxt_get_link_ksettings(struct net_device *dev,
u32 ethtool_speed;
ethtool_link_ksettings_zero_link_mode(lk_ksettings, supported);
+ mutex_lock(&bp->link_lock);
bnxt_fw_to_ethtool_support_spds(link_info, lk_ksettings);
ethtool_link_ksettings_zero_link_mode(lk_ksettings, advertising);
@@ -1099,6 +1100,7 @@ static int bnxt_get_link_ksettings(struct net_device *dev,
base->port = PORT_FIBRE;
}
base->phy_address = link_info->phy_addr;
+ mutex_unlock(&bp->link_lock);
return 0;
}
@@ -1190,6 +1192,7 @@ static int bnxt_set_link_ksettings(struct net_device *dev,
if (!BNXT_SINGLE_PF(bp))
return -EOPNOTSUPP;
+ mutex_lock(&bp->link_lock);
if (base->autoneg == AUTONEG_ENABLE) {
BNXT_ETHTOOL_TO_FW_SPDS(fw_advertising, lk_ksettings,
advertising);
@@ -1234,6 +1237,7 @@ static int bnxt_set_link_ksettings(struct net_device *dev,
rc = bnxt_hwrm_set_link_setting(bp, set_pause, false);
set_setting_exit:
+ mutex_unlock(&bp->link_lock);
return rc;
}
--
1.8.3.1
^ permalink raw reply related
* [PATCH net 3/6] bnxt_en: Fix VF PCIe link speed and width logic.
From: Michael Chan @ 2017-10-14 1:09 UTC (permalink / raw)
To: davem; +Cc: netdev, Vasundhara Volam
In-Reply-To: <1507943374-13308-1-git-send-email-michael.chan@broadcom.com>
From: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
PCIE PCIE_EP_REG_LINK_STATUS_CONTROL register is only defined in PF
config space, so we must read it from the PF.
Fixes: 90c4f788f6c0 ("bnxt_en: Report PCIe link speed and width during driver load")
Signed-off-by: Vasundhara Volam <vasundhara-v.volam@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 3f596de..4ffa0b1 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -7965,7 +7965,7 @@ static void bnxt_parse_log_pcie_link(struct bnxt *bp)
enum pcie_link_width width = PCIE_LNK_WIDTH_UNKNOWN;
enum pci_bus_speed speed = PCI_SPEED_UNKNOWN;
- if (pcie_get_minimum_link(bp->pdev, &speed, &width) ||
+ if (pcie_get_minimum_link(pci_physfn(bp->pdev), &speed, &width) ||
speed == PCI_SPEED_UNKNOWN || width == PCIE_LNK_WIDTH_UNKNOWN)
netdev_info(bp->dev, "Failed to determine PCIe Link Info\n");
else
--
1.8.3.1
^ permalink raw reply related
* [PATCH net 4/6] bnxt_en: Fix VF resource checking.
From: Michael Chan @ 2017-10-14 1:09 UTC (permalink / raw)
To: davem; +Cc: netdev
In-Reply-To: <1507943374-13308-1-git-send-email-michael.chan@broadcom.com>
In bnxt_sriov_enable(), we calculate to see if we have enough hardware
resources to enable the requested number of VFs. The logic to check
for minimum completion rings and statistics contexts is missing. Add
the required checks so that VF configuration won't fail.
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
---
drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
index d37925a..5ee1866 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt_sriov.c
@@ -502,6 +502,7 @@ static int bnxt_sriov_enable(struct bnxt *bp, int *num_vfs)
int rc = 0, vfs_supported;
int min_rx_rings, min_tx_rings, min_rss_ctxs;
int tx_ok = 0, rx_ok = 0, rss_ok = 0;
+ int avail_cp, avail_stat;
/* Check if we can enable requested num of vf's. At a mininum
* we require 1 RX 1 TX rings for each VF. In this minimum conf
@@ -509,6 +510,10 @@ static int bnxt_sriov_enable(struct bnxt *bp, int *num_vfs)
*/
vfs_supported = *num_vfs;
+ avail_cp = bp->pf.max_cp_rings - bp->cp_nr_rings;
+ avail_stat = bp->pf.max_stat_ctxs - bp->num_stat_ctxs;
+ avail_cp = min_t(int, avail_cp, avail_stat);
+
while (vfs_supported) {
min_rx_rings = vfs_supported;
min_tx_rings = vfs_supported;
@@ -523,10 +528,12 @@ static int bnxt_sriov_enable(struct bnxt *bp, int *num_vfs)
min_rx_rings)
rx_ok = 1;
}
- if (bp->pf.max_vnics - bp->nr_vnics < min_rx_rings)
+ if (bp->pf.max_vnics - bp->nr_vnics < min_rx_rings ||
+ avail_cp < min_rx_rings)
rx_ok = 0;
- if (bp->pf.max_tx_rings - bp->tx_nr_rings >= min_tx_rings)
+ if (bp->pf.max_tx_rings - bp->tx_nr_rings >= min_tx_rings &&
+ avail_cp >= min_tx_rings)
tx_ok = 1;
if (bp->pf.max_rsscos_ctxs - bp->rsscos_nr_ctxs >= min_rss_ctxs)
--
1.8.3.1
^ 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