* [PATCH net-next v2 4/5] bridge: vlan: learn to count
From: Nikolay Aleksandrov @ 2016-04-28 15:52 UTC (permalink / raw)
To: netdev; +Cc: roopa, davem, stephen, jhs, Nikolay Aleksandrov
In-Reply-To: <1461858771-4732-1-git-send-email-nikolay@cumulusnetworks.com>
Add support for per-VLAN Tx/Rx statistics. Every global vlan context gets
allocated a per-cpu stats which is then set in each per-port vlan context
for quick access. The br_allowed_ingress() common function is used to
account for Rx packets and the br_handle_vlan() common function is used
to account for Tx packets.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v2: no change
net/bridge/br_private.h | 11 +++++++++-
net/bridge/br_vlan.c | 53 +++++++++++++++++++++++++++++++++++++++----------
2 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 50d70b5eb307..f6876ed718a5 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -77,12 +77,21 @@ struct bridge_mcast_querier {
};
#endif
+struct br_vlan_stats {
+ u64 rx_bytes;
+ u64 rx_packets;
+ u64 tx_bytes;
+ u64 tx_packets;
+ struct u64_stats_sync syncp;
+};
+
/**
* struct net_bridge_vlan - per-vlan entry
*
* @vnode: rhashtable member
* @vid: VLAN id
* @flags: bridge vlan flags
+ * @stats: per-cpu VLAN statistics
* @br: if MASTER flag set, this points to a bridge struct
* @port: if MASTER flag unset, this points to a port struct
* @refcnt: if MASTER flag set, this is bumped for each port referencing it
@@ -100,6 +109,7 @@ struct net_bridge_vlan {
struct rhash_head vnode;
u16 vid;
u16 flags;
+ struct br_vlan_stats __percpu *stats;
union {
struct net_bridge *br;
struct net_bridge_port *port;
@@ -866,7 +876,6 @@ static inline struct net_bridge_vlan_group *nbp_vlan_group_rcu(
{
return NULL;
}
-
#endif
struct nf_br_ops {
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index 4fab7665df8c..d7a70c2ea3ec 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -151,6 +151,17 @@ static struct net_bridge_vlan *br_vlan_get_master(struct net_bridge *br, u16 vid
return masterv;
}
+static void br_master_vlan_rcu_free(struct rcu_head *rcu)
+{
+ struct net_bridge_vlan *v;
+
+ v = container_of(rcu, struct net_bridge_vlan, rcu);
+ WARN_ON(!br_vlan_is_master(v));
+ free_percpu(v->stats);
+ v->stats = NULL;
+ kfree(v);
+}
+
static void br_vlan_put_master(struct net_bridge_vlan *masterv)
{
struct net_bridge_vlan_group *vg;
@@ -163,7 +174,7 @@ static void br_vlan_put_master(struct net_bridge_vlan *masterv)
rhashtable_remove_fast(&vg->vlan_hash,
&masterv->vnode, br_vlan_rht_params);
__vlan_del_list(masterv);
- kfree_rcu(masterv, rcu);
+ call_rcu(&masterv->rcu, br_master_vlan_rcu_free);
}
}
@@ -219,6 +230,7 @@ static int __vlan_add(struct net_bridge_vlan *v, u16 flags)
if (!masterv)
goto out_filt;
v->brvlan = masterv;
+ v->stats = masterv->stats;
}
/* Add the dev mac and count the vlan only if it's usable */
@@ -320,6 +332,7 @@ struct sk_buff *br_handle_vlan(struct net_bridge *br,
struct net_bridge_vlan_group *vg,
struct sk_buff *skb)
{
+ struct br_vlan_stats *stats;
struct net_bridge_vlan *v;
u16 vid;
@@ -346,9 +359,14 @@ struct sk_buff *br_handle_vlan(struct net_bridge *br,
return NULL;
}
}
+ stats = this_cpu_ptr(v->stats);
+ u64_stats_update_begin(&stats->syncp);
+ stats->tx_bytes += skb->len;
+ stats->tx_packets++;
+ u64_stats_update_end(&stats->syncp);
+
if (v->flags & BRIDGE_VLAN_INFO_UNTAGGED)
skb->vlan_tci = 0;
-
out:
return skb;
}
@@ -357,7 +375,8 @@ out:
static bool __allowed_ingress(struct net_bridge_vlan_group *vg, __be16 proto,
struct sk_buff *skb, u16 *vid)
{
- const struct net_bridge_vlan *v;
+ struct br_vlan_stats *stats;
+ struct net_bridge_vlan *v;
bool tagged;
BR_INPUT_SKB_CB(skb)->vlan_filtered = true;
@@ -418,14 +437,21 @@ static bool __allowed_ingress(struct net_bridge_vlan_group *vg, __be16 proto,
* We update only VID field and preserve PCP field.
*/
skb->vlan_tci |= v->vid;
-
- return true;
+ } else {
+ /* Frame had a valid vlan tag. See if vlan is allowed */
+ v = br_vlan_find(vg, *vid);
}
+ if (!v || !br_vlan_should_use(v))
+ goto drop;
+
+ stats = this_cpu_ptr(v->stats);
+ u64_stats_update_begin(&stats->syncp);
+ stats->rx_bytes += skb->len;
+ stats->rx_packets++;
+ u64_stats_update_end(&stats->syncp);
+
+ return true;
- /* Frame had a valid vlan tag. See if vlan is allowed */
- v = br_vlan_find(vg, *vid);
- if (v && br_vlan_should_use(v))
- return true;
drop:
kfree_skb(skb);
return false;
@@ -538,6 +564,11 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags)
if (!vlan)
return -ENOMEM;
+ vlan->stats = netdev_alloc_pcpu_stats(struct br_vlan_stats);
+ if (!vlan->stats) {
+ kfree(vlan);
+ return -ENOMEM;
+ }
vlan->vid = vid;
vlan->flags = flags | BRIDGE_VLAN_INFO_MASTER;
vlan->flags &= ~BRIDGE_VLAN_INFO_PVID;
@@ -545,8 +576,10 @@ int br_vlan_add(struct net_bridge *br, u16 vid, u16 flags)
if (flags & BRIDGE_VLAN_INFO_BRENTRY)
atomic_set(&vlan->refcnt, 1);
ret = __vlan_add(vlan, flags);
- if (ret)
+ if (ret) {
+ free_percpu(vlan->stats);
kfree(vlan);
+ }
return ret;
}
--
2.4.11
^ permalink raw reply related
* [PATCH net-next v2 5/5] bridge: netlink: export per-vlan stats
From: Nikolay Aleksandrov @ 2016-04-28 15:52 UTC (permalink / raw)
To: netdev; +Cc: roopa, davem, stephen, jhs, Nikolay Aleksandrov
In-Reply-To: <1461858771-4732-1-git-send-email-nikolay@cumulusnetworks.com>
Add a new LINK_XSTATS_TYPE_BRIDGE attribute and implement the
RTM_GETSTATS callbacks for IFLA_STATS_LINK_XSTATS (fill_linkxstats and
get_linkxstats_size) in order to export the per-vlan stats.
The paddings were added because soon these fields will be needed for
per-port per-vlan stats (or something else if someone beats me to it) so
avoiding at least a few more netlink attributes.
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v2: remove unused pvid pointer, fix the case where bridge has 0 vlans
but there're global contexts and move to rtnl link type private
attributes nested into a LINK_XSTATS_TYPE_ attribute. The paddings were
added because soon these fields will be needed for per-port per-vlan
stats (or something else if someone beats me to it) so avoiding at least
a few more netlink attributes.
include/uapi/linux/if_bridge.h | 18 ++++++++++++
include/uapi/linux/if_link.h | 1 +
net/bridge/br_netlink.c | 65 ++++++++++++++++++++++++++++++++++++++++++
net/bridge/br_private.h | 7 +++++
net/bridge/br_vlan.c | 27 ++++++++++++++++++
5 files changed, 118 insertions(+)
diff --git a/include/uapi/linux/if_bridge.h b/include/uapi/linux/if_bridge.h
index 0536eefff9bf..397d503fdedb 100644
--- a/include/uapi/linux/if_bridge.h
+++ b/include/uapi/linux/if_bridge.h
@@ -134,6 +134,16 @@ struct bridge_vlan_info {
__u16 vid;
};
+struct bridge_vlan_xstats {
+ __u64 rx_bytes;
+ __u64 rx_packets;
+ __u64 tx_bytes;
+ __u64 tx_packets;
+ __u16 vid;
+ __u16 pad1;
+ __u32 pad2;
+};
+
/* Bridge multicast database attributes
* [MDBA_MDB] = {
* [MDBA_MDB_ENTRY] = {
@@ -233,4 +243,12 @@ enum {
};
#define MDBA_SET_ENTRY_MAX (__MDBA_SET_ENTRY_MAX - 1)
+/* Embedded inside LINK_XSTATS_TYPE_BRIDGE */
+enum {
+ BRIDGE_XSTATS_UNSPEC,
+ BRIDGE_XSTATS_VLAN,
+ __BRIDGE_XSTATS_MAX
+};
+#define BRIDGE_XSTATS_MAX (__BRIDGE_XSTATS_MAX - 1)
+
#endif /* _UAPI_LINUX_IF_BRIDGE_H */
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 8577c0e4116f..fd0621ec9222 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -817,6 +817,7 @@ enum {
*/
enum {
LINK_XSTATS_TYPE_UNSPEC,
+ LINK_XSTATS_TYPE_BRIDGE,
__LINK_XSTATS_TYPE_MAX
};
#define LINK_XSTATS_TYPE_MAX (__LINK_XSTATS_TYPE_MAX - 1)
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 9c74fa438a8c..c36bafcad569 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -1222,6 +1222,69 @@ static int br_fill_info(struct sk_buff *skb, const struct net_device *brdev)
return 0;
}
+static size_t br_get_linkxstats_size(const struct net_device *dev)
+{
+ struct net_bridge *br = netdev_priv(dev);
+ struct net_bridge_vlan_group *vg;
+ struct net_bridge_vlan *v;
+ int numvls = 0;
+
+ vg = br_vlan_group(br);
+ if (!vg)
+ return 0;
+
+ /* we need to count all, even placeholder entries */
+ list_for_each_entry(v, &vg->vlan_list, vlist)
+ numvls++;
+
+ /* account for the vlans and the link xstats type nest attribute */
+ return numvls * nla_total_size(sizeof(struct bridge_vlan_xstats)) +
+ nla_total_size(0);
+}
+
+static int br_fill_linkxstats(struct sk_buff *skb, const struct net_device *dev,
+ int *prividx)
+{
+ struct net_bridge *br = netdev_priv(dev);
+ struct net_bridge_vlan_group *vg;
+ struct net_bridge_vlan *v;
+ struct nlattr *nest;
+ int vl_idx = 0;
+
+ vg = br_vlan_group(br);
+ if (!vg)
+ goto out;
+ nest = nla_nest_start(skb, LINK_XSTATS_TYPE_BRIDGE);
+ if (!nest)
+ return -EMSGSIZE;
+ list_for_each_entry(v, &vg->vlan_list, vlist) {
+ struct bridge_vlan_xstats vxi;
+ struct br_vlan_stats stats;
+
+ if (vl_idx++ < *prividx)
+ continue;
+ memset(&vxi, 0, sizeof(vxi));
+ vxi.vid = v->vid;
+ br_vlan_get_stats(v, &stats);
+ vxi.rx_bytes = stats.rx_bytes;
+ vxi.rx_packets = stats.rx_packets;
+ vxi.tx_bytes = stats.tx_bytes;
+ vxi.tx_packets = stats.tx_packets;
+
+ if (nla_put(skb, BRIDGE_XSTATS_VLAN, sizeof(vxi), &vxi))
+ goto nla_put_failure;
+ }
+ nla_nest_end(skb, nest);
+ *prividx = 0;
+out:
+ return 0;
+
+nla_put_failure:
+ nla_nest_end(skb, nest);
+ *prividx = vl_idx;
+
+ return -EMSGSIZE;
+}
static struct rtnl_af_ops br_af_ops __read_mostly = {
.family = AF_BRIDGE,
@@ -1240,6 +1303,8 @@ struct rtnl_link_ops br_link_ops __read_mostly = {
.dellink = br_dev_delete,
.get_size = br_get_size,
.fill_info = br_fill_info,
+ .fill_linkxstats = br_fill_linkxstats,
+ .get_linkxstats_size = br_get_linkxstats_size,
.slave_maxtype = IFLA_BRPORT_MAX,
.slave_policy = br_port_policy,
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index f6876ed718a5..a10f7ed26f3b 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -709,6 +709,8 @@ int nbp_vlan_delete(struct net_bridge_port *port, u16 vid);
void nbp_vlan_flush(struct net_bridge_port *port);
int nbp_vlan_init(struct net_bridge_port *port);
int nbp_get_num_vlan_infos(struct net_bridge_port *p, u32 filter_mask);
+void br_vlan_get_stats(const struct net_bridge_vlan *v,
+ struct br_vlan_stats *stats);
static inline struct net_bridge_vlan_group *br_vlan_group(
const struct net_bridge *br)
@@ -876,6 +878,11 @@ static inline struct net_bridge_vlan_group *nbp_vlan_group_rcu(
{
return NULL;
}
+
+static inline void br_vlan_get_stats(const struct net_bridge_vlan *v,
+ struct br_vlan_stats *stats)
+{
+}
#endif
struct nf_br_ops {
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index d7a70c2ea3ec..b39d9f5761d9 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -1029,3 +1029,30 @@ void nbp_vlan_flush(struct net_bridge_port *port)
synchronize_rcu();
__vlan_group_free(vg);
}
+
+void br_vlan_get_stats(const struct net_bridge_vlan *v,
+ struct br_vlan_stats *stats)
+{
+ int i;
+
+ memset(stats, 0, sizeof(*stats));
+ for_each_possible_cpu(i) {
+ u64 rxpackets, rxbytes, txpackets, txbytes;
+ struct br_vlan_stats *cpu_stats;
+ unsigned int start;
+
+ cpu_stats = per_cpu_ptr(v->stats, i);
+ do {
+ start = u64_stats_fetch_begin_irq(&cpu_stats->syncp);
+ rxpackets = cpu_stats->rx_packets;
+ rxbytes = cpu_stats->rx_bytes;
+ txbytes = cpu_stats->tx_bytes;
+ txpackets = cpu_stats->tx_packets;
+ } while (u64_stats_fetch_retry_irq(&cpu_stats->syncp, start));
+
+ stats->rx_packets += rxpackets;
+ stats->rx_bytes += rxbytes;
+ stats->tx_bytes += txbytes;
+ stats->tx_packets += txpackets;
+ }
+}
--
2.4.11
^ permalink raw reply related
* Re: [PATCH net-next] drivers/net: add 6WIND SHULTI support
From: David Miller @ 2016-04-28 15:54 UTC (permalink / raw)
To: nicolas.dichtel; +Cc: jiri, fw, netdev
In-Reply-To: <5721FB26.6070305@6wind.com>
From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Date: Thu, 28 Apr 2016 13:59:34 +0200
> Le 27/04/2016 18:55, David Miller a écrit :
>> From: Jiri Pirko <jiri@resnulli.us>
> [snip]
>>> The difference is that it this tries to allow userspace crap to mirror
>>> setting user does for bridge/ovs. Basically this looks to me like an
>>> attempt to enable userspace SDKs and such.
>>
>> +1
> I don't think so because a userspace can receive all the bridge/ovs settings by
> mean of netlink mirror, without the need for this driver at all.
You can say whatever you want, but the facilities you are adding to
this driver enables proprietary userland SDK components.
And this is precisely what we are trying to avoid by having a clean,
fully featured switch device model in the kernel.
It is against your interestes of upstreaming your driver to continue
denying what your changes facilitate.
THanks.
^ permalink raw reply
* [PATCH v4 net-next 0/2] ppp: add rtnetlink support
From: Guillaume Nault @ 2016-04-28 15:55 UTC (permalink / raw)
To: netdev
Cc: linux-ppp, Paul Mackerras, David Miller, Stephen Hemminger,
walter harms
PPP devices lack the ability to be customised at creation time. In
particular they can't be created in a given netns or with a particular
name. Moving or renaming the device after creation is possible, but
creates undesirable transient effects on servers where PPP devices are
constantly created and removed, as users connect and disconnect.
Implementing rtnetlink support solves this problem.
The rtnetlink handlers implemented in this series are minimal, and can
only replace the PPPIOCNEWUNIT ioctl. The rest of PPP ioctls remains
necessary for any other operation on channels and units.
It is perfectly possible to mix PPP devices created by rtnl
and by ioctl(PPPIOCNEWUNIT). Devices will behave in the same way.
mutex_trylock() is used to resolve the locking issue wrt. locking
dependency between rtnl_lock() and ppp_mutex (see ppp_nl_newlink() in
patch #2).
A user visible difference brought by this series is that old PPP
interfaces (those created with ioctl(PPPIOCNEWUNIT)), can now be
removed by "ip link del", just like new rtnl based PPP devices.
Changes since v3:
- Rebase on net-next.
- Not an RFC anymore.
Changes since v2:
- Define ->rtnl_link_ops for ioctl based PPP devices, so they can
handle rtnl messages just like rtnl based ones (suggested by
Stephen Hemminger).
- Move back to original lock ordering between ppp_mutex and rtnl_lock
to simplify patch series. Handle lock inversion issue using
mutex_trylock() (suggested by Stephen Hemminger).
- Do file descriptor lookup directly in ppp_nl_newlink(), to simplify
ppp_dev_configure().
Changes since v1:
- Rebase on net-next.
- Invert locking order wrt. ppp_mutex and rtnl_lock and protect
file->private_data with ppp_mutex.
Guillaume Nault (2):
ppp: define reusable device creation functions
ppp: add rtnetlink device creation support
drivers/net/ppp/ppp_generic.c | 315 ++++++++++++++++++++++++++++++------------
include/uapi/linux/if_link.h | 8 ++
2 files changed, 235 insertions(+), 88 deletions(-)
--
2.8.1
^ permalink raw reply
* [PATCH v4 net-next 1/2] ppp: define reusable device creation functions
From: Guillaume Nault @ 2016-04-28 15:55 UTC (permalink / raw)
To: netdev
Cc: linux-ppp, Paul Mackerras, David Miller, Stephen Hemminger,
walter harms
In-Reply-To: <cover.1461854990.git.g.nault@alphalink.fr>
Move PPP device initialisation and registration out of
ppp_create_interface().
This prepares code for device registration with rtnetlink.
While there, simplify the prototype of ppp_create_interface():
* Since ppp_dev_configure() takes care of setting file->private_data,
there's no need to return a ppp structure to ppp_unattached_ioctl()
anymore.
* The unit parameter is made read/write so that ppp_create_interface()
can tell which unit number has been assigned.
Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
drivers/net/ppp/ppp_generic.c | 206 ++++++++++++++++++++++++------------------
1 file changed, 118 insertions(+), 88 deletions(-)
diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index f572b31..59077c8 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -183,6 +183,11 @@ struct channel {
#endif /* CONFIG_PPP_MULTILINK */
};
+struct ppp_config {
+ struct file *file;
+ s32 unit;
+};
+
/*
* SMP locking issues:
* Both the ppp.rlock and ppp.wlock locks protect the ppp.channels
@@ -269,8 +274,7 @@ static void ppp_ccp_peek(struct ppp *ppp, struct sk_buff *skb, int inbound);
static void ppp_ccp_closed(struct ppp *ppp);
static struct compressor *find_compressor(int type);
static void ppp_get_stats(struct ppp *ppp, struct ppp_stats *st);
-static struct ppp *ppp_create_interface(struct net *net, int unit,
- struct file *file, int *retp);
+static int ppp_create_interface(struct net *net, struct file *file, int *unit);
static void init_ppp_file(struct ppp_file *pf, int kind);
static void ppp_destroy_interface(struct ppp *ppp);
static struct ppp *ppp_find_unit(struct ppp_net *pn, int unit);
@@ -853,12 +857,12 @@ static int ppp_unattached_ioctl(struct net *net, struct ppp_file *pf,
/* Create a new ppp unit */
if (get_user(unit, p))
break;
- ppp = ppp_create_interface(net, unit, file, &err);
- if (!ppp)
+ err = ppp_create_interface(net, file, &unit);
+ if (err < 0)
break;
- file->private_data = &ppp->file;
+
err = -EFAULT;
- if (put_user(ppp->file.index, p))
+ if (put_user(unit, p))
break;
err = 0;
break;
@@ -960,6 +964,94 @@ static struct pernet_operations ppp_net_ops = {
.size = sizeof(struct ppp_net),
};
+static int ppp_unit_register(struct ppp *ppp, int unit)
+{
+ struct ppp_net *pn = ppp_pernet(ppp->ppp_net);
+ int ret;
+
+ mutex_lock(&pn->all_ppp_mutex);
+
+ if (unit < 0) {
+ ret = unit_get(&pn->units_idr, ppp);
+ if (ret < 0)
+ goto err;
+ } else {
+ /* Caller asked for a specific unit number. Fail with -EEXIST
+ * if unavailable. For backward compatibility, return -EEXIST
+ * too if idr allocation fails; this makes pppd retry without
+ * requesting a specific unit number.
+ */
+ if (unit_find(&pn->units_idr, unit)) {
+ ret = -EEXIST;
+ goto err;
+ }
+ ret = unit_set(&pn->units_idr, ppp, unit);
+ if (ret < 0) {
+ /* Rewrite error for backward compatibility */
+ ret = -EEXIST;
+ goto err;
+ }
+ }
+ ppp->file.index = ret;
+
+ snprintf(ppp->dev->name, IFNAMSIZ, "ppp%i", ppp->file.index);
+
+ ret = register_netdevice(ppp->dev);
+ if (ret < 0)
+ goto err_unit;
+
+ atomic_inc(&ppp_unit_count);
+
+ mutex_unlock(&pn->all_ppp_mutex);
+
+ return 0;
+
+err_unit:
+ unit_put(&pn->units_idr, ppp->file.index);
+err:
+ mutex_unlock(&pn->all_ppp_mutex);
+
+ return ret;
+}
+
+static int ppp_dev_configure(struct net *src_net, struct net_device *dev,
+ const struct ppp_config *conf)
+{
+ struct ppp *ppp = netdev_priv(dev);
+ int indx;
+ int err;
+
+ ppp->dev = dev;
+ ppp->ppp_net = src_net;
+ ppp->mru = PPP_MRU;
+ ppp->owner = conf->file;
+
+ init_ppp_file(&ppp->file, INTERFACE);
+ ppp->file.hdrlen = PPP_HDRLEN - 2; /* don't count proto bytes */
+
+ for (indx = 0; indx < NUM_NP; ++indx)
+ ppp->npmode[indx] = NPMODE_PASS;
+ INIT_LIST_HEAD(&ppp->channels);
+ spin_lock_init(&ppp->rlock);
+ spin_lock_init(&ppp->wlock);
+#ifdef CONFIG_PPP_MULTILINK
+ ppp->minseq = -1;
+ skb_queue_head_init(&ppp->mrq);
+#endif /* CONFIG_PPP_MULTILINK */
+#ifdef CONFIG_PPP_FILTER
+ ppp->pass_filter = NULL;
+ ppp->active_filter = NULL;
+#endif /* CONFIG_PPP_FILTER */
+
+ err = ppp_unit_register(ppp, conf->unit);
+ if (err < 0)
+ return err;
+
+ conf->file->private_data = &ppp->file;
+
+ return 0;
+}
+
#define PPP_MAJOR 108
/* Called at boot time if ppp is compiled into the kernel,
@@ -2732,102 +2824,40 @@ ppp_get_stats(struct ppp *ppp, struct ppp_stats *st)
* or if there is already a unit with the requested number.
* unit == -1 means allocate a new number.
*/
-static struct ppp *ppp_create_interface(struct net *net, int unit,
- struct file *file, int *retp)
+static int ppp_create_interface(struct net *net, struct file *file, int *unit)
{
+ struct ppp_config conf = {
+ .file = file,
+ .unit = *unit,
+ };
+ struct net_device *dev;
struct ppp *ppp;
- struct ppp_net *pn;
- struct net_device *dev = NULL;
- int ret = -ENOMEM;
- int i;
+ int err;
dev = alloc_netdev(sizeof(struct ppp), "", NET_NAME_ENUM, ppp_setup);
- if (!dev)
- goto out1;
-
- pn = ppp_pernet(net);
-
- ppp = netdev_priv(dev);
- ppp->dev = dev;
- ppp->mru = PPP_MRU;
- init_ppp_file(&ppp->file, INTERFACE);
- ppp->file.hdrlen = PPP_HDRLEN - 2; /* don't count proto bytes */
- ppp->owner = file;
- for (i = 0; i < NUM_NP; ++i)
- ppp->npmode[i] = NPMODE_PASS;
- INIT_LIST_HEAD(&ppp->channels);
- spin_lock_init(&ppp->rlock);
- spin_lock_init(&ppp->wlock);
-#ifdef CONFIG_PPP_MULTILINK
- ppp->minseq = -1;
- skb_queue_head_init(&ppp->mrq);
-#endif /* CONFIG_PPP_MULTILINK */
-#ifdef CONFIG_PPP_FILTER
- ppp->pass_filter = NULL;
- ppp->active_filter = NULL;
-#endif /* CONFIG_PPP_FILTER */
-
- /*
- * drum roll: don't forget to set
- * the net device is belong to
- */
+ if (!dev) {
+ err = -ENOMEM;
+ goto err;
+ }
dev_net_set(dev, net);
rtnl_lock();
- mutex_lock(&pn->all_ppp_mutex);
-
- if (unit < 0) {
- unit = unit_get(&pn->units_idr, ppp);
- if (unit < 0) {
- ret = unit;
- goto out2;
- }
- } else {
- ret = -EEXIST;
- if (unit_find(&pn->units_idr, unit))
- goto out2; /* unit already exists */
- /*
- * if caller need a specified unit number
- * lets try to satisfy him, otherwise --
- * he should better ask us for new unit number
- *
- * NOTE: yes I know that returning EEXIST it's not
- * fair but at least pppd will ask us to allocate
- * new unit in this case so user is happy :)
- */
- unit = unit_set(&pn->units_idr, ppp, unit);
- if (unit < 0)
- goto out2;
- }
-
- /* Initialize the new ppp unit */
- ppp->file.index = unit;
- sprintf(dev->name, "ppp%d", unit);
- ret = register_netdevice(dev);
- if (ret != 0) {
- unit_put(&pn->units_idr, unit);
- netdev_err(ppp->dev, "PPP: couldn't register device %s (%d)\n",
- dev->name, ret);
- goto out2;
- }
-
- ppp->ppp_net = net;
+ err = ppp_dev_configure(net, dev, &conf);
+ if (err < 0)
+ goto err_dev;
+ ppp = netdev_priv(dev);
+ *unit = ppp->file.index;
- atomic_inc(&ppp_unit_count);
- mutex_unlock(&pn->all_ppp_mutex);
rtnl_unlock();
- *retp = 0;
- return ppp;
+ return 0;
-out2:
- mutex_unlock(&pn->all_ppp_mutex);
+err_dev:
rtnl_unlock();
free_netdev(dev);
-out1:
- *retp = ret;
- return NULL;
+err:
+ return err;
}
/*
--
2.8.1
^ permalink raw reply related
* [PATCH v4 net-next 2/2] ppp: add rtnetlink device creation support
From: Guillaume Nault @ 2016-04-28 15:55 UTC (permalink / raw)
To: netdev
Cc: linux-ppp, Paul Mackerras, David Miller, Stephen Hemminger,
walter harms
In-Reply-To: <cover.1461854990.git.g.nault@alphalink.fr>
Define PPP device handler for use with rtnetlink.
The only PPP specific attribute is IFLA_PPP_DEV_FD. It is mandatory and
contains the file descriptor of the associated /dev/ppp instance (the
file descriptor which would have been used for ioctl(PPPIOCNEWUNIT) in
the ioctl-based API). The PPP device is removed when this file
descriptor is released (same behaviour as with ioctl based PPP
devices).
PPP devices created with the rtnetlink API behave like the ones created
with ioctl(PPPIOCNEWUNIT). In particular existing ioctls work the same
way, no matter how the PPP device was created.
The rtnl callbacks are also assigned to ioctl based PPP devices. This
way, rtnl messages have the same effect on any PPP devices.
The immediate effect is that all PPP devices, even ioctl-based
ones, can now be removed with "ip link del".
A minor difference still exists between ioctl and rtnl based PPP
interfaces: in the device name, the number following the "ppp" prefix
corresponds to the PPP unit number for ioctl based devices, while it is
just an unrelated incrementing index for rtnl ones.
Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
---
drivers/net/ppp/ppp_generic.c | 115 ++++++++++++++++++++++++++++++++++++++++--
include/uapi/linux/if_link.h | 8 +++
2 files changed, 120 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index 59077c8..8dedafa 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -46,6 +46,7 @@
#include <linux/device.h>
#include <linux/mutex.h>
#include <linux/slab.h>
+#include <linux/file.h>
#include <asm/unaligned.h>
#include <net/slhc_vj.h>
#include <linux/atomic.h>
@@ -186,6 +187,7 @@ struct channel {
struct ppp_config {
struct file *file;
s32 unit;
+ bool ifname_is_set;
};
/*
@@ -286,6 +288,7 @@ static int unit_get(struct idr *p, void *ptr);
static int unit_set(struct idr *p, void *ptr, int n);
static void unit_put(struct idr *p, int n);
static void *unit_find(struct idr *p, int n);
+static void ppp_setup(struct net_device *dev);
static const struct net_device_ops ppp_netdev_ops;
@@ -964,7 +967,7 @@ static struct pernet_operations ppp_net_ops = {
.size = sizeof(struct ppp_net),
};
-static int ppp_unit_register(struct ppp *ppp, int unit)
+static int ppp_unit_register(struct ppp *ppp, int unit, bool ifname_is_set)
{
struct ppp_net *pn = ppp_pernet(ppp->ppp_net);
int ret;
@@ -994,7 +997,8 @@ static int ppp_unit_register(struct ppp *ppp, int unit)
}
ppp->file.index = ret;
- snprintf(ppp->dev->name, IFNAMSIZ, "ppp%i", ppp->file.index);
+ if (!ifname_is_set)
+ snprintf(ppp->dev->name, IFNAMSIZ, "ppp%i", ppp->file.index);
ret = register_netdevice(ppp->dev);
if (ret < 0)
@@ -1043,7 +1047,7 @@ static int ppp_dev_configure(struct net *src_net, struct net_device *dev,
ppp->active_filter = NULL;
#endif /* CONFIG_PPP_FILTER */
- err = ppp_unit_register(ppp, conf->unit);
+ err = ppp_unit_register(ppp, conf->unit, conf->ifname_is_set);
if (err < 0)
return err;
@@ -1052,6 +1056,99 @@ static int ppp_dev_configure(struct net *src_net, struct net_device *dev,
return 0;
}
+static const struct nla_policy ppp_nl_policy[IFLA_PPP_MAX + 1] = {
+ [IFLA_PPP_DEV_FD] = { .type = NLA_S32 },
+};
+
+static int ppp_nl_validate(struct nlattr *tb[], struct nlattr *data[])
+{
+ if (!data)
+ return -EINVAL;
+
+ if (!data[IFLA_PPP_DEV_FD])
+ return -EINVAL;
+ if (nla_get_s32(data[IFLA_PPP_DEV_FD]) < 0)
+ return -EBADF;
+
+ return 0;
+}
+
+static int ppp_nl_newlink(struct net *src_net, struct net_device *dev,
+ struct nlattr *tb[], struct nlattr *data[])
+{
+ struct ppp_config conf = {
+ .unit = -1,
+ .ifname_is_set = true,
+ };
+ struct file *file;
+ int err;
+
+ file = fget(nla_get_s32(data[IFLA_PPP_DEV_FD]));
+ if (!file)
+ return -EBADF;
+
+ /* rtnl_lock is already held here, but ppp_create_interface() locks
+ * ppp_mutex before holding rtnl_lock. Using mutex_trylock() avoids
+ * possible deadlock due to lock order inversion, at the cost of
+ * pushing the problem back to userspace.
+ */
+ if (!mutex_trylock(&ppp_mutex)) {
+ err = -EBUSY;
+ goto out;
+ }
+
+ if (file->f_op != &ppp_device_fops || file->private_data) {
+ err = -EBADF;
+ goto out_unlock;
+ }
+
+ conf.file = file;
+ err = ppp_dev_configure(src_net, dev, &conf);
+
+out_unlock:
+ mutex_unlock(&ppp_mutex);
+out:
+ fput(file);
+
+ return err;
+}
+
+static void ppp_nl_dellink(struct net_device *dev, struct list_head *head)
+{
+ unregister_netdevice_queue(dev, head);
+}
+
+static size_t ppp_nl_get_size(const struct net_device *dev)
+{
+ return 0;
+}
+
+static int ppp_nl_fill_info(struct sk_buff *skb, const struct net_device *dev)
+{
+ return 0;
+}
+
+static struct net *ppp_nl_get_link_net(const struct net_device *dev)
+{
+ struct ppp *ppp = netdev_priv(dev);
+
+ return ppp->ppp_net;
+}
+
+static struct rtnl_link_ops ppp_link_ops __read_mostly = {
+ .kind = "ppp",
+ .maxtype = IFLA_PPP_MAX,
+ .policy = ppp_nl_policy,
+ .priv_size = sizeof(struct ppp),
+ .setup = ppp_setup,
+ .validate = ppp_nl_validate,
+ .newlink = ppp_nl_newlink,
+ .dellink = ppp_nl_dellink,
+ .get_size = ppp_nl_get_size,
+ .fill_info = ppp_nl_fill_info,
+ .get_link_net = ppp_nl_get_link_net,
+};
+
#define PPP_MAJOR 108
/* Called at boot time if ppp is compiled into the kernel,
@@ -1080,11 +1177,19 @@ static int __init ppp_init(void)
goto out_chrdev;
}
+ err = rtnl_link_register(&ppp_link_ops);
+ if (err) {
+ pr_err("failed to register rtnetlink PPP handler\n");
+ goto out_class;
+ }
+
/* not a big deal if we fail here :-) */
device_create(ppp_class, NULL, MKDEV(PPP_MAJOR, 0), NULL, "ppp");
return 0;
+out_class:
+ class_destroy(ppp_class);
out_chrdev:
unregister_chrdev(PPP_MAJOR, "ppp");
out_net:
@@ -2829,6 +2934,7 @@ static int ppp_create_interface(struct net *net, struct file *file, int *unit)
struct ppp_config conf = {
.file = file,
.unit = *unit,
+ .ifname_is_set = false,
};
struct net_device *dev;
struct ppp *ppp;
@@ -2840,6 +2946,7 @@ static int ppp_create_interface(struct net *net, struct file *file, int *unit)
goto err;
}
dev_net_set(dev, net);
+ dev->rtnl_link_ops = &ppp_link_ops;
rtnl_lock();
@@ -3046,6 +3153,7 @@ static void __exit ppp_cleanup(void)
/* should never happen */
if (atomic_read(&ppp_unit_count) || atomic_read(&channel_count))
pr_err("PPP: removing module but units remain!\n");
+ rtnl_link_unregister(&ppp_link_ops);
unregister_chrdev(PPP_MAJOR, "ppp");
device_destroy(ppp_class, MKDEV(PPP_MAJOR, 0));
class_destroy(ppp_class);
@@ -3104,4 +3212,5 @@ EXPORT_SYMBOL(ppp_register_compressor);
EXPORT_SYMBOL(ppp_unregister_compressor);
MODULE_LICENSE("GPL");
MODULE_ALIAS_CHARDEV(PPP_MAJOR, 0);
+MODULE_ALIAS_RTNL_LINK("ppp");
MODULE_ALIAS("devname:ppp");
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index d82de33..3e80974 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -520,6 +520,14 @@ enum {
};
#define IFLA_GENEVE_MAX (__IFLA_GENEVE_MAX - 1)
+/* PPP section */
+enum {
+ IFLA_PPP_UNSPEC,
+ IFLA_PPP_DEV_FD,
+ __IFLA_PPP_MAX
+};
+#define IFLA_PPP_MAX (__IFLA_PPP_MAX - 1)
+
/* Bonding section */
enum {
--
2.8.1
^ permalink raw reply related
* Re: [PATCH net-next] net: snmp: fix 64bit stats on 32bit arches
From: David Miller @ 2016-04-28 15:55 UTC (permalink / raw)
To: eric.dumazet; +Cc: nicolas.dichtel, edumazet, netdev
In-Reply-To: <1461850404.5535.103.camel@edumazet-glaptop3.roam.corp.google.com>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 28 Apr 2016 06:33:24 -0700
> From: Eric Dumazet <edumazet@google.com>
>
> I accidentally replaced BH disabling by preemption disabling
> in SNMP_ADD_STATS64() and SNMP_UPD_PO_STATS64() on 32bit builds.
>
> For 64bit stats on 32bit arch, we really need to disable BH,
> since the "struct u64_stats_sync syncp" might be manipulated
> both from process and BH contexts.
>
> Fixes: 6aef70a851ac ("net: snmp: kill various STATS_USER() helpers")
> Reported-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Tested-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied.
^ permalink raw reply
* Re: [PATCH v2] net: macb: do not scan PHYs manually
From: Nathan Sullivan @ 2016-04-28 15:55 UTC (permalink / raw)
To: Nicolas Ferre; +Cc: netdev, linux-kernel, Florian Fainelli, Alexandre Belloni
In-Reply-To: <57222FCE.8050407@atmel.com>
On Thu, Apr 28, 2016 at 05:44:14PM +0200, Nicolas Ferre wrote:
> Le 28/04/2016 16:46, Nathan Sullivan a écrit :
> > Since of_mdiobus_register and mdiobus_register will scan automatically,
> > do not manually scan for PHY devices in the macb ethernet driver. Doing
> > so will result in many nonexistent PHYs on the MDIO bus if the MDIO
> > lines are floating or grounded, such as when they are not used.
> >
> > Signed-off-by: Nathan Sullivan <nathan.sullivan@ni.com>
>
> Well, as explained in the commit message that added this feature and in
> the comment, if no phy is specified in the DT we end up without phy...
>
> There are AT91 platforms which lack specification for the phy node in
> the DT. So, I don't know if there is a better way to deal with this case
> but I see this removal as risky.
>
> Bye,
>
> Nicolas Ferre
Hmm, are AT91 platforms special in this regard? As far as I can tell, this
driver (macb) and Marvell PXA are the only ethernet drivers that call
mdiobus_scan directly, and PXA does it on a known address. I do see that there
are trees that use macb and don't have a phy listed, which is unfortunate.
Another way to fix our issue would be to consider all 0x0s a bad ID in
mdiobus_scan, so grounded MDIO lines do not get PHYs scanned. Or we could add a
DT property to disable the manual scan. I'm not sure what the correct solution
is, do you have a preference?
^ permalink raw reply
* Re: [PATCH net v3 0/5] drivers: net: cpsw: phy-handle fixes
From: Grygorii Strashko @ 2016-04-28 15:55 UTC (permalink / raw)
To: David Rivshin (Allworx), netdev, linux-omap
Cc: linux-arm-kernel, devicetree, linux-kernel, David Miller,
Mugunthan V N, Andrew Goodbody, Markus Brunner, Nicolas Chauvet
In-Reply-To: <1461805808-4102-1-git-send-email-drivshin.allworx@gmail.com>
On 04/28/2016 04:10 AM, David Rivshin (Allworx) wrote:
> From: David Rivshin <drivshin@allworx.com>
>
> This series fixes a number of related issues around using phy-handle
> properties in cpsw emac nodes.
>
> Patch 1 fixes a bug if more than one slave is used, and either
> slave uses the phy-handle property in the devicetree.
>
> Patch 2 fixes a NULL pointer dereference which can occur if a
> phy-handle property is used and of_phy_connect() return NULL,
> such as with a bad devicetree.
>
> Patch 3 fixes an issue where the phy-mode property would be ignored
> if a phy-handle property was used. This also fixes a bogus error
> message that would be emitted.
>
> Patch 4 fixes makes the binding documentation more explicit that
> exactly one PHY property should be used, and also marks phy_id as
> deprecated.
>
> Patch 5 cleans up the fixed-link case to work like the now-fixed
> phy-handle case.
>
> I have tested on the following hardware configurations:
> - (EVMSK) dual emac, phy_id property in both slaves
> - (EVMSK) dual emac, phy-handle property in both slaves
> - (EVMSK) a bad phy-handle property pointing to &mmc1
> - (EVMSK) phy_id property with incorrect PHY address
> - (BeagleBoneBlack) single emac, phy_id property
> - (custom) single emac, fixed-link subnode
>
> Andrew Goodbody reported testing v2 on a board that doesn't use
> dual_emac mode, but with 2 PHYs using phy-handle properties [1].
>
> Nicolas Chauvet reported testing v2 on an HP t410 (dm8148).
>
> Markus Brunner reported testing v1 on the following [2]:
> - emac0 with phy_id and emac1 with fixed phy
> - emac0 with phy-handle and emac1 with fixed phy
> - emac0 with fixed phy and emac1 with fixed phy
>
> [1] https://lkml.org/lkml/2016/4/22/537
> [2] http://www.spinics.net/lists/netdev/msg357890.html
>
> David Rivshin (5):
> drivers: net: cpsw: fix parsing of phy-handle DT property in dual_emac
> config
> drivers: net: cpsw: fix segfault in case of bad phy-handle
> drivers: net: cpsw: don't ignore phy-mode if phy-handle is used
> dt: cpsw: phy-handle, phy_id, and fixed-link are mutually exclusive
> drivers: net: cpsw: use of_phy_connect() in fixed-link case
>
> Documentation/devicetree/bindings/net/cpsw.txt | 6 +--
> drivers/net/ethernet/ti/cpsw.c | 69 ++++++++++++++------------
> drivers/net/ethernet/ti/cpsw.h | 1 +
> 3 files changed, 41 insertions(+), 35 deletions(-)
>
Thanks a lot.
Reviewed-by: Grygorii Strashko <grygorii.strashko@ti.com>
--
regards,
-grygorii
^ permalink raw reply
* [PATCH net-next] net: constify is_skb_forwardable's arguments
From: Nikolay Aleksandrov @ 2016-04-28 15:59 UTC (permalink / raw)
To: netdev; +Cc: davem, roopa, Nikolay Aleksandrov
is_skb_forwardable is not supposed to change anything so constify its
arguments
Signed-off-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
Hit this while working on the bridge per-vlan stats and needed to pass a
constified skb down. skb_is_gso() already takes a const skb.
include/linux/netdevice.h | 3 ++-
net/core/dev.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 18d8394f2e5d..f9d573e26db3 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -3260,7 +3260,8 @@ struct sk_buff *dev_hard_start_xmit(struct sk_buff *skb, struct net_device *dev,
struct netdev_queue *txq, int *ret);
int __dev_forward_skb(struct net_device *dev, struct sk_buff *skb);
int dev_forward_skb(struct net_device *dev, struct sk_buff *skb);
-bool is_skb_forwardable(struct net_device *dev, struct sk_buff *skb);
+bool is_skb_forwardable(const struct net_device *dev,
+ const struct sk_buff *skb);
extern int netdev_budget;
diff --git a/net/core/dev.c b/net/core/dev.c
index 6324bc9267f7..36f0170a0754 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1741,7 +1741,7 @@ static inline void net_timestamp_set(struct sk_buff *skb)
__net_timestamp(SKB); \
} \
-bool is_skb_forwardable(struct net_device *dev, struct sk_buff *skb)
+bool is_skb_forwardable(const struct net_device *dev, const struct sk_buff *skb)
{
unsigned int len;
--
2.4.11
^ permalink raw reply related
* Re: [RFC PATCH net] ipv6/ila: fix nlsize calculation for lwtunnel
From: Tom Herbert @ 2016-04-28 16:07 UTC (permalink / raw)
To: David Miller; +Cc: Nicolas Dichtel, Linux Kernel Network Developers
In-Reply-To: <20160427.152053.2243888701043810040.davem@davemloft.net>
On Wed, Apr 27, 2016 at 12:20 PM, David Miller <davem@davemloft.net> wrote:
> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> Date: Fri, 22 Apr 2016 17:58:02 +0200
>
>> The handler 'ila_fill_encap_info' adds one attribute: ILA_ATTR_LOCATOR.
>>
>> Fixes: 65d7ab8de582 ("net: Identifier Locator Addressing module")
>> CC: Tom Herbert <tom@herbertland.com>
>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>> ---
>>
>> Tom, when I read the comment, I feel I'm misssing something, but what?
>
> Tom, seriously, please look at this.
>
Yes, it is an issue. Need to do add size for ILA_ATTR_LOCATOR and
ILA_ATTR_CSUM_MODE. Also not we made ILA_ATTR_CSUM_MODE u64 in fill
encap info. I will send a path shortly.
> And with recent changes in net-next the csum attribute size needs to
> be specified as well, plus the locator needs to use the 64-bit
> alignment sizing helper.
Something other than nla_put_u64_64bit?
Thanks,
Tom
^ permalink raw reply
* Re: [RFC PATCH net] ipv6/ila: fix nlsize calculation for lwtunnel
From: David Miller @ 2016-04-28 16:16 UTC (permalink / raw)
To: tom; +Cc: nicolas.dichtel, netdev
In-Reply-To: <CALx6S36jkPqx5U+B9akRYZ+xiRhozk_AdOmKBEr4N9mhwTKQMw@mail.gmail.com>
From: Tom Herbert <tom@herbertland.com>
Date: Thu, 28 Apr 2016 09:07:25 -0700
> On Wed, Apr 27, 2016 at 12:20 PM, David Miller <davem@davemloft.net> wrote:
>> From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>> Date: Fri, 22 Apr 2016 17:58:02 +0200
>>
>>> The handler 'ila_fill_encap_info' adds one attribute: ILA_ATTR_LOCATOR.
>>>
>>> Fixes: 65d7ab8de582 ("net: Identifier Locator Addressing module")
>>> CC: Tom Herbert <tom@herbertland.com>
>>> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
>>> ---
>>>
>>> Tom, when I read the comment, I feel I'm misssing something, but what?
>>
>> Tom, seriously, please look at this.
>>
> Yes, it is an issue. Need to do add size for ILA_ATTR_LOCATOR and
> ILA_ATTR_CSUM_MODE. Also not we made ILA_ATTR_CSUM_MODE u64 in fill
> encap info. I will send a path shortly.
>
>> And with recent changes in net-next the csum attribute size needs to
>> be specified as well, plus the locator needs to use the 64-bit
>> alignment sizing helper.
>
> Something other than nla_put_u64_64bit?
That's what it should use, yes.
^ permalink raw reply
* Re: [PATCH v2] net: macb: do not scan PHYs manually
From: Andrew Lunn @ 2016-04-28 16:32 UTC (permalink / raw)
To: Nathan Sullivan
Cc: Nicolas Ferre, netdev, linux-kernel, Florian Fainelli,
Alexandre Belloni
In-Reply-To: <20160428155556.GA8333@nathan3500-linux-VM>
> Hmm, are AT91 platforms special in this regard? As far as I can tell, this
> driver (macb) and Marvell PXA are the only ethernet drivers that call
> mdiobus_scan directly, and PXA does it on a known address. I do see that there
> are trees that use macb and don't have a phy listed, which is unfortunate.
How it is supposed to work is that you do one of two things:
1) Your device tree does not have an mdio node. In this case, you call
mdiobus_register() and it will perform a scan of the bus, and find the
phys.
2) Your device tree does have an MDIO node, and you list your PHYs.
Having an MDIO node and not listing the PHYs is broken...
There are however, a few broken device trees around, and a few drivers
have workarounds. e.g. davinci_mdio.c
/* register the mii bus
* Create PHYs from DT only in case if PHY child nodes are explicitly
* defined to support backward compatibility with DTs which assume that
* Davinci MDIO will always scan the bus for PHYs detection.
*/
if (dev->of_node && of_get_child_count(dev->of_node)) {
data->skip_scan = true;
ret = of_mdiobus_register(data->bus, dev->of_node);
} else {
ret = mdiobus_register(data->bus);
}
You probably need to do the same for AT91, count the number of
children, and it if it zero, fall back to the non-DT way. It would
also be good to print a warning to get people to fix their device
tree.
Andrew
^ permalink raw reply
* Re: [PATCH net-next] macvtap: add namespace support to the sysfs device class
From: Marc Angel @ 2016-04-28 16:35 UTC (permalink / raw)
To: Eric W. Biederman; +Cc: netdev
In-Reply-To: <8737q933kv.fsf@x220.int.ebiederm.org>
On Mon, Apr 25, 2016 at 8:12 PM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
>> The 'net' device class is isolated between network namespaces so each
>> one has its own hierarchy of net devices.
>> This isn't the case for the 'macvtap' device class.
>> The problem occurs half-way through the netdev registration, when
>> `macvtap_device_event` is called-back to create the 'tapNN' macvtap
>> class device under the 'macvtapX' net class device.
>>
>> This patch adds namespace support the the 'macvtap' device class so
>> that /sys/class/macvtap is no longer shared between net namespaces.
>>
>> However, doing this has the side effect of changing
>> /sys/devices/virtual/net/macvtapX/tapNN into
>> /sys/devices/virtual/net/macvtapX/macvtap/tapNN
>
> I forget the details of how this interface works, but
> /sys/devices/virtual/net is definitely allows different overlapping
> content per network namespace, so we should not need to add an extra
> directory to make this work.
It really seems like we do, unfortunately.
For a kernfs_node to have the KERNFS_NS flag enabled, sysfs_enable_ns
has to be called on it. This is only done in the create_dir function of
lib/kobject.c, and only when the parent of that kobject has a ktype with
the child_ns_type field set to something.
This is the case for class_dir_ktype which is the type used for the
"glue" dirs (the extra macvtap/ that is created under macvtapX).
This, however, is not the case for device_ktype, which is the type
used for every device directory.
When we create tapN directly under macvtapX, tapN doesn't get the
KERNFS_NS flag enabled -- unlike when created under the "glue" dir.
This is problematic when creating the following symlink:
/sys/class/macvtap/tapN -> /sys/devices/virtual/net/macvtapX/tapN.
The tapN in /sys/class/macvtap inherits the namespace tag from
/sys/devices/virtual/net/macvtapX/tapN, which doesn't have one anymore
and kernfs_add_one fails because it expects it to.
Adding a child_ns_type field to device_ktype is probably not a good idea
and seems to cause other problems.
The best workaround is probably to just create a symlink inside the
macvtapX device directory (tapN -> macvtap/tapN).
I'll update my patch accordingly if you don't have a better idea.
>> Should it even be possible to add a device of a class that doesn't
>> support namespaces under one that does?
>> This could lead to dead symlinks in the new device class directory or
>> duplicate warnings because a device of the same name already exists in
>> another namespace.
>
> This definitely looks like something that bears digging into, and fixing
> properly.
>
> Eric
^ permalink raw reply
* Re: [net-next PATCH V4 3/5] samples/bpf: add a README file to get users started
From: Alexei Starovoitov @ 2016-04-28 16:49 UTC (permalink / raw)
To: Jesper Dangaard Brouer
Cc: netdev, linux-kbuild, bblanco, naveen.n.rao, borkmann
In-Reply-To: <20160428122103.16807.41837.stgit@firesoul>
On Thu, Apr 28, 2016 at 02:21:04PM +0200, Jesper Dangaard Brouer wrote:
> Getting started with using examples in samples/bpf/ is not
> straightforward. There are several dependencies, and specific
> versions of these dependencies.
>
> Just compiling the example tool is also slightly obscure, e.g. one
> need to call make like:
>
> make samples/bpf/
>
> Do notice the "/" slash after the directory name.
>
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
Acked-by: Alexei Starovoitov <ast@kernel.org>
^ permalink raw reply
* Re: iproute2: bash completion function for tc
From: Alexei Starovoitov @ 2016-04-28 16:53 UTC (permalink / raw)
To: Quentin Monnet
Cc: Stephen Hemminger, Jamal Hadi Salim, Vincent Jardin, netdev
In-Reply-To: <5721FEE1.7080807@6wind.com>
On Thu, Apr 28, 2016 at 02:15:29PM +0200, Quentin Monnet wrote:
> Hi Alexei, Stephen,
>
> 2016-04-27 (22:13 UTC-0700) ~ Stephen Hemminger:
> > On Wed, 27 Apr 2016 20:19:26 -0700
> > Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> >
> >> On Tue, Apr 26, 2016 at 09:28:17AM +0200, Quentin Monnet wrote:
> >>> Hi Jamal, Stephen,
> >>>
> >>> I searched for a function providing auto-completion for `tc` utility in
> >>> bash, but I found none. So I have created one, and I would like share it
> >>> with the community. It is available here:
> >>> https://github.com/6WIND/tc_bash-completion/blob/master/tc
> >>> I would like to make it easily available to tc users, so here is a
> >>> twofold request:
> >>>
> >>> * I do not know where to submit the code. Should I submit here on netdev
> >>> for inclusion in iproute2 package, or rather to the bash-completion
> >>> repository on GitHub? I feel like it would receive better feedback and
> >>> updates if pushed to iproute2. Could you please provide some advice here?
> >>> * The completion for `tc` seems to work well; I have tested it with many
> >>> commands, but I am no tc expert, and there are probably some cases where
> >>> the completion fails to propose the correct choices. I would be really
> >>> interested in any feedback/bug reports that you, or anyone on this list
> >>> who uses tc, could provide.
> >>
> >> that looks very interesting.
> >> I think making it a part of iproute2 is a good thing.
> >> How about installing it into /etc/iproute2/ ?
> >> Stephen, any comments?
> >>
> >
> > I am ok with keeping it in the repository.
> > But it would need to be installed in the standard bash directory,
> > is that distro dependent?
> >
>
> As far as I know the bash-completion directory is not distro dependent,
> but it moved at some point (2011) from /etc/bash_completion.d/ to
> /usr/share/bash-completion/completions/. While backward compatibility is
> provided with the former location, it is now recommended to use the latter.
>
> So one idea could be to check (in iproute2 Makefile) for existence of
> /usr/share/bash-completion/completions/ directory, and if not found to
> fall back to /etc/bash_completion.d/. If none is found, use
> /usr/share/bash-completion/completions/. Does this seem correct?
make sense to me.
For some reason there is also /etc/bash-completion.d/ (note - vs _)
and git installs there (which seems to be a bug), but your proposal is good as-is.
^ permalink raw reply
* Re: [RFC 07/20] net: dsa: list ports in switch\\
From: Florian Fainelli @ 2016-04-28 17:00 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot
Cc: netdev, linux-kernel, kernel, David S. Miller, Jiri Pirko
In-Reply-To: <20160427231537.GJ29024@lunn.ch>
On 27/04/16 16:15, Andrew Lunn wrote:
> On Wed, Apr 27, 2016 at 06:30:04PM -0400, Vivien Didelot wrote:
>> List DSA port structures in their switch structure, so that drivers can
>> iterate on them to retrieve information such as their ports membership.
>
> And this would be so much easier using a plan array.
Agreed, I do not see much value in doing this at the moment. Even if you
have unused ports in a switch, allocating an array is a small price to
pay compared to directly indexing by port number.
NAK from me unless there is a compelling reason for doing so.
--
Florian
^ permalink raw reply
* [PATCH nf-next 1/9] netfilter: conntrack: keep BH enabled during lookup
From: Florian Westphal @ 2016-04-28 17:13 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, Florian Westphal
In-Reply-To: <1461863628-23350-1-git-send-email-fw@strlen.de>
No need to disable BH here anymore:
stats are switched to _ATOMIC variant (== this_cpu_inc()), which
nowadays generates same code as the non _ATOMIC NF_STAT, at least on x86.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/netfilter/nf_conntrack_core.c | 25 ++++++++-----------------
1 file changed, 8 insertions(+), 17 deletions(-)
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 1fd0ff1..1b63359 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -472,18 +472,13 @@ ____nf_conntrack_find(struct net *net, const struct nf_conntrack_zone *zone,
struct hlist_nulls_node *n;
unsigned int bucket = hash_bucket(hash, net);
- /* Disable BHs the entire time since we normally need to disable them
- * at least once for the stats anyway.
- */
- local_bh_disable();
begin:
hlist_nulls_for_each_entry_rcu(h, n, &net->ct.hash[bucket], hnnode) {
if (nf_ct_key_equal(h, tuple, zone)) {
- NF_CT_STAT_INC(net, found);
- local_bh_enable();
+ NF_CT_STAT_INC_ATOMIC(net, found);
return h;
}
- NF_CT_STAT_INC(net, searched);
+ NF_CT_STAT_INC_ATOMIC(net, searched);
}
/*
* if the nulls value we got at the end of this lookup is
@@ -491,10 +486,9 @@ begin:
* We probably met an item that was moved to another chain.
*/
if (get_nulls_value(n) != bucket) {
- NF_CT_STAT_INC(net, search_restart);
+ NF_CT_STAT_INC_ATOMIC(net, search_restart);
goto begin;
}
- local_bh_enable();
return NULL;
}
@@ -735,22 +729,19 @@ nf_conntrack_tuple_taken(const struct nf_conntrack_tuple *tuple,
zone = nf_ct_zone(ignored_conntrack);
hash = hash_conntrack(net, tuple);
- /* Disable BHs the entire time since we need to disable them at
- * least once for the stats anyway.
- */
- rcu_read_lock_bh();
+ rcu_read_lock();
hlist_nulls_for_each_entry_rcu(h, n, &net->ct.hash[hash], hnnode) {
ct = nf_ct_tuplehash_to_ctrack(h);
if (ct != ignored_conntrack &&
nf_ct_tuple_equal(tuple, &h->tuple) &&
nf_ct_zone_equal(ct, zone, NF_CT_DIRECTION(h))) {
- NF_CT_STAT_INC(net, found);
- rcu_read_unlock_bh();
+ NF_CT_STAT_INC_ATOMIC(net, found);
+ rcu_read_unlock();
return 1;
}
- NF_CT_STAT_INC(net, searched);
+ NF_CT_STAT_INC_ATOMIC(net, searched);
}
- rcu_read_unlock_bh();
+ rcu_read_unlock();
return 0;
}
--
2.7.3
^ permalink raw reply related
* [PATCH nf-next 0/9] netfilter: remove per-netns conntrack tables, part 1
From: Florian Westphal @ 2016-04-28 17:13 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev
[ CCing netdev so netns folks can have a look too ]
This patch series removes the per-netns connection tracking tables.
All conntrack objects are then stored in one global global table.
This avoids the infamous 'vmalloc' when lots of namespaces are used:
We no longer allocate a new conntrack table for each namespace (with 64k
size this saves 512kb of memory per netns).
- net namespace address is made part of conntrack hash, to spread
conntracks over entire table even if netns has overlapping ip addresses.
- lookup and iterators net_eq() to skip conntracks living in a different
namespace.
Only the main conntrack table is converted here:
NAT bysrc and expectation hashes are still per namespace (will be unified
in a followup series). Also, this retains the per-namespace kmem cache
for the conntrack objects. This will also be resolved in a followup series.
Comments welcome.
include/net/netfilter/nf_conntrack_core.h | 11
include/net/netns/conntrack.h | 2
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4.c | 2
net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c | 38 ++
net/netfilter/nf_conntrack_core.c | 233 +++++++++---------
net/netfilter/nf_conntrack_helper.c | 6
net/netfilter/nf_conntrack_netlink.c | 11
net/netfilter/nf_conntrack_standalone.c | 13 -
net/netfilter/nf_nat_core.c | 2
net/netfilter/nfnetlink_cttimeout.c | 6
10 files changed, 179 insertions(+), 145 deletions(-)
^ permalink raw reply
* [PATCH nf-next 2/9] netfilter: conntrack: fix lookup race during hash resize
From: Florian Westphal @ 2016-04-28 17:13 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, Florian Westphal
In-Reply-To: <1461863628-23350-1-git-send-email-fw@strlen.de>
When resizing the conntrack hash table at runtime via
echo 42 > /sys/module/nf_conntrack/parameters/hashsize, we are racing with
the conntrack lookup path -- reads can happen in parallel and nothing
prevents readers from observing a the newly allocated hash but the old
size (or vice versa).
So access to hash[bucket] can trigger OOB read access in case the table got
expanded and we saw the new size but the old hash pointer (or it got shrunk
and we got new hash ptr but the size of the old and larger table):
kasan: GPF could be caused by NULL-ptr deref or user memory access general protection fault: 0000 [#1] SMP KASAN
CPU: 0 PID: 3 Comm: ksoftirqd/0 Not tainted 4.6.0-rc2+ #107
[..]
Call Trace:
[<ffffffff822c3d6a>] ? nf_conntrack_tuple_taken+0x12a/0xe90
[<ffffffff822c3ac1>] ? nf_ct_invert_tuplepr+0x221/0x3a0
[<ffffffff8230e703>] get_unique_tuple+0xfb3/0x2760
Use generation counter to obtain the address/length of the same table.
Also add a synchronize_net before freeing the old hash.
AFAICS, without it we might access ct_hash[bucket] after ct_hash has been
freed, provided that lockless reader got delayed by another event:
CPU1 CPU2
seq_begin
seq_retry
<delay> resize occurs
free oldhash
for_each(oldhash[size])
Note that resize is only supported in init_netns, it took over 2 minutes
of constant resizing+flooding to produce the warning, so this isn't a
big problem in practice.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/netfilter/nf_conntrack_core.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 1b63359..29fa08b 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -469,11 +469,18 @@ ____nf_conntrack_find(struct net *net, const struct nf_conntrack_zone *zone,
const struct nf_conntrack_tuple *tuple, u32 hash)
{
struct nf_conntrack_tuple_hash *h;
+ struct hlist_nulls_head *ct_hash;
struct hlist_nulls_node *n;
- unsigned int bucket = hash_bucket(hash, net);
+ unsigned int bucket, sequence;
begin:
- hlist_nulls_for_each_entry_rcu(h, n, &net->ct.hash[bucket], hnnode) {
+ do {
+ sequence = read_seqcount_begin(&nf_conntrack_generation);
+ bucket = hash_bucket(hash, net);
+ ct_hash = net->ct.hash;
+ } while (read_seqcount_retry(&nf_conntrack_generation, sequence));
+
+ hlist_nulls_for_each_entry_rcu(h, n, &ct_hash[bucket], hnnode) {
if (nf_ct_key_equal(h, tuple, zone)) {
NF_CT_STAT_INC_ATOMIC(net, found);
return h;
@@ -722,15 +729,21 @@ nf_conntrack_tuple_taken(const struct nf_conntrack_tuple *tuple,
struct net *net = nf_ct_net(ignored_conntrack);
const struct nf_conntrack_zone *zone;
struct nf_conntrack_tuple_hash *h;
+ struct hlist_nulls_head *ct_hash;
+ unsigned int hash, sequence;
struct hlist_nulls_node *n;
struct nf_conn *ct;
- unsigned int hash;
zone = nf_ct_zone(ignored_conntrack);
- hash = hash_conntrack(net, tuple);
rcu_read_lock();
- hlist_nulls_for_each_entry_rcu(h, n, &net->ct.hash[hash], hnnode) {
+ do {
+ sequence = read_seqcount_begin(&nf_conntrack_generation);
+ hash = hash_conntrack(net, tuple);
+ ct_hash = net->ct.hash;
+ } while (read_seqcount_retry(&nf_conntrack_generation, sequence));
+
+ hlist_nulls_for_each_entry_rcu(h, n, &ct_hash[hash], hnnode) {
ct = nf_ct_tuplehash_to_ctrack(h);
if (ct != ignored_conntrack &&
nf_ct_tuple_equal(tuple, &h->tuple) &&
@@ -1607,6 +1620,7 @@ int nf_conntrack_set_hashsize(const char *val, struct kernel_param *kp)
nf_conntrack_all_unlock();
local_bh_enable();
+ synchronize_net();
nf_ct_free_hashtable(old_hash, old_size);
return 0;
}
--
2.7.3
^ permalink raw reply related
* [PATCH nf-next 3/9] netfilter: conntrack: don't attempt to iterate over empty table
From: Florian Westphal @ 2016-04-28 17:13 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, Florian Westphal
In-Reply-To: <1461863628-23350-1-git-send-email-fw@strlen.de>
Once we place all conntracks into same table iteration becomes more
costly because the table contains conntracks that we are not interested
in (belonging to other netns).
So don't bother scanning if the current namespace has no entries.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/netfilter/nf_conntrack_core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 29fa08b..f2e75a5 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -1428,6 +1428,9 @@ void nf_ct_iterate_cleanup(struct net *net,
might_sleep();
+ if (atomic_read(&net->ct.count) == 0)
+ return;
+
while ((ct = get_next_corpse(net, iter, data, &bucket)) != NULL) {
/* Time to push up daises... */
if (del_timer(&ct->timeout))
--
2.7.3
^ permalink raw reply related
* [PATCH nf-next 4/9] netfilter: conntrack: use nf_ct_key_equal() in more places
From: Florian Westphal @ 2016-04-28 17:13 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, Florian Westphal
In-Reply-To: <1461863628-23350-1-git-send-email-fw@strlen.de>
This prepares for upcoming change that places all conntracks into a
single, global table. For this to work we will need to also compare
net pointer during lookup. To avoid open-coding such check use the
nf_ct_key_equal helper and then later extend it to also consider net_eq.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/netfilter/nf_conntrack_core.c | 29 +++++++++++------------------
1 file changed, 11 insertions(+), 18 deletions(-)
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index f2e75a5..3b9c302 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -572,16 +572,13 @@ nf_conntrack_hash_check_insert(struct nf_conn *ct)
/* See if there's one in the list already, including reverse */
hlist_nulls_for_each_entry(h, n, &net->ct.hash[hash], hnnode)
- if (nf_ct_tuple_equal(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
- &h->tuple) &&
- nf_ct_zone_equal(nf_ct_tuplehash_to_ctrack(h), zone,
- NF_CT_DIRECTION(h)))
+ if (nf_ct_key_equal(h, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
+ zone))
goto out;
+
hlist_nulls_for_each_entry(h, n, &net->ct.hash[reply_hash], hnnode)
- if (nf_ct_tuple_equal(&ct->tuplehash[IP_CT_DIR_REPLY].tuple,
- &h->tuple) &&
- nf_ct_zone_equal(nf_ct_tuplehash_to_ctrack(h), zone,
- NF_CT_DIRECTION(h)))
+ if (nf_ct_key_equal(h, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
+ zone))
goto out;
add_timer(&ct->timeout);
@@ -665,16 +662,13 @@ __nf_conntrack_confirm(struct sk_buff *skb)
NAT could have grabbed it without realizing, since we're
not in the hash. If there is, we lost race. */
hlist_nulls_for_each_entry(h, n, &net->ct.hash[hash], hnnode)
- if (nf_ct_tuple_equal(&ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
- &h->tuple) &&
- nf_ct_zone_equal(nf_ct_tuplehash_to_ctrack(h), zone,
- NF_CT_DIRECTION(h)))
+ if (nf_ct_key_equal(h, &ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple,
+ zone))
goto out;
+
hlist_nulls_for_each_entry(h, n, &net->ct.hash[reply_hash], hnnode)
- if (nf_ct_tuple_equal(&ct->tuplehash[IP_CT_DIR_REPLY].tuple,
- &h->tuple) &&
- nf_ct_zone_equal(nf_ct_tuplehash_to_ctrack(h), zone,
- NF_CT_DIRECTION(h)))
+ if (nf_ct_key_equal(h, &ct->tuplehash[IP_CT_DIR_REPLY].tuple,
+ zone))
goto out;
/* Timer relative to confirmation time, not original
@@ -746,8 +740,7 @@ nf_conntrack_tuple_taken(const struct nf_conntrack_tuple *tuple,
hlist_nulls_for_each_entry_rcu(h, n, &ct_hash[hash], hnnode) {
ct = nf_ct_tuplehash_to_ctrack(h);
if (ct != ignored_conntrack &&
- nf_ct_tuple_equal(tuple, &h->tuple) &&
- nf_ct_zone_equal(ct, zone, NF_CT_DIRECTION(h))) {
+ nf_ct_key_equal(h, tuple, zone)) {
NF_CT_STAT_INC_ATOMIC(net, found);
rcu_read_unlock();
return 1;
--
2.7.3
^ permalink raw reply related
* [PATCH nf-next 5/9] netfilter: conntrack: small refactoring of conntrack seq_printf
From: Florian Westphal @ 2016-04-28 17:13 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, Florian Westphal
In-Reply-To: <1461863628-23350-1-git-send-email-fw@strlen.de>
The iteration process is lockless, so we test if the conntrack object is
eligible for printing (e.g. is AF_INET) after obtaining the reference
count.
Once we put all conntracks into same hash table we might see more
entries that need to be skipped.
So add a helper and first perform the test in a lockless fashion
for fast skip.
Once we obtain the reference count, just repeat the check.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
.../netfilter/nf_conntrack_l3proto_ipv4_compat.c | 24 +++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
index f0dfe92..483cf79 100644
--- a/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
+++ b/net/ipv4/netfilter/nf_conntrack_l3proto_ipv4_compat.c
@@ -114,6 +114,19 @@ static inline void ct_show_secctx(struct seq_file *s, const struct nf_conn *ct)
}
#endif
+static bool ct_seq_should_skip(const struct nf_conn *ct,
+ const struct nf_conntrack_tuple_hash *hash)
+{
+ /* we only want to print DIR_ORIGINAL */
+ if (NF_CT_DIRECTION(hash))
+ return true;
+
+ if (nf_ct_l3num(ct) != AF_INET)
+ return true;
+
+ return false;
+}
+
static int ct_seq_show(struct seq_file *s, void *v)
{
struct nf_conntrack_tuple_hash *hash = v;
@@ -123,14 +136,15 @@ static int ct_seq_show(struct seq_file *s, void *v)
int ret = 0;
NF_CT_ASSERT(ct);
- if (unlikely(!atomic_inc_not_zero(&ct->ct_general.use)))
+ if (ct_seq_should_skip(ct, hash))
return 0;
+ if (unlikely(!atomic_inc_not_zero(&ct->ct_general.use)))
+ return 0;
- /* we only want to print DIR_ORIGINAL */
- if (NF_CT_DIRECTION(hash))
- goto release;
- if (nf_ct_l3num(ct) != AF_INET)
+ /* check if we raced w. object reuse */
+ if (!nf_ct_is_confirmed(ct) ||
+ ct_seq_should_skip(ct, hash))
goto release;
l3proto = __nf_ct_l3proto_find(nf_ct_l3num(ct));
--
2.7.3
^ permalink raw reply related
* [PATCH nf-next 7/9] netfilter: conntrack: make netns address part of hash
From: Florian Westphal @ 2016-04-28 17:13 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, Florian Westphal
In-Reply-To: <1461863628-23350-1-git-send-email-fw@strlen.de>
Once we place all conntracks into a global hash table we want them to be
spread across entire hash table, even if namespaces have overlapping ip
addresses.
We add nf_conntrack_netns_hash helper to later re-use it for nat bysrc
and expectation hash handling. The helper also allows us to avoid the
(then) pointless hashing of init_net if kernel is built without netns
support.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
include/net/netfilter/nf_conntrack_core.h | 10 +++++++++
net/netfilter/nf_conntrack_core.c | 34 +++++++++++++++----------------
2 files changed, 27 insertions(+), 17 deletions(-)
diff --git a/include/net/netfilter/nf_conntrack_core.h b/include/net/netfilter/nf_conntrack_core.h
index 62e17d1..389e6da 100644
--- a/include/net/netfilter/nf_conntrack_core.h
+++ b/include/net/netfilter/nf_conntrack_core.h
@@ -12,6 +12,7 @@
#ifndef _NF_CONNTRACK_CORE_H
#define _NF_CONNTRACK_CORE_H
+#include <linux/hash.h>
#include <linux/netfilter.h>
#include <net/netfilter/nf_conntrack_l3proto.h>
#include <net/netfilter/nf_conntrack_l4proto.h>
@@ -86,4 +87,13 @@ void nf_conntrack_lock(spinlock_t *lock);
extern spinlock_t nf_conntrack_expect_lock;
+static inline u32 nf_conntrack_netns_hash(const struct net *net)
+{
+#ifdef CONFIG_NET_NS
+ return hash_ptr(net, 32);
+#else
+ return 0;
+#endif
+}
+
#endif /* _NF_CONNTRACK_CORE_H */
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index 10ae2ee..c29b929 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -144,9 +144,11 @@ EXPORT_PER_CPU_SYMBOL(nf_conntrack_untracked);
static unsigned int nf_conntrack_hash_rnd __read_mostly;
-static u32 hash_conntrack_raw(const struct nf_conntrack_tuple *tuple)
+static u32 hash_conntrack_raw(const struct nf_conntrack_tuple *tuple,
+ const struct net *net)
{
unsigned int n;
+ u32 seed;
get_random_once(&nf_conntrack_hash_rnd, sizeof(nf_conntrack_hash_rnd));
@@ -154,32 +156,29 @@ static u32 hash_conntrack_raw(const struct nf_conntrack_tuple *tuple)
* destination ports (which is a multiple of 4) and treat the last
* three bytes manually.
*/
+ seed = nf_conntrack_hash_rnd ^ nf_conntrack_netns_hash(net);
n = (sizeof(tuple->src) + sizeof(tuple->dst.u3)) / sizeof(u32);
- return jhash2((u32 *)tuple, n, nf_conntrack_hash_rnd ^
+ return jhash2((u32 *)tuple, n, seed ^
(((__force __u16)tuple->dst.u.all << 16) |
tuple->dst.protonum));
}
-static u32 __hash_bucket(u32 hash, unsigned int size)
-{
- return reciprocal_scale(hash, size);
-}
-
static u32 hash_bucket(u32 hash, const struct net *net)
{
- return __hash_bucket(hash, net->ct.htable_size);
+ return reciprocal_scale(hash, net->ct.htable_size);
}
-static u_int32_t __hash_conntrack(const struct nf_conntrack_tuple *tuple,
- unsigned int size)
+static u32 __hash_conntrack(const struct net *net,
+ const struct nf_conntrack_tuple *tuple,
+ unsigned int size)
{
- return __hash_bucket(hash_conntrack_raw(tuple), size);
+ return reciprocal_scale(hash_conntrack_raw(tuple, net), size);
}
-static inline u_int32_t hash_conntrack(const struct net *net,
- const struct nf_conntrack_tuple *tuple)
+static u32 hash_conntrack(const struct net *net,
+ const struct nf_conntrack_tuple *tuple)
{
- return __hash_conntrack(tuple, net->ct.htable_size);
+ return __hash_conntrack(net, tuple, net->ct.htable_size);
}
bool
@@ -535,7 +534,7 @@ nf_conntrack_find_get(struct net *net, const struct nf_conntrack_zone *zone,
const struct nf_conntrack_tuple *tuple)
{
return __nf_conntrack_find_get(net, zone, tuple,
- hash_conntrack_raw(tuple));
+ hash_conntrack_raw(tuple, net));
}
EXPORT_SYMBOL_GPL(nf_conntrack_find_get);
@@ -1041,7 +1040,7 @@ resolve_normal_ct(struct net *net, struct nf_conn *tmpl,
/* look for tuple match */
zone = nf_ct_zone_tmpl(tmpl, skb, &tmp);
- hash = hash_conntrack_raw(&tuple);
+ hash = hash_conntrack_raw(&tuple, net);
h = __nf_conntrack_find_get(net, zone, &tuple, hash);
if (!h) {
h = init_conntrack(net, tmpl, &tuple, l3proto, l4proto,
@@ -1605,7 +1604,8 @@ int nf_conntrack_set_hashsize(const char *val, struct kernel_param *kp)
struct nf_conntrack_tuple_hash, hnnode);
ct = nf_ct_tuplehash_to_ctrack(h);
hlist_nulls_del_rcu(&h->hnnode);
- bucket = __hash_conntrack(&h->tuple, hashsize);
+ bucket = __hash_conntrack(nf_ct_net(ct),
+ &h->tuple, hashsize);
hlist_nulls_add_head_rcu(&h->hnnode, &hash[bucket]);
}
}
--
2.7.3
^ permalink raw reply related
* [PATCH nf-next 9/9] netfilter: conntrack: consider ct netns in early_drop logic
From: Florian Westphal @ 2016-04-28 17:13 UTC (permalink / raw)
To: netfilter-devel; +Cc: netdev, Florian Westphal
In-Reply-To: <1461863628-23350-1-git-send-email-fw@strlen.de>
When iterating, skip conntrack entries living in a different netns.
We could ignore netns and kill some other non-assured one, but it
has two problems:
- a netns can kill non-assured conntracks in other namespace
- we would start to 'over-subscribe' the affected/overlimit netns.
Signed-off-by: Florian Westphal <fw@strlen.de>
---
net/netfilter/nf_conntrack_core.c | 43 +++++++++++++++++++++++----------------
1 file changed, 25 insertions(+), 18 deletions(-)
diff --git a/net/netfilter/nf_conntrack_core.c b/net/netfilter/nf_conntrack_core.c
index d58b597..418e4bc 100644
--- a/net/netfilter/nf_conntrack_core.c
+++ b/net/netfilter/nf_conntrack_core.c
@@ -763,18 +763,20 @@ static noinline int early_drop(struct net *net, unsigned int _hash)
{
/* Use oldest entry, which is roughly LRU */
struct nf_conntrack_tuple_hash *h;
- struct nf_conn *ct = NULL, *tmp;
+ struct nf_conn *tmp;
struct hlist_nulls_node *n;
- unsigned int i = 0, cnt = 0;
- int dropped = 0;
- unsigned int hash, sequence;
+ unsigned int i, hash, sequence;
+ struct nf_conn *ct = NULL;
spinlock_t *lockp;
+ bool ret = false;
+
+ i = 0;
local_bh_disable();
restart:
sequence = read_seqcount_begin(&nf_conntrack_generation);
- hash = scale_hash(_hash);
- for (; i < nf_conntrack_htable_size; i++) {
+ for (; i < NF_CT_EVICTION_RANGE; i++) {
+ hash = scale_hash(_hash++);
lockp = &nf_conntrack_locks[hash % CONNTRACK_LOCKS];
nf_conntrack_lock(lockp);
if (read_seqcount_retry(&nf_conntrack_generation, sequence)) {
@@ -784,35 +786,40 @@ restart:
hlist_nulls_for_each_entry_rcu(h, n, &nf_conntrack_hash[hash],
hnnode) {
tmp = nf_ct_tuplehash_to_ctrack(h);
- if (!test_bit(IPS_ASSURED_BIT, &tmp->status) &&
- !nf_ct_is_dying(tmp) &&
- atomic_inc_not_zero(&tmp->ct_general.use)) {
+
+ if (test_bit(IPS_ASSURED_BIT, &tmp->status) ||
+ !net_eq(nf_ct_net(tmp), net) ||
+ nf_ct_is_dying(tmp))
+ continue;
+
+ if (atomic_inc_not_zero(&tmp->ct_general.use)) {
ct = tmp;
break;
}
- cnt++;
}
- hash = (hash + 1) % nf_conntrack_htable_size;
spin_unlock(lockp);
-
- if (ct || cnt >= NF_CT_EVICTION_RANGE)
+ if (ct)
break;
-
}
+
local_bh_enable();
if (!ct)
- return dropped;
+ return false;
- if (del_timer(&ct->timeout)) {
+ /* kill only if in same netns -- might have moved due to
+ * SLAB_DESTROY_BY_RCU rules
+ */
+ if (net_eq(nf_ct_net(ct), net) && del_timer(&ct->timeout)) {
if (nf_ct_delete(ct, 0, 0)) {
- dropped = 1;
NF_CT_STAT_INC_ATOMIC(net, early_drop);
+ ret = true;
}
}
+
nf_ct_put(ct);
- return dropped;
+ return ret;
}
static struct nf_conn *
--
2.7.3
^ 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