* Re: [PATCH] cnic: Fix an error handling path in 'cnic_alloc_bnx2x_resc()'
From: David Miller @ 2017-09-24 0:04 UTC (permalink / raw)
To: christophe.jaillet
Cc: jmaxwell37, stephen, danielj, parav, netdev, linux-kernel,
kernel-janitors
In-Reply-To: <20170921230111.10135-1-christophe.jaillet@wanadoo.fr>
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Date: Fri, 22 Sep 2017 01:01:11 +0200
> All the error handling paths 'goto error', except this one.
> We should also go to error in this case, or some resources will be
> leaking.
>
> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next v3 3/6] rtnetlink: add helper to dump ifalias
From: Florian Westphal @ 2017-09-23 21:21 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Florian Westphal, netdev
In-Reply-To: <1506200690.29839.217.camel@edumazet-glaptop3.roam.corp.google.com>
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sat, 2017-09-23 at 21:26 +0200, Florian Westphal wrote:
> > Reviewed-by: David Ahern <dsahern@gmail.com>
> > Signed-off-by: Florian Westphal <fw@strlen.de>
> > ---
> > Changes since v3: don't add rtnl assertion, I placed the assertion
> > in my working tree instead as a reminder.
> >
> > net/core/rtnetlink.c | 11 +++++++++--
> > 1 file changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> > index c801212ee40e..47c17c3de79a 100644
> > --- a/net/core/rtnetlink.c
> > +++ b/net/core/rtnetlink.c
> > @@ -1332,6 +1332,14 @@ static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev)
> > return nla_put_u32(skb, IFLA_LINK, ifindex);
> > }
> >
> > +static noinline int nla_put_ifalias(struct sk_buff *skb, struct net_device *dev)
>
>
> Why noinline here ?
>
> This function does not use stack at all (and that would call for
> noinline_for_stack )
>
> > +{
> > + if (dev->ifalias)
> > + return nla_put_string(skb, IFLA_IFALIAS, dev->ifalias);
> > +
> > + return 0;
> > +}
> > +
>
> I really do not see the point of not making this RCU aware right away,
> or at least make it in the same patch series...
I saw no point to mix these refactoring with actual change :-|
(and it doesn't help either as-is with netlink dumping, only sysfs path can
elide rtnl).
Subject: [PATCH net-next] net: core: decouple ifalias get/set from rtnl lock
Device alias can be set by either rtnetlink (rtnl is held) or sysfs.
rtnetlink holds rtnl mutex, sysfs acquires it for this purpose.
Add a new mutex for it plus a seqcount to get a consistent snapshot
of the alias buffer.
This allows the sysfs path to not take rtnl and would later allow
to not hold it when dumping ifalias.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
include/linux/netdevice.h | 3 +-
net/core/dev.c | 70 +++++++++++++++++++++++++++++++++++++++--------
net/core/net-sysfs.c | 14 ++++------
net/core/rtnetlink.c | 7 +++--
4 files changed, 70 insertions(+), 24 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index f535779d9dc1..0bcedb498f6f 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1632,7 +1632,7 @@ enum netdev_priv_flags {
struct net_device {
char name[IFNAMSIZ];
struct hlist_node name_hlist;
- char *ifalias;
+ char __rcu *ifalias;
/*
* I/O specific fields
* FIXME: Merge these and struct ifmap into one
@@ -3275,6 +3275,7 @@ void __dev_notify_flags(struct net_device *, unsigned int old_flags,
unsigned int gchanges);
int dev_change_name(struct net_device *, const char *);
int dev_set_alias(struct net_device *, const char *, size_t);
+int dev_get_alias(const struct net_device *, char *, size_t);
int dev_change_net_namespace(struct net_device *, struct net *, const char *);
int __dev_set_mtu(struct net_device *, int);
int dev_set_mtu(struct net_device *, int);
diff --git a/net/core/dev.c b/net/core/dev.c
index 97abddd9039a..92d87b4a2db1 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -188,6 +188,9 @@ static struct napi_struct *napi_by_id(unsigned int napi_id);
DEFINE_RWLOCK(dev_base_lock);
EXPORT_SYMBOL(dev_base_lock);
+static DEFINE_MUTEX(ifalias_mutex);
+static seqcount_t ifalias_rename_seq;
+
/* protects napi_hash addition/deletion and napi_gen_id */
static DEFINE_SPINLOCK(napi_hash_lock);
@@ -1265,29 +1268,72 @@ int dev_change_name(struct net_device *dev, const char *newname)
*/
int dev_set_alias(struct net_device *dev, const char *alias, size_t len)
{
- char *new_ifalias;
-
- ASSERT_RTNL();
+ char *new_ifalias, *old_ifalias;
if (len >= IFALIASZ)
return -EINVAL;
+ mutex_lock(&ifalias_mutex);
+
+ old_ifalias = rcu_dereference_protected(dev->ifalias,
+ mutex_is_locked(&ifalias_mutex));
if (!len) {
- kfree(dev->ifalias);
- dev->ifalias = NULL;
- return 0;
+ RCU_INIT_POINTER(dev->ifalias, NULL);
+ goto out;
}
- new_ifalias = krealloc(dev->ifalias, len + 1, GFP_KERNEL);
- if (!new_ifalias)
+ new_ifalias = __krealloc(old_ifalias, len + 1, GFP_KERNEL);
+ if (!new_ifalias) {
+ mutex_unlock(&ifalias_mutex);
return -ENOMEM;
- dev->ifalias = new_ifalias;
- memcpy(dev->ifalias, alias, len);
- dev->ifalias[len] = 0;
+ }
+ if (new_ifalias == old_ifalias) {
+ write_seqcount_begin(&ifalias_rename_seq);
+ memcpy(new_ifalias, alias, len);
+ new_ifalias[len] = 0;
+ write_seqcount_end(&ifalias_rename_seq);
+ mutex_unlock(&ifalias_mutex);
+ return len;
+ }
+
+ memcpy(new_ifalias, alias, len);
+ new_ifalias[len] = 0;
+
+ rcu_assign_pointer(dev->ifalias, new_ifalias);
+out:
+ mutex_unlock(&ifalias_mutex);
+ if (old_ifalias) {
+ synchronize_net();
+ kfree(old_ifalias);
+ }
return len;
}
+int dev_get_alias(const struct net_device *dev, char *alias, size_t len)
+{
+ unsigned int seq;
+ int ret;
+
+ for (;;) {
+ const char *name;
+
+ ret = 0;
+ rcu_read_lock();
+ name = rcu_dereference(dev->ifalias);
+ seq = raw_seqcount_begin(&ifalias_rename_seq);
+ if (name)
+ ret = snprintf(alias, len, "%s", name);
+ rcu_read_unlock();
+
+ if (!read_seqcount_retry(&ifalias_rename_seq, seq))
+ break;
+
+ cond_resched();
+ }
+
+ return ret;
+}
/**
* netdev_features_change - device changes features
@@ -8749,6 +8795,8 @@ static int __init net_dev_init(void)
NULL, dev_cpu_dead);
WARN_ON(rc < 0);
rc = 0;
+
+ seqcount_init(&ifalias_rename_seq);
out:
return rc;
}
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index 927a6dcbad96..530de7996d65 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -391,10 +391,7 @@ static ssize_t ifalias_store(struct device *dev, struct device_attribute *attr,
if (len > 0 && buf[len - 1] == '\n')
--count;
- if (!rtnl_trylock())
- return restart_syscall();
ret = dev_set_alias(netdev, buf, count);
- rtnl_unlock();
return ret < 0 ? ret : len;
}
@@ -403,13 +400,12 @@ static ssize_t ifalias_show(struct device *dev,
struct device_attribute *attr, char *buf)
{
const struct net_device *netdev = to_net_dev(dev);
+ char tmp[IFALIASZ];
ssize_t ret = 0;
- if (!rtnl_trylock())
- return restart_syscall();
- if (netdev->ifalias)
- ret = sprintf(buf, "%s\n", netdev->ifalias);
- rtnl_unlock();
+ ret = dev_get_alias(netdev, tmp, sizeof(tmp));
+ if (ret > 0)
+ ret = sprintf(buf, "%s\n", tmp);
return ret;
}
static DEVICE_ATTR_RW(ifalias);
@@ -1488,7 +1484,7 @@ static void netdev_release(struct device *d)
BUG_ON(dev->reg_state != NETREG_RELEASED);
- kfree(dev->ifalias);
+ dev_set_alias(dev, "", 0);
netdev_freemem(dev);
}
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index c69451964a44..bab108ced7d8 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1368,10 +1368,11 @@ static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev)
static noinline int nla_put_ifalias(struct sk_buff *skb, struct net_device *dev)
{
- if (dev->ifalias)
- return nla_put_string(skb, IFLA_IFALIAS, dev->ifalias);
+ char buf[IFALIASZ];
+ int ret;
- return 0;
+ ret = dev_get_alias(dev, buf, sizeof(buf));
+ return ret > 0 ? nla_put_string(skb, IFLA_IFALIAS, buf) : 0;
}
static noinline int rtnl_fill_link_netnsid(struct sk_buff *skb,
--
2.13.5
^ permalink raw reply related
* Re: [PATCH net-next v3 3/6] rtnetlink: add helper to dump ifalias
From: Eric Dumazet @ 2017-09-23 21:04 UTC (permalink / raw)
To: Florian Westphal; +Cc: netdev
In-Reply-To: <20170923192636.3932-4-fw@strlen.de>
On Sat, 2017-09-23 at 21:26 +0200, Florian Westphal wrote:
> Reviewed-by: David Ahern <dsahern@gmail.com>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> Changes since v3: don't add rtnl assertion, I placed the assertion
> in my working tree instead as a reminder.
>
> net/core/rtnetlink.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index c801212ee40e..47c17c3de79a 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1332,6 +1332,14 @@ static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev)
> return nla_put_u32(skb, IFLA_LINK, ifindex);
> }
>
> +static noinline int nla_put_ifalias(struct sk_buff *skb, struct net_device *dev)
Why noinline here ?
This function does not use stack at all (and that would call for
noinline_for_stack )
> +{
> + if (dev->ifalias)
> + return nla_put_string(skb, IFLA_IFALIAS, dev->ifalias);
> +
> + return 0;
> +}
> +
I really do not see the point of not making this RCU aware right away,
or at least make it in the same patch series...
^ permalink raw reply
* [PATCH] au1k_ir.c fix warning: Prefer [subsystem eg: netdev]_info([subsystem]dev, ...
From: Yurii Pavlenko @ 2017-09-23 20:13 UTC (permalink / raw)
To: samuel
Cc: gregkh, davem, manuel.lauss, netdev, devel, linux-kernel,
Yurii Pavlenko
This patch fixes the following checkpatch.pl warning: fix
Prefer [subsystem eg: netdev]_info([subsystem]dev, ... then dev_info(dev, ...
then pr_info(... to printk(KERN_INFO ...
Signed-off-by: Yurii Pavlenko <pyldev@gmail.com>
---
drivers/staging/irda/drivers/au1k_ir.c | 40 +++++++++++++++-------------------
1 file changed, 18 insertions(+), 22 deletions(-)
diff --git a/drivers/staging/irda/drivers/au1k_ir.c b/drivers/staging/irda/drivers/au1k_ir.c
index be4ea6a..73e3e4b 100644
--- a/drivers/staging/irda/drivers/au1k_ir.c
+++ b/drivers/staging/irda/drivers/au1k_ir.c
@@ -290,8 +290,7 @@ static int au1k_irda_set_speed(struct net_device *dev, int speed)
while (irda_read(aup, IR_STATUS) & (IR_RX_STATUS | IR_TX_STATUS)) {
msleep(20);
if (!timeout--) {
- printk(KERN_ERR "%s: rx/tx disable timeout\n",
- dev->name);
+ netdev_err(dev, "rx/tx disable timeout\n");
break;
}
}
@@ -349,7 +348,7 @@ static int au1k_irda_set_speed(struct net_device *dev, int speed)
IR_RX_ENABLE);
break;
default:
- printk(KERN_ERR "%s unsupported speed %x\n", dev->name, speed);
+ netdev_err(dev, "unsupported speed %x\n", speed);
ret = -EINVAL;
break;
}
@@ -361,18 +360,18 @@ static int au1k_irda_set_speed(struct net_device *dev, int speed)
irda_write(aup, IR_RING_PROMPT, 0);
if (control & (1 << 14)) {
- printk(KERN_ERR "%s: configuration error\n", dev->name);
+ netdev_err(dev, "configuration error\n");
} else {
if (control & (1 << 11))
- printk(KERN_DEBUG "%s Valid SIR config\n", dev->name);
+ netdev_debug(dev, "Valid SIR config\n");
if (control & (1 << 12))
- printk(KERN_DEBUG "%s Valid MIR config\n", dev->name);
+ netdev_debug(dev, "Valid MIR config\n");
if (control & (1 << 13))
- printk(KERN_DEBUG "%s Valid FIR config\n", dev->name);
+ netdev_debug(dev, "Valid FIR config\n");
if (control & (1 << 10))
- printk(KERN_DEBUG "%s TX enabled\n", dev->name);
+ netdev_debug(dev, "TX enabled\n");
if (control & (1 << 9))
- printk(KERN_DEBUG "%s RX enabled\n", dev->name);
+ netdev_debug(dev, "RX enabled\n");
}
return ret;
@@ -584,23 +583,21 @@ static int au1k_irda_start(struct net_device *dev)
retval = au1k_init(dev);
if (retval) {
- printk(KERN_ERR "%s: error in au1k_init\n", dev->name);
+ netdev_err(dev, "error in au1k_init\n");
return retval;
}
retval = request_irq(aup->irq_tx, &au1k_irda_interrupt, 0,
dev->name, dev);
if (retval) {
- printk(KERN_ERR "%s: unable to get IRQ %d\n",
- dev->name, dev->irq);
+ netdev_err(dev, "unable to get IRQ %d\n", dev->irq);
return retval;
}
retval = request_irq(aup->irq_rx, &au1k_irda_interrupt, 0,
dev->name, dev);
if (retval) {
free_irq(aup->irq_tx, dev);
- printk(KERN_ERR "%s: unable to get IRQ %d\n",
- dev->name, dev->irq);
+ netdev_err(dev, "unable to get IRQ %d\n", dev->irq);
return retval;
}
@@ -673,12 +670,12 @@ static int au1k_irda_hard_xmit(struct sk_buff *skb, struct net_device *dev)
flags = ptxd->flags;
if (flags & AU_OWN) {
- printk(KERN_DEBUG "%s: tx_full\n", dev->name);
+ netdev_debug(dev, "tx_full\n");
netif_stop_queue(dev);
aup->tx_full = 1;
return 1;
} else if (((aup->tx_head + 1) & (NUM_IR_DESC - 1)) == aup->tx_tail) {
- printk(KERN_DEBUG "%s: tx_full\n", dev->name);
+ netdev_debug(dev, "tx_full\n");
netif_stop_queue(dev);
aup->tx_full = 1;
return 1;
@@ -688,7 +685,7 @@ static int au1k_irda_hard_xmit(struct sk_buff *skb, struct net_device *dev)
#if 0
if (irda_read(aup, IR_RX_BYTE_CNT) != 0) {
- printk(KERN_DEBUG "tx warning: rx byte cnt %x\n",
+ netdev_debug(dev, "tx warning: rx byte cnt %x\n",
irda_read(aup, IR_RX_BYTE_CNT));
}
#endif
@@ -726,7 +723,7 @@ static void au1k_tx_timeout(struct net_device *dev)
u32 speed;
struct au1k_private *aup = netdev_priv(dev);
- printk(KERN_ERR "%s: tx timeout\n", dev->name);
+ netdev_err(dev, "tx timeout\n");
speed = aup->speed;
aup->speed = 0;
au1k_irda_set_speed(dev, speed);
@@ -751,8 +748,7 @@ static int au1k_irda_ioctl(struct net_device *dev, struct ifreq *ifreq, int cmd)
ret = au1k_irda_set_speed(dev,
rq->ifr_baudrate);
else {
- printk(KERN_ERR "%s ioctl: !netif_running\n",
- dev->name);
+ netdev_err(dev, "ioctl: !netif_running\n");
ret = 0;
}
}
@@ -868,7 +864,7 @@ static int au1k_irda_net_init(struct net_device *dev)
out2:
kfree(aup->rx_buff.head);
out1:
- printk(KERN_ERR "au1k_irda_net_init() failed. Returns %d\n", retval);
+ netdev_err(dev, "au1k_irda_net_init() failed. Returns %d\n");
return retval;
}
@@ -934,7 +930,7 @@ static int au1k_irda_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, dev);
- printk(KERN_INFO "IrDA: Registered device %s\n", dev->name);
+ netdev_info(dev, "IrDA: Registered device\n");
return 0;
out4:
--
2.7.4
^ permalink raw reply related
* [PATCH 2/2] neigh: make strucrt neigh_table::entry_size unsigned int
From: Alexey Dobriyan @ 2017-09-23 20:03 UTC (permalink / raw)
To: davem; +Cc: netdev, ganeshgr
In-Reply-To: <20170923200106.GA24928@avx2>
Key length can't be negative.
Leave comparisons against nla_len() signed just in case truncated attribute
can sneak in there.
Space savings:
add/remove: 0/0 grow/shrink: 0/7 up/down: 0/-7 (-7)
function old new delta
pneigh_delete 273 272 -1
mlx5e_rep_netevent_event 1415 1414 -1
mlx5e_create_encap_header_ipv6 1194 1193 -1
mlx5e_create_encap_header_ipv4 1071 1070 -1
cxgb4_l2t_get 1104 1103 -1
__pneigh_lookup 69 68 -1
__neigh_create 2452 2451 -1
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
drivers/net/ethernet/chelsio/cxgb4/l2t.c | 4 ++--
include/net/neighbour.h | 2 +-
net/core/neighbour.c | 18 +++++++++---------
3 files changed, 12 insertions(+), 12 deletions(-)
--- a/drivers/net/ethernet/chelsio/cxgb4/l2t.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/l2t.c
@@ -422,7 +422,7 @@ struct l2t_entry *cxgb4_l2t_get(struct l2t_data *d, struct neighbour *neigh,
u8 lport;
u16 vlan;
struct l2t_entry *e;
- int addr_len = neigh->tbl->key_len;
+ unsigned int addr_len = neigh->tbl->key_len;
u32 *addr = (u32 *)neigh->primary_key;
int ifidx = neigh->dev->ifindex;
int hash = addr_hash(d, addr, addr_len, ifidx);
@@ -536,7 +536,7 @@ void t4_l2t_update(struct adapter *adap, struct neighbour *neigh)
struct l2t_entry *e;
struct sk_buff_head *arpq = NULL;
struct l2t_data *d = adap->l2t;
- int addr_len = neigh->tbl->key_len;
+ unsigned int addr_len = neigh->tbl->key_len;
u32 *addr = (u32 *) neigh->primary_key;
int ifidx = neigh->dev->ifindex;
int hash = addr_hash(d, addr, addr_len, ifidx);
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -191,7 +191,7 @@ struct neigh_hash_table {
struct neigh_table {
int family;
unsigned int entry_size;
- int key_len;
+ unsigned int key_len;
__be16 protocol;
__u32 (*hash)(const void *pkey,
const struct net_device *dev,
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -457,7 +457,7 @@ struct neighbour *neigh_lookup_nodev(struct neigh_table *tbl, struct net *net,
const void *pkey)
{
struct neighbour *n;
- int key_len = tbl->key_len;
+ unsigned int key_len = tbl->key_len;
u32 hash_val;
struct neigh_hash_table *nht;
@@ -488,7 +488,7 @@ struct neighbour *__neigh_create(struct neigh_table *tbl, const void *pkey,
struct net_device *dev, bool want_ref)
{
u32 hash_val;
- int key_len = tbl->key_len;
+ unsigned int key_len = tbl->key_len;
int error;
struct neighbour *n1, *rc, *n = neigh_alloc(tbl, dev);
struct neigh_hash_table *nht;
@@ -572,7 +572,7 @@ struct neighbour *__neigh_create(struct neigh_table *tbl, const void *pkey,
}
EXPORT_SYMBOL(__neigh_create);
-static u32 pneigh_hash(const void *pkey, int key_len)
+static u32 pneigh_hash(const void *pkey, unsigned int key_len)
{
u32 hash_val = *(u32 *)(pkey + key_len - 4);
hash_val ^= (hash_val >> 16);
@@ -585,7 +585,7 @@ static u32 pneigh_hash(const void *pkey, int key_len)
static struct pneigh_entry *__pneigh_lookup_1(struct pneigh_entry *n,
struct net *net,
const void *pkey,
- int key_len,
+ unsigned int key_len,
struct net_device *dev)
{
while (n) {
@@ -601,7 +601,7 @@ static struct pneigh_entry *__pneigh_lookup_1(struct pneigh_entry *n,
struct pneigh_entry *__pneigh_lookup(struct neigh_table *tbl,
struct net *net, const void *pkey, struct net_device *dev)
{
- int key_len = tbl->key_len;
+ unsigned int key_len = tbl->key_len;
u32 hash_val = pneigh_hash(pkey, key_len);
return __pneigh_lookup_1(tbl->phash_buckets[hash_val],
@@ -614,7 +614,7 @@ struct pneigh_entry * pneigh_lookup(struct neigh_table *tbl,
struct net_device *dev, int creat)
{
struct pneigh_entry *n;
- int key_len = tbl->key_len;
+ unsigned int key_len = tbl->key_len;
u32 hash_val = pneigh_hash(pkey, key_len);
read_lock_bh(&tbl->lock);
@@ -659,7 +659,7 @@ int pneigh_delete(struct neigh_table *tbl, struct net *net, const void *pkey,
struct net_device *dev)
{
struct pneigh_entry *n, **np;
- int key_len = tbl->key_len;
+ unsigned int key_len = tbl->key_len;
u32 hash_val = pneigh_hash(pkey, key_len);
write_lock_bh(&tbl->lock);
@@ -1662,7 +1662,7 @@ static int neigh_delete(struct sk_buff *skb, struct nlmsghdr *nlh,
if (tbl == NULL)
return -EAFNOSUPPORT;
- if (nla_len(dst_attr) < tbl->key_len)
+ if (nla_len(dst_attr) < (int)tbl->key_len)
goto out;
if (ndm->ndm_flags & NTF_PROXY) {
@@ -1730,7 +1730,7 @@ static int neigh_add(struct sk_buff *skb, struct nlmsghdr *nlh,
if (tbl == NULL)
return -EAFNOSUPPORT;
- if (nla_len(tb[NDA_DST]) < tbl->key_len)
+ if (nla_len(tb[NDA_DST]) < (int)tbl->key_len)
goto out;
dst = nla_data(tb[NDA_DST]);
lladdr = tb[NDA_LLADDR] ? nla_data(tb[NDA_LLADDR]) : NULL;
^ permalink raw reply
* [PATCH 1/2] neigh: make struct neigh_table::entry_size unsigned int
From: Alexey Dobriyan @ 2017-09-23 20:01 UTC (permalink / raw)
To: davem; +Cc: netdev
Neigh entry size can't be negative.
Space savings:
add/remove: 0/0 grow/shrink: 0/5 up/down: 0/-7 (-7)
function old new delta
lowpan_neigh_construct 25 24 -1
clip_seq_sub_iter 152 151 -1
clip_ioctl 1475 1474 -1
clip_constructor 93 92 -1
__neigh_create 2455 2452 -3
Signed-off-by: Alexey Dobriyan <adobriyan@gmail.com>
---
include/net/neighbour.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -190,7 +190,7 @@ struct neigh_hash_table {
struct neigh_table {
int family;
- int entry_size;
+ unsigned int entry_size;
int key_len;
__be16 protocol;
__u32 (*hash)(const void *pkey,
^ permalink raw reply
* [PATCH net-next] net: speed up skb_rbtree_purge()
From: Eric Dumazet @ 2017-09-23 19:39 UTC (permalink / raw)
To: David Miller; +Cc: netdev
From: Eric Dumazet <edumazet@google.com>
As measured in my prior patch ("sch_netem: faster rb tree removal"),
rbtree_postorder_for_each_entry_safe() is nice looking but much slower
than using rb_next() directly, except when tree is small enough
to fit in CPU caches (then the cost is the same)
Also note that there is not even an increase of text size :
$ size net/core/skbuff.o.before net/core/skbuff.o
text data bss dec hex filename
40711 1298 0 42009 a419 net/core/skbuff.o.before
40711 1298 0 42009 a419 net/core/skbuff.o
From: Eric Dumazet <edumazet@google.com>
---
net/core/skbuff.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 16982de649b97b92423a4f9f5eac1e98ca803370..000ce735fa8d649e7abeeef2ebab8501dea96efd 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -2848,12 +2848,15 @@ EXPORT_SYMBOL(skb_queue_purge);
*/
void skb_rbtree_purge(struct rb_root *root)
{
- struct sk_buff *skb, *next;
+ struct rb_node *p = rb_first(root);
- rbtree_postorder_for_each_entry_safe(skb, next, root, rbnode)
- kfree_skb(skb);
+ while (p) {
+ struct sk_buff *skb = rb_entry(p, struct sk_buff, rbnode);
- *root = RB_ROOT;
+ p = rb_next(p);
+ rb_erase(&skb->rbnode, root);
+ kfree_skb(skb);
+ }
}
/**
^ permalink raw reply related
* [RESEND] Re: usb/net/p54: trying to register non-static key in p54_unregister_leds
From: Christian Lamparter @ 2017-09-23 19:37 UTC (permalink / raw)
To: Andrey Konovalov
Cc: Johannes Berg, Kalle Valo, linux-wireless, netdev, LKML,
Dmitry Vyukov, Kostya Serebryany, syzkaller, Stephen Boyd,
Tejun Heo, Yong Zhang
In-Reply-To: <CAAeHK+wWRZuCDNhoZJ0ACDhiNQw8tVRp2HW0j1EDH0HOe523xA@mail.gmail.com>
This got rejected by gmail once. Let's see if it works now.
On Thursday, September 21, 2017 8:22:45 PM CEST Andrey Konovalov wrote:
> On Wed, Sep 20, 2017 at 9:55 PM, Johannes Berg
> <johannes@sipsolutions.net> wrote:
> > On Wed, 2017-09-20 at 21:27 +0200, Christian Lamparter wrote:
> >
> >> It seems this is caused as a result of:
> >> -> lock_map_acquire(&work->lockdep_map);
> >> lock_map_release(&work->lockdep_map);
> >>
> >> in flush_work() [0]
> >
> > Agree.
> >
> >> This was added by:
> >>
> >> commit 0976dfc1d0cd80a4e9dfaf87bd8744612bde475a
> >> Author: Stephen Boyd <sboyd@codeaurora.org>
> >> Date: Fri Apr 20 17:28:50 2012 -0700
> >>
> >> workqueue: Catch more locking problems with flush_work()
> >
> > Yes, but that doesn't matter.
> >
> >> Looking at the Stephen's patch, it's clear that it was made
> >> with "static DECLARE_WORK(work, my_work)" in mind. However
> >> p54's led_work is "per-device", hence it is stored in the
> >> devices context p54_common, which is dynamically allocated.
> >> So, maybe revert Stephen's patch?
> >
> > I disagree - as the lockdep warning says:
> >
> >> > INFO: trying to register non-static key.
> >> > the code is fine but needs lockdep annotation.
> >> > turning off the locking correctness validator.
> >
> > What it needs is to actually correctly go through initializing the work
> > at least once.
> >
> > Without more information, I can't really say what's going on, but I
> > assume that something is failing and p54_unregister_leds() is getting
> > invoked without p54_init_leds() having been invoked, so essentially
> > it's trying to flush a work that was never initialized?
> >
> > INIT_DELAYED_WORK() does, after all, initialize the lockdep map
> > properly via __INIT_WORK().
Ok, thanks. This does indeed explain it.
But this also begs the question: Is this really working then?
>From what I can tell, if CONFIG_LOCKDEP is not set then there's no BUG
no WARN, no other splat or any other odd system behaviour. Does
[cancel | flush]_[delayed_]work[_sync] really "just work" by *accident*,
as long the delayed_work | work_struct is zeroed out?
And should it work in the future as well?
> Since I'm able to reproduce this, please let me know if you need me to
> collect some debug traces to help with the triage.
Do you want to take a shot at making a patch too? At a quick glance, it
should be enough to move the [#ifdef CONFIG_P54_LEDS ... #endif] block
in p54_unregister_common() into the if (priv->registered) { block
(preferably before the ieee80211_unregister_hw(dev).
Regards,
Christian
^ permalink raw reply
* [PATCH net-next v3 6/6] rtnetlink: rtnl_have_link_slave_info doesn't need rtnl
From: Florian Westphal @ 2017-09-23 19:26 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170923192636.3932-1-fw@strlen.de>
it can be switched to rcu.
Reviewed-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
Changes since v2: remove ASSERT_RTNL.
net/core/rtnetlink.c | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index e858a2b48d7e..c69451964a44 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -522,11 +522,15 @@ static size_t rtnl_link_get_af_size(const struct net_device *dev,
static bool rtnl_have_link_slave_info(const struct net_device *dev)
{
struct net_device *master_dev;
+ bool ret = false;
- master_dev = netdev_master_upper_dev_get((struct net_device *) dev);
+ rcu_read_lock();
+
+ master_dev = netdev_master_upper_dev_get_rcu((struct net_device *)dev);
if (master_dev && master_dev->rtnl_link_ops)
- return true;
- return false;
+ ret = true;
+ rcu_read_unlock();
+ return ret;
}
static int rtnl_link_slave_info_fill(struct sk_buff *skb,
--
2.13.5
^ permalink raw reply related
* [PATCH net-next v3 5/6] rtnetlink: add helpers to dump netnsid information
From: Florian Westphal @ 2017-09-23 19:26 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170923192636.3932-1-fw@strlen.de>
Reviewed-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
Changes since v2:
this hunk was part of patch #4.
net/core/rtnetlink.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 625342d27c44..e858a2b48d7e 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1370,6 +1370,23 @@ static noinline int nla_put_ifalias(struct sk_buff *skb, struct net_device *dev)
return 0;
}
+static noinline int rtnl_fill_link_netnsid(struct sk_buff *skb,
+ const struct net_device *dev)
+{
+ if (dev->rtnl_link_ops && dev->rtnl_link_ops->get_link_net) {
+ struct net *link_net = dev->rtnl_link_ops->get_link_net(dev);
+
+ if (!net_eq(dev_net(dev), link_net)) {
+ int id = peernet2id_alloc(dev_net(dev), link_net);
+
+ if (nla_put_s32(skb, IFLA_LINK_NETNSID, id))
+ return -EMSGSIZE;
+ }
+ }
+
+ return 0;
+}
+
static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
int type, u32 pid, u32 seq, u32 change,
unsigned int flags, u32 ext_filter_mask,
@@ -1458,17 +1475,8 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
goto nla_put_failure;
}
- if (dev->rtnl_link_ops &&
- dev->rtnl_link_ops->get_link_net) {
- struct net *link_net = dev->rtnl_link_ops->get_link_net(dev);
-
- if (!net_eq(dev_net(dev), link_net)) {
- int id = peernet2id_alloc(dev_net(dev), link_net);
-
- if (nla_put_s32(skb, IFLA_LINK_NETNSID, id))
- goto nla_put_failure;
- }
- }
+ if (rtnl_fill_link_netnsid(skb, dev))
+ goto nla_put_failure;
if (!(af_spec = nla_nest_start(skb, IFLA_AF_SPEC)))
goto nla_put_failure;
--
2.13.5
^ permalink raw reply related
* [PATCH net-next v3 4/6] rtnetlink: add helpers to dump vf information
From: Florian Westphal @ 2017-09-23 19:26 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170923192636.3932-1-fw@strlen.de>
similar to earlier patches, split out more parts of this function to
better see what is happening and where we assume rtnl is locked.
Reviewed-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
changes since v2: split this patch into two, last submission also
added netnsid helper, which was moved to next patch.
net/core/rtnetlink.c | 50 +++++++++++++++++++++++++++++++-------------------
1 file changed, 31 insertions(+), 19 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 47c17c3de79a..625342d27c44 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1211,6 +1211,36 @@ static noinline_for_stack int rtnl_fill_vfinfo(struct sk_buff *skb,
return -EMSGSIZE;
}
+static noinline_for_stack int rtnl_fill_vf(struct sk_buff *skb,
+ struct net_device *dev,
+ u32 ext_filter_mask)
+{
+ struct nlattr *vfinfo;
+ int i, num_vfs;
+
+ if (!dev->dev.parent || ((ext_filter_mask & RTEXT_FILTER_VF) == 0))
+ return 0;
+
+ num_vfs = dev_num_vf(dev->dev.parent);
+ if (nla_put_u32(skb, IFLA_NUM_VF, num_vfs))
+ return -EMSGSIZE;
+
+ if (!dev->netdev_ops->ndo_get_vf_config)
+ return 0;
+
+ vfinfo = nla_nest_start(skb, IFLA_VFINFO_LIST);
+ if (!vfinfo)
+ return -EMSGSIZE;
+
+ for (i = 0; i < num_vfs; i++) {
+ if (rtnl_fill_vfinfo(skb, dev, i, vfinfo))
+ return -EMSGSIZE;
+ }
+
+ nla_nest_end(skb, vfinfo);
+ return 0;
+}
+
static int rtnl_fill_link_ifmap(struct sk_buff *skb, struct net_device *dev)
{
struct rtnl_link_ifmap map;
@@ -1414,27 +1444,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
if (rtnl_fill_stats(skb, dev))
goto nla_put_failure;
- if (dev->dev.parent && (ext_filter_mask & RTEXT_FILTER_VF) &&
- nla_put_u32(skb, IFLA_NUM_VF, dev_num_vf(dev->dev.parent)))
+ if (rtnl_fill_vf(skb, dev, ext_filter_mask))
goto nla_put_failure;
- if (dev->netdev_ops->ndo_get_vf_config && dev->dev.parent &&
- ext_filter_mask & RTEXT_FILTER_VF) {
- int i;
- struct nlattr *vfinfo;
- int num_vfs = dev_num_vf(dev->dev.parent);
-
- vfinfo = nla_nest_start(skb, IFLA_VFINFO_LIST);
- if (!vfinfo)
- goto nla_put_failure;
- for (i = 0; i < num_vfs; i++) {
- if (rtnl_fill_vfinfo(skb, dev, i, vfinfo))
- goto nla_put_failure;
- }
-
- nla_nest_end(skb, vfinfo);
- }
-
if (rtnl_port_fill(skb, dev, ext_filter_mask))
goto nla_put_failure;
--
2.13.5
^ permalink raw reply related
* [PATCH net-next v3 3/6] rtnetlink: add helper to dump ifalias
From: Florian Westphal @ 2017-09-23 19:26 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170923192636.3932-1-fw@strlen.de>
Reviewed-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
Changes since v3: don't add rtnl assertion, I placed the assertion
in my working tree instead as a reminder.
net/core/rtnetlink.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index c801212ee40e..47c17c3de79a 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1332,6 +1332,14 @@ static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev)
return nla_put_u32(skb, IFLA_LINK, ifindex);
}
+static noinline int nla_put_ifalias(struct sk_buff *skb, struct net_device *dev)
+{
+ if (dev->ifalias)
+ return nla_put_string(skb, IFLA_IFALIAS, dev->ifalias);
+
+ return 0;
+}
+
static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
int type, u32 pid, u32 seq, u32 change,
unsigned int flags, u32 ext_filter_mask,
@@ -1374,8 +1382,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
nla_put_u8(skb, IFLA_CARRIER, netif_carrier_ok(dev)) ||
(dev->qdisc &&
nla_put_string(skb, IFLA_QDISC, dev->qdisc->ops->id)) ||
- (dev->ifalias &&
- nla_put_string(skb, IFLA_IFALIAS, dev->ifalias)) ||
+ nla_put_ifalias(skb, dev) ||
nla_put_u32(skb, IFLA_CARRIER_CHANGES,
atomic_read(&dev->carrier_changes)) ||
nla_put_u8(skb, IFLA_PROTO_DOWN, dev->proto_down))
--
2.13.5
^ permalink raw reply related
* [PATCH net-next v3 2/6] rtnetlink: add helper to put master and link ifindexes
From: Florian Westphal @ 2017-09-23 19:26 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170923192636.3932-1-fw@strlen.de>
rtnl_fill_ifinfo currently requires caller to hold the rtnl mutex.
Unfortunately the function is quite large which makes it harder to see
which spots require the lock, which spots assume it and which ones could
do without.
Add helpers to factor out the ifindex dumping, one can use rcu to avoid
rtnl dependency.
Reviewed-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
no changes since v2.
net/core/rtnetlink.c | 32 +++++++++++++++++++++++++++-----
1 file changed, 27 insertions(+), 5 deletions(-)
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index a78fd61da0ec..c801212ee40e 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1307,6 +1307,31 @@ static u32 rtnl_get_event(unsigned long event)
return rtnl_event_type;
}
+static int put_master_ifindex(struct sk_buff *skb, struct net_device *dev)
+{
+ const struct net_device *upper_dev;
+ int ret = 0;
+
+ rcu_read_lock();
+
+ upper_dev = netdev_master_upper_dev_get_rcu(dev);
+ if (upper_dev)
+ ret = nla_put_u32(skb, IFLA_MASTER, upper_dev->ifindex);
+
+ rcu_read_unlock();
+ return ret;
+}
+
+static int nla_put_iflink(struct sk_buff *skb, const struct net_device *dev)
+{
+ int ifindex = dev_get_iflink(dev);
+
+ if (dev->ifindex == ifindex)
+ return 0;
+
+ return nla_put_u32(skb, IFLA_LINK, ifindex);
+}
+
static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
int type, u32 pid, u32 seq, u32 change,
unsigned int flags, u32 ext_filter_mask,
@@ -1316,7 +1341,6 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
struct nlmsghdr *nlh;
struct nlattr *af_spec;
struct rtnl_af_ops *af_ops;
- struct net_device *upper_dev = netdev_master_upper_dev_get(dev);
ASSERT_RTNL();
nlh = nlmsg_put(skb, pid, seq, type, sizeof(*ifm), flags);
@@ -1345,10 +1369,8 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
#ifdef CONFIG_RPS
nla_put_u32(skb, IFLA_NUM_RX_QUEUES, dev->num_rx_queues) ||
#endif
- (dev->ifindex != dev_get_iflink(dev) &&
- nla_put_u32(skb, IFLA_LINK, dev_get_iflink(dev))) ||
- (upper_dev &&
- nla_put_u32(skb, IFLA_MASTER, upper_dev->ifindex)) ||
+ nla_put_iflink(skb, dev) ||
+ put_master_ifindex(skb, dev) ||
nla_put_u8(skb, IFLA_CARRIER, netif_carrier_ok(dev)) ||
(dev->qdisc &&
nla_put_string(skb, IFLA_QDISC, dev->qdisc->ops->id)) ||
--
2.13.5
^ permalink raw reply related
* [PATCH net-next v3 1/6] selftests: rtnetlink.sh: add rudimentary vrf test
From: Florian Westphal @ 2017-09-23 19:26 UTC (permalink / raw)
To: netdev; +Cc: Florian Westphal
In-Reply-To: <20170923192636.3932-1-fw@strlen.de>
Acked-by: David Ahern <dsahern@gmail.com>
Signed-off-by: Florian Westphal <fw@strlen.de>
---
Changes since v1: indent all lines with tabs, not spaces
tools/testing/selftests/net/rtnetlink.sh | 42 ++++++++++++++++++++++++++++++++
1 file changed, 42 insertions(+)
diff --git a/tools/testing/selftests/net/rtnetlink.sh b/tools/testing/selftests/net/rtnetlink.sh
index 4b48de565cae..a048f7a5f94c 100755
--- a/tools/testing/selftests/net/rtnetlink.sh
+++ b/tools/testing/selftests/net/rtnetlink.sh
@@ -291,6 +291,47 @@ kci_test_ifalias()
echo "PASS: set ifalias $namewant for $devdummy"
}
+kci_test_vrf()
+{
+ vrfname="test-vrf"
+ ret=0
+
+ ip link show type vrf 2>/dev/null
+ if [ $? -ne 0 ]; then
+ echo "SKIP: vrf: iproute2 too old"
+ return 0
+ fi
+
+ ip link add "$vrfname" type vrf table 10
+ check_err $?
+ if [ $ret -ne 0 ];then
+ echo "FAIL: can't add vrf interface, skipping test"
+ return 0
+ fi
+
+ ip -br link show type vrf | grep -q "$vrfname"
+ check_err $?
+ if [ $ret -ne 0 ];then
+ echo "FAIL: created vrf device not found"
+ return 1
+ fi
+
+ ip link set dev "$vrfname" up
+ check_err $?
+
+ ip link set dev "$devdummy" master "$vrfname"
+ check_err $?
+ ip link del dev "$vrfname"
+ check_err $?
+
+ if [ $ret -ne 0 ];then
+ echo "FAIL: vrf"
+ return 1
+ fi
+
+ echo "PASS: vrf"
+}
+
kci_test_rtnl()
{
kci_add_dummy
@@ -306,6 +347,7 @@ kci_test_rtnl()
kci_test_bridge
kci_test_addrlabel
kci_test_ifalias
+ kci_test_vrf
kci_del_dummy
}
--
2.13.5
^ permalink raw reply related
* [PATCH net-next v3 0/6] rtnetlink: preparation patches for further rtnl lock pushdown/removal
From: Florian Westphal @ 2017-09-23 19:26 UTC (permalink / raw)
To: netdev
First patch adds a rudimentary vrf test case.
Remaining patches split large rtnl_fill_ifinfo into smaller chunks
to better see which parts
1. require rtnl
2. do not require it at all
3. rely on rtnl locking now but could be converted
I removed all the ASSERT_RTNL spots that v1 and v2 added,
i will keep that back in my working branch since those
are just 'todo' markers for myself.
Eric Dumazet pointed out that qdiscs are now freed directly without
call_rcu so I dropped the patch that made this assumption from the series.
As the remaining patches did not see major changes vs. v2 I retained
all reviewed/acked-by tags from David Ahern.
^ permalink raw reply
* Re: [PATCH net-next v2 3/6] rtnetlink: add helper to dump qdisc name
From: Florian Westphal @ 2017-09-23 18:38 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Florian Westphal, netdev
In-Reply-To: <1506187888.29839.181.camel@edumazet-glaptop3.roam.corp.google.com>
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2017-09-22 at 08:10 +0200, Florian Westphal wrote:
> > We can use rcu here to make this safe even if we would not hold rtnl:
> > qdisc_destroy uses call_rcu to free the Qdisc struct.
>
>
> Where do you see call_rcu() called from qdisc_destroy() ?
>
> You missed this commit I guess
>
> 752fbcc33405d6f8249465e4b2c4e420091bb825
> ("net_sched: no need to free qdisc in RCU callback")
Indeed, I did, patch dropped, thanks for the heads-up.
^ permalink raw reply
* [PATCH net-next] sch_netem: faster rb tree removal
From: Eric Dumazet @ 2017-09-23 18:07 UTC (permalink / raw)
To: David Miller; +Cc: netdev, Stephen Hemminger
From: Eric Dumazet <edumazet@google.com>
While running TCP tests involving netem storing millions of packets,
I had the idea to speed up tfifo_reset() and did experiments.
I tried the rbtree_postorder_for_each_entry_safe() method that is
used in skb_rbtree_purge() but discovered it was slower than the
current tfifo_reset() method.
I measured time taken to release skbs with three occupation levels :
10^4, 10^5 and 10^6 skbs with three methods :
1) (current 'naive' method)
while ((p = rb_first(&q->t_root))) {
struct sk_buff *skb = netem_rb_to_skb(p);
rb_erase(p, &q->t_root);
rtnl_kfree_skbs(skb, skb);
}
2) Use rb_next() instead of rb_first() in the loop :
p = rb_first(&q->t_root);
while (p) {
struct sk_buff *skb = netem_rb_to_skb(p);
p = rb_next(p);
rb_erase(&skb->rbnode, &q->t_root);
rtnl_kfree_skbs(skb, skb);
}
3) "optimized" method using rbtree_postorder_for_each_entry_safe()
struct sk_buff *skb, *next;
rbtree_postorder_for_each_entry_safe(skb, next,
&q->t_root, rbnode) {
rtnl_kfree_skbs(skb, skb);
}
q->t_root = RB_ROOT;
Results :
method_1:while (rb_first()) rb_erase() 10000 skbs in 690378 ns (69 ns per skb)
method_2:rb_first; while (p) { p = rb_next(p); ...} 10000 skbs in 541846 ns (54 ns per skb)
method_3:rbtree_postorder_for_each_entry_safe() 10000 skbs in 868307 ns (86 ns per skb)
method_1:while (rb_first()) rb_erase() 99996 skbs in 7804021 ns (78 ns per skb)
method_2:rb_first; while (p) { p = rb_next(p); ...} 100000 skbs in 5942456 ns (59 ns per skb)
method_3:rbtree_postorder_for_each_entry_safe() 100000 skbs in 11584940 ns (115 ns per skb)
method_1:while (rb_first()) rb_erase() 1000000 skbs in 108577838 ns (108 ns per skb)
method_2:rb_first; while (p) { p = rb_next(p); ...} 1000000 skbs in 82619635 ns (82 ns per skb)
method_3:rbtree_postorder_for_each_entry_safe() 1000000 skbs in 127328743 ns (127 ns per skb)
Method 2) is simply faster, probably because it maintains a smaller
working size set.
Note that this is the method we use in tcp_ofo_queue() already.
I will also change skb_rbtree_purge() in a second patch.
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
net/sched/sch_netem.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c
index 063a4bdb9ee6f26b01387959e8f6ccd15ec16191..5a4f1008029068372019a965186e7a3c0a18aac3 100644
--- a/net/sched/sch_netem.c
+++ b/net/sched/sch_netem.c
@@ -361,12 +361,13 @@ static psched_time_t packet_len_2_sched_time(unsigned int len, struct netem_sche
static void tfifo_reset(struct Qdisc *sch)
{
struct netem_sched_data *q = qdisc_priv(sch);
- struct rb_node *p;
+ struct rb_node *p = rb_first(&q->t_root);
- while ((p = rb_first(&q->t_root))) {
+ while (p) {
struct sk_buff *skb = netem_rb_to_skb(p);
- rb_erase(p, &q->t_root);
+ p = rb_next(p);
+ rb_erase(&skb->rbnode, &q->t_root);
rtnl_kfree_skbs(skb, skb);
}
}
^ permalink raw reply related
* Re: [PATCH net-next v2 3/6] rtnetlink: add helper to dump qdisc name
From: Eric Dumazet @ 2017-09-23 17:31 UTC (permalink / raw)
To: Florian Westphal; +Cc: netdev
In-Reply-To: <20170922061008.14723-4-fw@strlen.de>
On Fri, 2017-09-22 at 08:10 +0200, Florian Westphal wrote:
> We can use rcu here to make this safe even if we would not hold rtnl:
> qdisc_destroy uses call_rcu to free the Qdisc struct.
Where do you see call_rcu() called from qdisc_destroy() ?
You missed this commit I guess
752fbcc33405d6f8249465e4b2c4e420091bb825
("net_sched: no need to free qdisc in RCU callback")
^ permalink raw reply
* Re: [PATCH] net: dsa: avoid null pointer dereference on p->phy
From: Florian Fainelli @ 2017-09-23 17:21 UTC (permalink / raw)
To: Colin King, Andrew Lunn, Vivien Didelot, David S . Miller, netdev
Cc: kernel-janitors, linux-kernel
In-Reply-To: <20170923165720.18560-1-colin.king@canonical.com>
On 09/23/2017 09:57 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently p->phy is being null checked in several places to avoid
> null pointer dereferences on p->phy, however, the final call
> to phy_attached_info on p->phy when p->phy will perform a null
> pointer dereference. Fix this by simply moving the call into
> the previous code block that is only executed if p->phy is
> not null.
>
> Detected by CoverityScan, CID#1457034 ("Dereference after null check")
The code flow is not exactly easy to read, but I don't see how we can
actually wind up in that situation because we check the return values of
of_phy_connect() and dsa_slave_phy_connect() earlier on.
>
> Fixes: 2220943a21e2 ("phy: Centralise print about attached phy")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> net/dsa/slave.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 02ace7d462c4..29ab4e98639b 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -1115,10 +1115,9 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev)
> of_phy_deregister_fixed_link(port_dn);
> return ret;
> }
> + phy_attached_info(p->phy);
> }
>
> - phy_attached_info(p->phy);
> -
> return 0;
> }
>
>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next v2 6/6] rtnetlink: rtnl_have_link_slave_info doesn't need rtnl
From: David Ahern @ 2017-09-23 17:17 UTC (permalink / raw)
To: Florian Westphal, netdev
In-Reply-To: <20170922061008.14723-7-fw@strlen.de>
On 9/22/17 12:10 AM, Florian Westphal wrote:
> Switch it to rcu.
>
> rtnl_link_slave_info_fill on to other hand does need rtnl mutex for now so
> add an annotation to its caller as a reminder.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> Change since v1:
> - don't add ASSERT_RTNL to rtnl_link_slave_info_fill and
> rtnl_link_info_fill they are only called via rtnl_link_fill.
>
> net/core/rtnetlink.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 590823f70cc3..e6f9e36d9d5a 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -522,11 +522,15 @@ static size_t rtnl_link_get_af_size(const struct net_device *dev,
> static bool rtnl_have_link_slave_info(const struct net_device *dev)
> {
> struct net_device *master_dev;
> + bool ret = false;
>
> - master_dev = netdev_master_upper_dev_get((struct net_device *) dev);
> + rcu_read_lock();
> +
> + master_dev = netdev_master_upper_dev_get_rcu((struct net_device *)dev);
> if (master_dev && master_dev->rtnl_link_ops)
> - return true;
> - return false;
> + ret = true;
> + rcu_read_unlock();
> + return ret;
> }
>
> static int rtnl_link_slave_info_fill(struct sk_buff *skb,
> @@ -598,6 +602,8 @@ static int rtnl_link_fill(struct sk_buff *skb, const struct net_device *dev)
> struct nlattr *linkinfo;
> int err = -EMSGSIZE;
>
> + ASSERT_RTNL();
Rather than sprinkling the ASSERT_RTNL why not just add a comment above
the function which is done in many places. Since this is really meant as
your notes as you remove rtnl usage a comment serves the same purpose.
> +
> linkinfo = nla_nest_start(skb, IFLA_LINKINFO);
> if (linkinfo == NULL)
> goto out;
>
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [PATCH net-next v2 4/6] rtnetlink: add helper to dump ifalias
From: David Ahern @ 2017-09-23 17:14 UTC (permalink / raw)
To: Florian Westphal, netdev
In-Reply-To: <20170922061008.14723-5-fw@strlen.de>
On 9/22/17 12:10 AM, Florian Westphal wrote:
> ifalias is currently protected by rtnl mutex, add assertion
> as a reminder.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> net/core/rtnetlink.c | 13 +++++++++++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index ad3f27da37a8..42ff582a010e 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1345,6 +1345,16 @@ static int nla_put_qdisc(struct sk_buff *skb, struct net_device *dev)
> return ret;
> }
>
> +static noinline int nla_put_ifalias(struct sk_buff *skb, struct net_device *dev)
> +{
> + ASSERT_RTNL();
The assert is not needed given the code path.
> +
> + if (dev->ifalias)
> + return nla_put_string(skb, IFLA_IFALIAS, dev->ifalias);
> +
> + return 0;
> +}
> +
> static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
> int type, u32 pid, u32 seq, u32 change,
> unsigned int flags, u32 ext_filter_mask,
> @@ -1386,8 +1396,7 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
> put_master_ifindex(skb, dev) ||
> nla_put_u8(skb, IFLA_CARRIER, netif_carrier_ok(dev)) ||
> nla_put_qdisc(skb, dev) ||
> - (dev->ifalias &&
> - nla_put_string(skb, IFLA_IFALIAS, dev->ifalias)) ||
> + nla_put_ifalias(skb, dev) ||
> nla_put_u32(skb, IFLA_CARRIER_CHANGES,
> atomic_read(&dev->carrier_changes)) ||
> nla_put_u8(skb, IFLA_PROTO_DOWN, dev->proto_down))
>
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [PATCH] net: dsa: avoid null pointer dereference on p->phy
From: Joe Perches @ 2017-09-23 17:13 UTC (permalink / raw)
To: Colin King, Andrew Lunn, Vivien Didelot, Florian Fainelli,
David S . Miller, netdev
Cc: kernel-janitors, linux-kernel
In-Reply-To: <20170923165720.18560-1-colin.king@canonical.com>
On Sat, 2017-09-23 at 17:57 +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Currently p->phy is being null checked in several places to avoid
> null pointer dereferences on p->phy, however, the final call
> to phy_attached_info on p->phy when p->phy will perform a null
> pointer dereference. Fix this by simply moving the call into
> the previous code block that is only executed if p->phy is
> not null.
>
> Detected by CoverityScan, CID#1457034 ("Dereference after null check")
>
> Fixes: 2220943a21e2 ("phy: Centralise print about attached phy")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> net/dsa/slave.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 02ace7d462c4..29ab4e98639b 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -1115,10 +1115,9 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev)
> of_phy_deregister_fixed_link(port_dn);
> return ret;
> }
> + phy_attached_info(p->phy);
> }
>
> - phy_attached_info(p->phy);
> -
> return 0;
> }
Huh? Why move this into the test?
The test of the block above this change is
if (!p->phy) {
Perhaps this should be
'
if (p->phy)
phy_attached_info(p->phy);
or simpler
} else {
phy_attached_info(p->phy);
}
or maybe reverse the block
if (p->phy) {
phy_attached_info(p->phy);
} else {
ret = dsa_slave_phy_connect(slave_dev, p->dp->index);
if (ret) {
netdev_err(slave_dev, "failed to connect to port %d: %d\n",
p->dp->index, ret);
if (phy_is_fixed)
of_phy_deregister_fixed_link(port_dn);
return ret;
}
}
return 0;
}
^ permalink raw reply
* Re: [PATCH net-next v2 5/6] rtnetlink: add helpers to dump vf and netnsid information
From: David Ahern @ 2017-09-23 17:12 UTC (permalink / raw)
To: Florian Westphal, netdev
In-Reply-To: <20170922061008.14723-6-fw@strlen.de>
On 9/22/17 12:10 AM, Florian Westphal wrote:
> +static noinline_for_stack int rtnl_fill_vf(struct sk_buff *skb,
> + struct net_device *dev,
> + u32 ext_filter_mask)
> +{
> + struct nlattr *vfinfo;
> + int i, num_vfs;
> +
> + if (!dev->dev.parent || ((ext_filter_mask & RTEXT_FILTER_VF) == 0))
> + return 0;
> +
> + num_vfs = dev_num_vf(dev->dev.parent);
> + if (nla_put_u32(skb, IFLA_NUM_VF, num_vfs))
> + return -EMSGSIZE;
> +
> + if (!dev->netdev_ops->ndo_get_vf_config)
> + return 0;
> +
> + vfinfo = nla_nest_start(skb, IFLA_VFINFO_LIST);
> + if (!vfinfo)
> + return -EMSGSIZE;
> +
> + for (i = 0; i < num_vfs; i++) {
> + if (rtnl_fill_vfinfo(skb, dev, i, vfinfo))
> + return -EMSGSIZE;
> + }
> +
> + nla_nest_end(skb, vfinfo);
> + return 0;
> +}
> +
> static int rtnl_fill_link_ifmap(struct sk_buff *skb, struct net_device *dev)
> {
> struct rtnl_link_ifmap map;
> @@ -1355,6 +1385,23 @@ static noinline int nla_put_ifalias(struct sk_buff *skb, struct net_device *dev)
> return 0;
> }
>
> +static noinline int rtnl_fill_link_netnsid(struct sk_buff *skb,
> + const struct net_device *dev)
> +{
> + if (dev->rtnl_link_ops && dev->rtnl_link_ops->get_link_net) {
> + struct net *link_net = dev->rtnl_link_ops->get_link_net(dev);
> +
> + if (!net_eq(dev_net(dev), link_net)) {
> + int id = peernet2id_alloc(dev_net(dev), link_net);
> +
> + if (nla_put_s32(skb, IFLA_LINK_NETNSID, id))
> + return -EMSGSIZE;
> + }
> + }
> +
> + return 0;
> +}
> +
No reason to combine vf and netnsid into 1 patch; completely separate topics
> static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
> int type, u32 pid, u32 seq, u32 change,
> unsigned int flags, u32 ext_filter_mask,
> @@ -1428,27 +1475,9 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
> if (rtnl_fill_stats(skb, dev))
> goto nla_put_failure;
>
> - if (dev->dev.parent && (ext_filter_mask & RTEXT_FILTER_VF) &&
> - nla_put_u32(skb, IFLA_NUM_VF, dev_num_vf(dev->dev.parent)))
> + if (rtnl_fill_vf(skb, dev, ext_filter_mask))
> goto nla_put_failure;
>
> - if (dev->netdev_ops->ndo_get_vf_config && dev->dev.parent &&
> - ext_filter_mask & RTEXT_FILTER_VF) {
> - int i;
> - struct nlattr *vfinfo;
> - int num_vfs = dev_num_vf(dev->dev.parent);
> -
> - vfinfo = nla_nest_start(skb, IFLA_VFINFO_LIST);
> - if (!vfinfo)
> - goto nla_put_failure;
> - for (i = 0; i < num_vfs; i++) {
> - if (rtnl_fill_vfinfo(skb, dev, i, vfinfo))
> - goto nla_put_failure;
> - }
> -
> - nla_nest_end(skb, vfinfo);
> - }
> -
> if (rtnl_port_fill(skb, dev, ext_filter_mask))
> goto nla_put_failure;
>
> @@ -1460,17 +1489,8 @@ static int rtnl_fill_ifinfo(struct sk_buff *skb, struct net_device *dev,
> goto nla_put_failure;
> }
>
> - if (dev->rtnl_link_ops &&
> - dev->rtnl_link_ops->get_link_net) {
> - struct net *link_net = dev->rtnl_link_ops->get_link_net(dev);
> -
> - if (!net_eq(dev_net(dev), link_net)) {
> - int id = peernet2id_alloc(dev_net(dev), link_net);
> -
> - if (nla_put_s32(skb, IFLA_LINK_NETNSID, id))
> - goto nla_put_failure;
> - }
> - }
> + if (rtnl_fill_link_netnsid(skb, dev))
> + goto nla_put_failure;
>
> if (!(af_spec = nla_nest_start(skb, IFLA_AF_SPEC)))
> goto nla_put_failure;
>
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
* Re: [PATCH net-next v2 1/6] selftests: rtnetlink.sh: add rudimentary vrf test
From: David Ahern @ 2017-09-23 17:07 UTC (permalink / raw)
To: Florian Westphal, netdev
In-Reply-To: <20170922061008.14723-2-fw@strlen.de>
On 9/22/17 12:10 AM, Florian Westphal wrote:
> +kci_test_vrf()
> +{
> + vrfname="test-vrf"
> + ret=0
> +
> + ip link show type vrf 2>/dev/null
> + if [ $? -ne 0 ]; then
> + echo "SKIP: vrf: iproute2 too old"
> + return 0
> + fi
> +
> + ip link add "$vrfname" type vrf table 10
> + check_err $?
> + if [ $ret -ne 0 ];then
> + echo "FAIL: can't add vrf interface, skipping test"
> + return 0
> + fi
> +
> + ip -br link show type vrf | grep -q "$vrfname"
> + check_err $?
> + if [ $ret -ne 0 ];then
> + echo "FAIL: created vrf device not found"
> + return 1
> + fi
> +
> + ip link set dev "$vrfname" up
BTW, if there is a v3 of this set, that ip command is shifted - uses
spaces instead of tab.
^ permalink raw reply
* Re: [PATCH net-next v2 3/6] rtnetlink: add helper to dump qdisc name
From: David Ahern @ 2017-09-23 17:03 UTC (permalink / raw)
To: Florian Westphal, netdev
In-Reply-To: <20170922061008.14723-4-fw@strlen.de>
On 9/22/17 12:10 AM, Florian Westphal wrote:
> We can use rcu here to make this safe even if we would not hold rtnl:
> qdisc_destroy uses call_rcu to free the Qdisc struct.
>
> Signed-off-by: Florian Westphal <fw@strlen.de>
> ---
> net/core/rtnetlink.c | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
Reviewed-by: David Ahern <dsahern@gmail.com>
^ permalink raw reply
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