* [PATCH v2 rfc 6/8] net: dsa: switch: Don't add CPU port to an mdb by default
From: Andrew Lunn @ 2017-09-05 23:35 UTC (permalink / raw)
To: netdev; +Cc: jiri, nikolay, Florian Fainelli, Vivien Didelot, Andrew Lunn
In-Reply-To: <1504654510-31004-1-git-send-email-andrew@lunn.ch>
Now that the host indicates when a multicast group should be forwarded
from the switch to the host, don't do it by default.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
net/dsa/switch.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 326680039c52..147295d8de94 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -145,7 +145,7 @@ static int dsa_switch_mdb_add(struct dsa_switch *ds,
if (ds->index == info->sw_index)
set_bit(info->port, group);
for (port = 0; port < ds->num_ports; port++)
- if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port))
+ if (dsa_is_dsa_port(ds, port))
set_bit(port, group);
return dsa_switch_mdb_add_bitmap(ds, info, mdb, group);
--
2.14.1
^ permalink raw reply related
* [PATCH v2 rfc 0/8] IGMP snooping for local traffic
From: Andrew Lunn @ 2017-09-05 23:35 UTC (permalink / raw)
To: netdev; +Cc: jiri, nikolay, Florian Fainelli, Vivien Didelot, Andrew Lunn
After the very useful feedback from Nikolay, i threw away what i had,
and started again. To recap:
The linux bridge supports IGMP snooping. It will listen to IGMP
reports on bridge ports and keep track of which groups have been
joined on an interface. It will then forward multicast based on this
group membership.
When the bridge adds or removed groups from an interface, it uses
switchdev to request the hardware add an mdb to a port, so the
hardware can perform the selective forwarding between ports.
What is not covered by the current bridge code, is IGMP joins/leaves
from the host on the brX interface. These are not reported via
switchdev so that hardware knows the local host is interested in the
multicast frames.
Luckily, the bridge does track joins/leaves on the brX interface. The
code is obfusticated, which is why i missed it with my first attempt.
So the first patch tries to remove this obfustication. Currently,
there is no notifications sent when the bridge interface joins a
group. The second patch adds them. bridge monitor then shows
joins/leaves in the same way as for other ports of the bridge.
Then starts the work passing down to the hardware that the host has
joined/left a group. The existing switchdev mdb object cannot be used,
since the semantics are different. The existing
SWITCHDEV_OBJ_ID_PORT_MDB is used to indicate a specific multicast
group should be forwarded out that port of the switch. However here we
require the exact opposite. We want multicast frames for the group
received on the port to the forwarded to the host. Hence add a new
object SWITCHDEV_OBJ_ID_HOST_MDB, a multicast database entry to
forward to the host. This new object is then propagated through the
DSA layers. No DSA driver changes should be needed, this should just
work...
Getting the frames to the bridge as requested turned up an issue or
three. The offload_fwd_mark is not being set by DSA, so the bridge
floods the received frames back to the switch ports, resulting in
duplication since the hardware has already flooded the packet. Fixing
that turned up an issue with the meaning of
SWITCHDEV_ATTR_ID_PORT_PARENT_ID in DSA. A DSA fabric of three
switches needs to look to the software bridge as a single
switch. Otherwise the offload_fwd_mark does not work, and we get
duplication on the non-ingress switch. But each switch returned a
different value. And they were not unique.
The third and last issue will be explained in a followup email.
Open questions:
Is sending notifications going to break userspace?
Is this new switchdev object O.K. for the few non-DSA switches that exist?
Is the SWITCHDEV_ATTR_ID_PORT_PARENT_ID change acceptable?
Andrew
Andrew Lunn (8):
net: bridge: Rename mglist to host_joined
net: bridge: Send notification when host join/leaves a group
net: bridge: Add/del switchdev object on host join/leave
net: dsa: slave: Handle switchdev host mdb add/del
net: dsa: switch: handle host mdb add/remove
net: dsa: switch: Don't add CPU port to an mdb by default
net: dsa: set offload_fwd_mark on received packets
net: dsa: Fix SWITCHDEV_ATTR_ID_PORT_PARENT_ID
include/net/switchdev.h | 1 +
net/bridge/br_input.c | 2 +-
net/bridge/br_mdb.c | 50 +++++++++++++++++++++++++++++---
net/bridge/br_multicast.c | 18 +++++++-----
net/bridge/br_private.h | 2 +-
net/dsa/dsa.c | 1 +
net/dsa/dsa_priv.h | 7 +++++
net/dsa/port.c | 26 +++++++++++++++++
net/dsa/slave.c | 16 ++++++++---
net/dsa/switch.c | 72 +++++++++++++++++++++++++++++++++++++++--------
net/switchdev/switchdev.c | 2 ++
11 files changed, 168 insertions(+), 29 deletions(-)
--
2.14.1
^ permalink raw reply
* [PATCH v2 rfc 4/8] net: dsa: slave: Handle switchdev host mdb add/del
From: Andrew Lunn @ 2017-09-05 23:35 UTC (permalink / raw)
To: netdev; +Cc: jiri, nikolay, Florian Fainelli, Vivien Didelot, Andrew Lunn
In-Reply-To: <1504654510-31004-1-git-send-email-andrew@lunn.ch>
Add code to handle switchdev host mdb add/del. As with normal mdb
add/del, send a notification to the switch layer.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
net/dsa/dsa_priv.h | 7 +++++++
net/dsa/port.c | 26 ++++++++++++++++++++++++++
net/dsa/slave.c | 6 ++++++
3 files changed, 39 insertions(+)
diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index 9c3eeb72462d..0ffe49f78d14 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -24,6 +24,8 @@ enum {
DSA_NOTIFIER_FDB_DEL,
DSA_NOTIFIER_MDB_ADD,
DSA_NOTIFIER_MDB_DEL,
+ DSA_NOTIFIER_HOST_MDB_ADD,
+ DSA_NOTIFIER_HOST_MDB_DEL,
DSA_NOTIFIER_VLAN_ADD,
DSA_NOTIFIER_VLAN_DEL,
};
@@ -131,6 +133,11 @@ int dsa_port_mdb_add(struct dsa_port *dp,
struct switchdev_trans *trans);
int dsa_port_mdb_del(struct dsa_port *dp,
const struct switchdev_obj_port_mdb *mdb);
+int dsa_host_mdb_add(struct dsa_port *dp,
+ const struct switchdev_obj_port_mdb *mdb,
+ struct switchdev_trans *trans);
+int dsa_host_mdb_del(struct dsa_port *dp,
+ const struct switchdev_obj_port_mdb *mdb);
int dsa_port_vlan_add(struct dsa_port *dp,
const struct switchdev_obj_port_vlan *vlan,
struct switchdev_trans *trans);
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 659676ba3f8b..5b18b9fe2219 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -199,6 +199,32 @@ int dsa_port_mdb_del(struct dsa_port *dp,
return dsa_port_notify(dp, DSA_NOTIFIER_MDB_DEL, &info);
}
+int dsa_host_mdb_add(struct dsa_port *dp,
+ const struct switchdev_obj_port_mdb *mdb,
+ struct switchdev_trans *trans)
+{
+ struct dsa_notifier_mdb_info info = {
+ .sw_index = dp->ds->index,
+ .port = dp->index,
+ .trans = trans,
+ .mdb = mdb,
+ };
+
+ return dsa_port_notify(dp, DSA_NOTIFIER_HOST_MDB_ADD, &info);
+}
+
+int dsa_host_mdb_del(struct dsa_port *dp,
+ const struct switchdev_obj_port_mdb *mdb)
+{
+ struct dsa_notifier_mdb_info info = {
+ .sw_index = dp->ds->index,
+ .port = dp->index,
+ .mdb = mdb,
+ };
+
+ return dsa_port_notify(dp, DSA_NOTIFIER_HOST_MDB_DEL, &info);
+}
+
int dsa_port_vlan_add(struct dsa_port *dp,
const struct switchdev_obj_port_vlan *vlan,
struct switchdev_trans *trans)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 78e78a6e6833..2e07be149415 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -330,6 +330,9 @@ static int dsa_slave_port_obj_add(struct net_device *dev,
case SWITCHDEV_OBJ_ID_PORT_MDB:
err = dsa_port_mdb_add(dp, SWITCHDEV_OBJ_PORT_MDB(obj), trans);
break;
+ case SWITCHDEV_OBJ_ID_HOST_MDB:
+ err = dsa_host_mdb_add(dp, SWITCHDEV_OBJ_PORT_MDB(obj), trans);
+ break;
case SWITCHDEV_OBJ_ID_PORT_VLAN:
err = dsa_port_vlan_add(dp, SWITCHDEV_OBJ_PORT_VLAN(obj),
trans);
@@ -353,6 +356,9 @@ static int dsa_slave_port_obj_del(struct net_device *dev,
case SWITCHDEV_OBJ_ID_PORT_MDB:
err = dsa_port_mdb_del(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
break;
+ case SWITCHDEV_OBJ_ID_HOST_MDB:
+ err = dsa_host_mdb_del(dp, SWITCHDEV_OBJ_PORT_MDB(obj));
+ break;
case SWITCHDEV_OBJ_ID_PORT_VLAN:
err = dsa_port_vlan_del(dp, SWITCHDEV_OBJ_PORT_VLAN(obj));
break;
--
2.14.1
^ permalink raw reply related
* [PATCH v2 rfc 8/8] net: dsa: Fix SWITCHDEV_ATTR_ID_PORT_PARENT_ID
From: Andrew Lunn @ 2017-09-05 23:35 UTC (permalink / raw)
To: netdev; +Cc: jiri, nikolay, Florian Fainelli, Vivien Didelot, Andrew Lunn
In-Reply-To: <1504654510-31004-1-git-send-email-andrew@lunn.ch>
SWITCHDEV_ATTR_ID_PORT_PARENT_ID is used by the software bridge when
determining which ports to flood a packet out. If the packet
originated from a switch, it assumes the switch has already flooded
the packet out the switches ports, so the bridge should not flood the
packet itself out switch ports. Ports on the same switch are expected
to return the same parent ID when SWITCHDEV_ATTR_ID_PORT_PARENT_ID is
called.
DSA gets this wrong with clusters of switches. As far as the software
bridge is concerned, the cluster is all one switch. A packet from any
switch in the cluster can be assumed to of been flooded as needed out
all ports of the cluster, not just the switch it originated
from. Hence all ports of a cluster should return the same parent. The
old implementation did not, each switch in the cluster had its own ID.
Also wrong was that the ID was not unique if multiple DSA instances
are in operation.
Use the MAC address of the master interface as the parent ID. This is
the same for all switches in a cluster, and should be unique if there
are multiple clusters.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
net/dsa/slave.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 2e07be149415..d2744b0dad6e 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -374,13 +374,15 @@ static int dsa_slave_port_attr_get(struct net_device *dev,
struct switchdev_attr *attr)
{
struct dsa_slave_priv *p = netdev_priv(dev);
- struct dsa_switch *ds = p->dp->ds;
switch (attr->id) {
- case SWITCHDEV_ATTR_ID_PORT_PARENT_ID:
- attr->u.ppid.id_len = sizeof(ds->index);
- memcpy(&attr->u.ppid.id, &ds->index, attr->u.ppid.id_len);
+ case SWITCHDEV_ATTR_ID_PORT_PARENT_ID: {
+ struct net_device *master = dsa_master_netdev(p);
+
+ attr->u.ppid.id_len = ETH_ALEN;
+ ether_addr_copy(attr->u.ppid.id, master->dev_addr);
break;
+ }
case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT:
attr->u.brport_flags_support = 0;
break;
--
2.14.1
^ permalink raw reply related
* [PATCH v2 rfc 5/8] net: dsa: switch: handle host mdb add/remove
From: Andrew Lunn @ 2017-09-05 23:35 UTC (permalink / raw)
To: netdev; +Cc: jiri, nikolay, Florian Fainelli, Vivien Didelot, Andrew Lunn
In-Reply-To: <1504654510-31004-1-git-send-email-andrew@lunn.ch>
When receiving notifications of the host mdb add/remove, add/remove an
mdb to the CPU port, so that traffic flows from a port to the CPU port
and hence to the host.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
net/dsa/switch.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++----------
1 file changed, 60 insertions(+), 12 deletions(-)
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index e6c06aa349a6..326680039c52 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -108,22 +108,14 @@ static int dsa_switch_fdb_del(struct dsa_switch *ds,
info->vid);
}
-static int dsa_switch_mdb_add(struct dsa_switch *ds,
- struct dsa_notifier_mdb_info *info)
+static int dsa_switch_mdb_add_bitmap(struct dsa_switch *ds,
+ struct dsa_notifier_mdb_info *info,
+ const struct switchdev_obj_port_mdb *mdb,
+ unsigned long *group)
{
- const struct switchdev_obj_port_mdb *mdb = info->mdb;
struct switchdev_trans *trans = info->trans;
- DECLARE_BITMAP(group, ds->num_ports);
int port, err;
- /* Build a mask of Multicast group members */
- bitmap_zero(group, ds->num_ports);
- if (ds->index == info->sw_index)
- set_bit(info->port, group);
- for (port = 0; port < ds->num_ports; port++)
- if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port))
- set_bit(port, group);
-
if (switchdev_trans_ph_prepare(trans)) {
if (!ds->ops->port_mdb_prepare || !ds->ops->port_mdb_add)
return -EOPNOTSUPP;
@@ -141,6 +133,40 @@ static int dsa_switch_mdb_add(struct dsa_switch *ds,
return 0;
}
+static int dsa_switch_mdb_add(struct dsa_switch *ds,
+ struct dsa_notifier_mdb_info *info)
+{
+ const struct switchdev_obj_port_mdb *mdb = info->mdb;
+ DECLARE_BITMAP(group, ds->num_ports);
+ int port;
+
+ /* Build a mask of Multicast group members */
+ bitmap_zero(group, ds->num_ports);
+ if (ds->index == info->sw_index)
+ set_bit(info->port, group);
+ for (port = 0; port < ds->num_ports; port++)
+ if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port))
+ set_bit(port, group);
+
+ return dsa_switch_mdb_add_bitmap(ds, info, mdb, group);
+}
+
+static int dsa_switch_host_mdb_add(struct dsa_switch *ds,
+ struct dsa_notifier_mdb_info *info)
+{
+ const struct switchdev_obj_port_mdb *mdb = info->mdb;
+ DECLARE_BITMAP(group, ds->num_ports);
+ int port;
+
+ /* Build a mask of Multicast group members */
+ bitmap_zero(group, ds->num_ports);
+ for (port = 0; port < ds->num_ports; port++)
+ if (dsa_is_cpu_port(ds, port))
+ set_bit(port, group);
+
+ return dsa_switch_mdb_add_bitmap(ds, info, mdb, group);
+}
+
static int dsa_switch_mdb_del(struct dsa_switch *ds,
struct dsa_notifier_mdb_info *info)
{
@@ -155,6 +181,22 @@ static int dsa_switch_mdb_del(struct dsa_switch *ds,
return 0;
}
+static int dsa_switch_host_mdb_del(struct dsa_switch *ds,
+ struct dsa_notifier_mdb_info *info)
+{
+ const struct switchdev_obj_port_mdb *mdb = info->mdb;
+ int port;
+
+ if (!ds->ops->port_mdb_del)
+ return -EOPNOTSUPP;
+
+ for (port = 0; port < ds->num_ports; port++)
+ if (dsa_is_cpu_port(ds, port))
+ ds->ops->port_mdb_del(ds, port, mdb);
+
+ return 0;
+}
+
static int dsa_switch_vlan_add(struct dsa_switch *ds,
struct dsa_notifier_vlan_info *info)
{
@@ -230,6 +272,12 @@ static int dsa_switch_event(struct notifier_block *nb,
case DSA_NOTIFIER_MDB_DEL:
err = dsa_switch_mdb_del(ds, info);
break;
+ case DSA_NOTIFIER_HOST_MDB_ADD:
+ err = dsa_switch_host_mdb_add(ds, info);
+ break;
+ case DSA_NOTIFIER_HOST_MDB_DEL:
+ err = dsa_switch_host_mdb_del(ds, info);
+ break;
case DSA_NOTIFIER_VLAN_ADD:
err = dsa_switch_vlan_add(ds, info);
break;
--
2.14.1
^ permalink raw reply related
* [PATCH v2 rfc 7/8] net: dsa: set offload_fwd_mark on received packets
From: Andrew Lunn @ 2017-09-05 23:35 UTC (permalink / raw)
To: netdev; +Cc: jiri, nikolay, Florian Fainelli, Vivien Didelot, Andrew Lunn
In-Reply-To: <1504654510-31004-1-git-send-email-andrew@lunn.ch>
The software bridge needs to know if a packet has already been bridged
by hardware offload to ports in the same hardware offload, in order
that it does not re-flood them, causing duplicates. This is
particularly true for multicast traffic which the host has required.
By setting offload_fwd_mark in the skb the bridge will only flood to
ports in other offloads and other netifs.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
net/dsa/dsa.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index 03c58b0eb082..5732696ac71c 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -213,6 +213,7 @@ static int dsa_switch_rcv(struct sk_buff *skb, struct net_device *dev,
skb_push(skb, ETH_HLEN);
skb->pkt_type = PACKET_HOST;
skb->protocol = eth_type_trans(skb, skb->dev);
+ skb->offload_fwd_mark = 1;
s = this_cpu_ptr(p->stats64);
u64_stats_update_begin(&s->syncp);
--
2.14.1
^ permalink raw reply related
* [PATCH v2 rfc 2/8] net: bridge: Send notification when host join/leaves a group
From: Andrew Lunn @ 2017-09-05 23:35 UTC (permalink / raw)
To: netdev; +Cc: jiri, nikolay, Florian Fainelli, Vivien Didelot, Andrew Lunn
In-Reply-To: <1504654510-31004-1-git-send-email-andrew@lunn.ch>
The host can join or leave a multicast group on the brX interface, as
indicated by IGMP snooping. This is tracked within the bridge
multicast code. Send a notification when this happens, in the same way
a notification is sent when a port of the bridge joins/leaves a group
because of IGMP snooping.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
---
net/bridge/br_mdb.c | 9 ++++++---
net/bridge/br_multicast.c | 6 +++++-
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index 71885e251988..80fa91ccc50c 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -316,7 +316,7 @@ static void __br_mdb_notify(struct net_device *dev, struct net_bridge_port *p,
#endif
mdb.obj.orig_dev = port_dev;
- if (port_dev && type == RTM_NEWMDB) {
+ if (p && port_dev && type == RTM_NEWMDB) {
complete_info = kmalloc(sizeof(*complete_info), GFP_ATOMIC);
if (complete_info) {
complete_info->port = p;
@@ -326,7 +326,7 @@ static void __br_mdb_notify(struct net_device *dev, struct net_bridge_port *p,
if (switchdev_port_obj_add(port_dev, &mdb.obj))
kfree(complete_info);
}
- } else if (port_dev && type == RTM_DELMDB) {
+ } else if (p && port_dev && type == RTM_DELMDB) {
switchdev_port_obj_del(port_dev, &mdb.obj);
}
@@ -352,7 +352,10 @@ void br_mdb_notify(struct net_device *dev, struct net_bridge_port *port,
struct br_mdb_entry entry;
memset(&entry, 0, sizeof(entry));
- entry.ifindex = port->dev->ifindex;
+ if (port)
+ entry.ifindex = port->dev->ifindex;
+ else
+ entry.ifindex = dev->ifindex;
entry.addr.proto = group->proto;
entry.addr.u.ip4 = group->u.ip4;
#if IS_ENABLED(CONFIG_IPV6)
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index c6b2b8d419e7..955f340fe719 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -250,6 +250,7 @@ static void br_multicast_group_expired(unsigned long data)
goto out;
mp->host_joined = false;
+ br_mdb_notify(br->dev, NULL, &mp->addr, RTM_DELMDB, 0);
if (mp->ports)
goto out;
@@ -775,7 +776,10 @@ static int br_multicast_add_group(struct net_bridge *br,
goto err;
if (!port) {
- mp->host_joined = true;
+ if (!mp->host_joined) {
+ mp->host_joined = true;
+ br_mdb_notify(br->dev, NULL, &mp->addr, RTM_NEWMDB, 0);
+ }
mod_timer(&mp->timer, now + br->multicast_membership_interval);
goto out;
}
--
2.14.1
^ permalink raw reply related
* Re: [RFC net-next] net: sch_clsact: add support for global per-netns classifier mode
From: Daniel Borkmann @ 2017-09-05 23:12 UTC (permalink / raw)
To: Roopa Prabhu, Cong Wang
Cc: Nikolay Aleksandrov, Linux Kernel Network Developers, David Ahern,
Jiri Pirko, Jamal Hadi Salim
In-Reply-To: <59AF291E.90508@iogearbox.net>
On 09/06/2017 12:45 AM, Daniel Borkmann wrote:
> On 09/06/2017 12:01 AM, Roopa Prabhu wrote:
>> On Tue, Sep 5, 2017 at 11:18 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>> On Tue, Sep 5, 2017 at 5:48 AM, Nikolay Aleksandrov
>>> <nikolay@cumulusnetworks.com> wrote:
>>>> Hi all,
>>>> This RFC adds a new mode for clsact which designates a device's egress
>>>> classifier as global per netns. The packets that are not classified for
>>>> a particular device will be classified using the global classifier.
>>>> We have needed a global classifier for some time now for various
>>>> purposes and setting the single bridge or loopback/vrf device as the
>
> Can you elaborate a bit more on the ... "we have needed a global
> classifier for some time now for various purposes".
>
>>>> global classifier device is acceptable for us. Doing it this way avoids
>>>> the act/cls device and queue dependencies.
>>>>
>>>> This is strictly an RFC patch just to show the intent, if we agree on
>>>> the details the proposed patch will have support for both ingress and
>>>> egress, and will be using a static key to avoid the fast path test when no
>>>> global classifier has been configured.
>>>>
>>>> Example (need a modified tc that adds TCA_OPTIONS when using q_clsact):
>>>> $ tc qdisc add dev lo clsact global
>>>> $ tc filter add dev lo egress protocol ip u32 match ip dst 4.3.2.1/32 action drop
>>>>
>>>> the last filter will be global for all devices that don't have a
>>>> specific egress_cl_list (i.e. have clsact configured).
>>>
>>> Sorry this is too ugly.
>
> +1
>
>>> netdevice is still implied in your command line even if you treat it
>>> as global. It is essentially hard to bypass netdevice layer since
>>> netdevice is the core of L2 and also where everything begins.
One could probably use special wildcard device name, e.g. tcpdump
allows for 'tcpdump -i any' to capture on all devices, so you could
indicate something like 'tc filter add dev any blah' to have similar
semantics in the realm of the netns w/o making it look too hacky
for tc users perhaps.
>>> Maybe the best we can do here is make tc filters standalone
>>> as tc actions so that filters can exist before qdisc's and netdevices.
>>> But this probably requires significant works to make it working
>>> with both existing non-standalone and bindings standalones
>>> with qdisc's.
>>
>> yes, like Nikolay says we have been discussing this as well. Nikolay's
>> patch is a cleaver and most importantly non-invasive
>> way today given the anchor point for tc rules is a netdev. we have
>> also considered a separate implicit tc anchor device.
>
> Seems ugly just as well. :( Hmm, why not just having the two list
> pointers (ingress, egress list) in the netns struct and when
> something configures them to be effectively non-zero, then devices
> in that netns could automatically get a clsact and inherit the
> lists from there such that sch_handle_ingress() and sch_handle_egress()
> require exactly zero changes in fast-path. You could then go and
> say that either you would make changes to clsact for individual
> devices immutable when they use the 'shared' list pointers, or then
> duplicate the configs when being altered from the global one. Would
> push the complexity to control path only at least. Just a brief
> thought.
^ permalink raw reply
* Re: linux-next: manual merge of the pci tree with the net tree
From: Stephen Rothwell @ 2017-09-05 23:10 UTC (permalink / raw)
To: Sinan Kaya
Cc: Bjorn Helgaas, David Miller, Networking, Linux-Next Mailing List,
Linux Kernel Mailing List, Ding Tianhong, Casey Leedom
In-Reply-To: <22fb2201-6ecd-bfc9-9f65-4807d7bf36a1@codeaurora.org>
Hi Sinan,
On Tue, 5 Sep 2017 18:54:38 -0400 Sinan Kaya <okaya@codeaurora.org> wrote:
>
> On 9/4/2017 12:54 AM, Stephen Rothwell wrote:
> > Just a reminder that this conflict still exists.
>
> I suppose this was a message for Bjorn, right?
Correct. It was just a reminder that he may have to tell Linus about
the conflict when he asks Linus to merge his tree (depending on how
complex the conflict is and if he gets in before Dave gets the net-next
tree merged).
--
Cheers,
Stephen Rothwell
^ permalink raw reply
* Re: linux-next: manual merge of the pci tree with the net tree
From: Sinan Kaya @ 2017-09-05 22:54 UTC (permalink / raw)
To: Stephen Rothwell, Bjorn Helgaas, David Miller, Networking
Cc: Linux-Next Mailing List, Linux Kernel Mailing List, Ding Tianhong,
Casey Leedom
In-Reply-To: <20170904145427.60b35b0c@canb.auug.org.au>
On 9/4/2017 12:54 AM, Stephen Rothwell wrote:
> Just a reminder that this conflict still exists.
I suppose this was a message for Bjorn, right?
I looked at the change and it looked good to me.
--
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
^ permalink raw reply
* Re: [PATCH net-next RFC 1/2] tun: enable NAPI for TUN/TAP driver
From: Stephen Hemminger @ 2017-09-05 22:51 UTC (permalink / raw)
To: Petar Penkov
Cc: netdev, Eric Dumazet, Mahesh Bandewar, Willem de Bruijn, davem,
ppenkov
In-Reply-To: <20170905223551.27925-2-ppenkov@google.com>
On Tue, 5 Sep 2017 15:35:50 -0700
Petar Penkov <ppenkov@google.com> wrote:
> Changes TUN driver to use napi_gro_receive() upon receiving packets
> rather than netif_rx_ni(). Adds flag CONFIG_TUN_NAPI that enables
> these changes and operation is not affected if the flag is disabled.
> SKBs are constructed upon packet arrival and are queued to be
> processed later.
>
> The new path was evaluated with a benchmark with the following setup:
> Open two tap devices and a receiver thread that reads in a loop for
> each device. Start one sender thread and pin all threads to different
> CPUs. Send 1M minimum UDP packets to each device and measure sending
> time for each of the sending methods:
> napi_gro_receive(): 4.90s
> netif_rx_ni(): 4.90s
> netif_receive_skb(): 7.20s
>
> Signed-off-by: Petar Penkov <ppenkov@google.com>
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Mahesh Bandewar <maheshb@google.com>
> Cc: Willem de Bruijn <willemb@google.com>
> Cc: davem@davemloft.net
> Cc: ppenkov@stanford.edu
Why is this optional? It adds two code paths both of which need
to be tested.
^ permalink raw reply
* Re: [RFC net-next] net: sch_clsact: add support for global per-netns classifier mode
From: Daniel Borkmann @ 2017-09-05 22:45 UTC (permalink / raw)
To: Roopa Prabhu, Cong Wang
Cc: Nikolay Aleksandrov, Linux Kernel Network Developers, David Ahern,
Jiri Pirko, Jamal Hadi Salim
In-Reply-To: <CAJieiUj_-o-Cg2AzN5b0vrdnBD69ryMmpu4fcC64-MHEmQ-u4g@mail.gmail.com>
On 09/06/2017 12:01 AM, Roopa Prabhu wrote:
> On Tue, Sep 5, 2017 at 11:18 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>> On Tue, Sep 5, 2017 at 5:48 AM, Nikolay Aleksandrov
>> <nikolay@cumulusnetworks.com> wrote:
>>> Hi all,
>>> This RFC adds a new mode for clsact which designates a device's egress
>>> classifier as global per netns. The packets that are not classified for
>>> a particular device will be classified using the global classifier.
>>> We have needed a global classifier for some time now for various
>>> purposes and setting the single bridge or loopback/vrf device as the
Can you elaborate a bit more on the ... "we have needed a global
classifier for some time now for various purposes".
>>> global classifier device is acceptable for us. Doing it this way avoids
>>> the act/cls device and queue dependencies.
>>>
>>> This is strictly an RFC patch just to show the intent, if we agree on
>>> the details the proposed patch will have support for both ingress and
>>> egress, and will be using a static key to avoid the fast path test when no
>>> global classifier has been configured.
>>>
>>> Example (need a modified tc that adds TCA_OPTIONS when using q_clsact):
>>> $ tc qdisc add dev lo clsact global
>>> $ tc filter add dev lo egress protocol ip u32 match ip dst 4.3.2.1/32 action drop
>>>
>>> the last filter will be global for all devices that don't have a
>>> specific egress_cl_list (i.e. have clsact configured).
>>
>> Sorry this is too ugly.
+1
>> netdevice is still implied in your command line even if you treat it
>> as global. It is essentially hard to bypass netdevice layer since
>> netdevice is the core of L2 and also where everything begins.
>>
>> Maybe the best we can do here is make tc filters standalone
>> as tc actions so that filters can exist before qdisc's and netdevices.
>> But this probably requires significant works to make it working
>> with both existing non-standalone and bindings standalones
>> with qdisc's.
>
> yes, like Nikolay says we have been discussing this as well. Nikolay's
> patch is a cleaver and most importantly non-invasive
> way today given the anchor point for tc rules is a netdev. we have
> also considered a separate implicit tc anchor device.
Seems ugly just as well. :( Hmm, why not just having the two list
pointers (ingress, egress list) in the netns struct and when
something configures them to be effectively non-zero, then devices
in that netns could automatically get a clsact and inherit the
lists from there such that sch_handle_ingress() and sch_handle_egress()
require exactly zero changes in fast-path. You could then go and
say that either you would make changes to clsact for individual
devices immutable when they use the 'shared' list pointers, or then
duplicate the configs when being altered from the global one. Would
push the complexity to control path only at least. Just a brief
thought.
^ permalink raw reply
* [PATCH net-next RFC 2/2] tun: enable napi_gro_frags() for TUN/TAP driver
From: Petar Penkov @ 2017-09-05 22:35 UTC (permalink / raw)
To: netdev
Cc: Petar Penkov, Eric Dumazet, Mahesh Bandewar, Willem de Bruijn,
davem, ppenkov
In-Reply-To: <20170905223551.27925-1-ppenkov@google.com>
Add a TUN/TAP receive mode that exercises the napi_gro_frags()
interface. This mode is available only in TAP mode, as the interface
expects packets with Ethernet headers.
Furthermore, packets follow the layout of the iovec_iter that was
received. The first iovec is the linear data, and every one after the
first is a fragment. If there are more fragments than the max number,
drop the packet. Additionally, invoke eth_get_headlen() to exercise flow
dissector code and to verify that the header resides in the linear data.
The napi_gro_frags() mode requires setting the IFF_NAPI_FRAGS option.
This is imposed because this mode is intended for testing via tools like
syzkaller and packetdrill, and the increased flexibility it provides can
introduce security vulnerabilities.
Signed-off-by: Petar Penkov <ppenkov@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Mahesh Bandewar <maheshb@google.com>
Cc: Willem de Bruijn <willemb@google.com>
Cc: davem@davemloft.net
Cc: ppenkov@stanford.edu
---
drivers/net/tun.c | 135 ++++++++++++++++++++++++++++++++++++++++++--
include/uapi/linux/if_tun.h | 1 +
2 files changed, 130 insertions(+), 6 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index d5c824e3ec42..2ba9809ab6cd 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -75,6 +75,7 @@
#include <linux/skb_array.h>
#include <linux/bpf.h>
#include <linux/bpf_trace.h>
+#include <linux/mutex.h>
#include <linux/uaccess.h>
@@ -120,8 +121,15 @@ do { \
#define TUN_VNET_LE 0x80000000
#define TUN_VNET_BE 0x40000000
+#if IS_ENABLED(CONFIG_TUN_NAPI)
+#define TUN_FEATURES_EXTRA IFF_NAPI_FRAGS
+#else
+#define TUN_FEATURES_EXTRA 0
+#endif
+
#define TUN_FEATURES (IFF_NO_PI | IFF_ONE_QUEUE | IFF_VNET_HDR | \
- IFF_MULTI_QUEUE)
+ IFF_MULTI_QUEUE | TUN_FEATURES_EXTRA)
+
#define GOODCOPY_LEN 128
#define FLT_EXACT_COUNT 8
@@ -173,6 +181,7 @@ struct tun_file {
unsigned int ifindex;
};
struct napi_struct napi;
+ struct mutex napi_mutex; /* Protects access to the above napi */
struct list_head next;
struct tun_struct *detached;
struct skb_array tx_array;
@@ -276,6 +285,7 @@ static void tun_napi_init(struct tun_struct *tun, struct tun_file *tfile)
netif_napi_add(tun->dev, &tfile->napi, tun_napi_poll,
NAPI_POLL_WEIGHT);
napi_enable(&tfile->napi);
+ mutex_init(&tfile->napi_mutex);
}
}
@@ -291,6 +301,11 @@ static void tun_napi_del(struct tun_file *tfile)
netif_napi_del(&tfile->napi);
}
+static bool tun_napi_frags_enabled(const struct tun_struct *tun)
+{
+ return READ_ONCE(tun->flags) & IFF_NAPI_FRAGS;
+}
+
#ifdef CONFIG_TUN_VNET_CROSS_LE
static inline bool tun_legacy_is_little_endian(struct tun_struct *tun)
{
@@ -1034,7 +1049,8 @@ static void tun_poll_controller(struct net_device *dev)
* supports polling, which enables bridge devices in virt setups to
* still use netconsole
* If NAPI is enabled, however, we need to schedule polling for all
- * queues.
+ * queues unless we are using napi_gro_frags(), which we call in
+ * process context and not in NAPI context.
*/
if (IS_ENABLED(CONFIG_TUN_NAPI)) {
@@ -1042,6 +1058,9 @@ static void tun_poll_controller(struct net_device *dev)
struct tun_file *tfile;
int i;
+ if (tun_napi_frags_enabled(tun))
+ return;
+
rcu_read_lock();
for (i = 0; i < tun->numqueues; i++) {
tfile = rcu_dereference(tun->tfiles[i]);
@@ -1264,6 +1283,64 @@ static unsigned int tun_chr_poll(struct file *file, poll_table *wait)
return mask;
}
+static struct sk_buff *tun_napi_alloc_frags(struct tun_file *tfile,
+ size_t len,
+ const struct iov_iter *it)
+{
+ struct sk_buff *skb;
+ size_t linear;
+ int err;
+ int i;
+
+ if (it->nr_segs > MAX_SKB_FRAGS + 1)
+ return ERR_PTR(-ENOMEM);
+
+ local_bh_disable();
+ skb = napi_get_frags(&tfile->napi);
+ local_bh_enable();
+ if (!skb)
+ return ERR_PTR(-ENOMEM);
+
+ linear = iov_iter_single_seg_count(it);
+ err = __skb_grow(skb, linear);
+ if (err)
+ goto free;
+
+ skb->len = len;
+ skb->data_len = len - linear;
+ skb->truesize += skb->data_len;
+
+ for (i = 1; i < it->nr_segs; i++) {
+ size_t fragsz = it->iov[i].iov_len;
+ unsigned long offset;
+ struct page *page;
+ void *data;
+
+ if (fragsz == 0 || fragsz > PAGE_SIZE) {
+ err = -EINVAL;
+ goto free;
+ }
+
+ local_bh_disable();
+ data = napi_alloc_frag(fragsz);
+ local_bh_enable();
+ if (!data) {
+ err = -ENOMEM;
+ goto free;
+ }
+
+ page = virt_to_page(data);
+ offset = offset_in_page(data);
+ skb_fill_page_desc(skb, i - 1, page, offset, fragsz);
+ }
+
+ return skb;
+free:
+ /* frees skb and all frags allocated with napi_alloc_frag() */
+ napi_free_frags(&tfile->napi);
+ return ERR_PTR(err);
+}
+
/* prepad is the amount to reserve at front. len is length after that.
* linear is a hint as to how much to copy (usually headers). */
static struct sk_buff *tun_alloc_skb(struct tun_file *tfile,
@@ -1466,6 +1543,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
int err;
u32 rxhash;
int generic_xdp = 1;
+ bool frags = tun_napi_frags_enabled(tun);
if (!(tun->dev->flags & IFF_UP))
return -EIO;
@@ -1523,7 +1601,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
zerocopy = true;
}
- if (tun_can_build_skb(tun, tfile, len, noblock, zerocopy)) {
+ if (!frags && tun_can_build_skb(tun, tfile, len, noblock, zerocopy)) {
skb = tun_build_skb(tun, tfile, from, &gso, len, &generic_xdp);
if (IS_ERR(skb)) {
this_cpu_inc(tun->pcpu_stats->rx_dropped);
@@ -1540,10 +1618,24 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
linear = tun16_to_cpu(tun, gso.hdr_len);
}
- skb = tun_alloc_skb(tfile, align, copylen, linear, noblock);
+ if (frags) {
+ mutex_lock(&tfile->napi_mutex);
+ skb = tun_napi_alloc_frags(tfile, copylen, from);
+ /* tun_napi_alloc_frags() enforces a layout for the skb.
+ * If zerocopy is enabled, then this layout will be
+ * overwritten by zerocopy_sg_from_iter().
+ */
+ zerocopy = false;
+ } else {
+ skb = tun_alloc_skb(tfile, align, copylen, linear,
+ noblock);
+ }
+
if (IS_ERR(skb)) {
if (PTR_ERR(skb) != -EAGAIN)
this_cpu_inc(tun->pcpu_stats->rx_dropped);
+ if (frags)
+ mutex_unlock(&tfile->napi_mutex);
return PTR_ERR(skb);
}
@@ -1555,6 +1647,11 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
if (err) {
this_cpu_inc(tun->pcpu_stats->rx_dropped);
kfree_skb(skb);
+ if (frags) {
+ tfile->napi.skb = NULL;
+ mutex_unlock(&tfile->napi_mutex);
+ }
+
return -EFAULT;
}
}
@@ -1562,6 +1659,11 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
if (virtio_net_hdr_to_skb(skb, &gso, tun_is_little_endian(tun))) {
this_cpu_inc(tun->pcpu_stats->rx_frame_errors);
kfree_skb(skb);
+ if (frags) {
+ tfile->napi.skb = NULL;
+ mutex_unlock(&tfile->napi_mutex);
+ }
+
return -EINVAL;
}
@@ -1587,7 +1689,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
skb->dev = tun->dev;
break;
case IFF_TAP:
- skb->protocol = eth_type_trans(skb, tun->dev);
+ if (!frags)
+ skb->protocol = eth_type_trans(skb, tun->dev);
break;
}
@@ -1622,7 +1725,23 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
rxhash = __skb_get_hash_symmetric(skb);
- if (IS_ENABLED(CONFIG_TUN_NAPI)) {
+ if (frags) {
+ /* Exercise flow dissector code path. */
+ u32 headlen = eth_get_headlen(skb->data, skb_headlen(skb));
+
+ if (headlen > skb_headlen(skb) || headlen < ETH_HLEN) {
+ this_cpu_inc(tun->pcpu_stats->rx_dropped);
+ napi_free_frags(&tfile->napi);
+ mutex_unlock(&tfile->napi_mutex);
+ WARN_ON(1);
+ return -ENOMEM;
+ }
+
+ local_bh_disable();
+ napi_gro_frags(&tfile->napi);
+ local_bh_enable();
+ mutex_unlock(&tfile->napi_mutex);
+ } else if (IS_ENABLED(CONFIG_TUN_NAPI)) {
struct sk_buff_head *queue = &tfile->sk.sk_write_queue;
int queue_len;
@@ -2168,6 +2287,10 @@ static int tun_set_iff(struct net *net, struct file *file, struct ifreq *ifr)
tun->flags = (tun->flags & ~TUN_FEATURES) |
(ifr->ifr_flags & TUN_FEATURES);
+ if (!IS_ENABLED(CONFIG_TUN_NAPI) ||
+ (tun->flags & TUN_TYPE_MASK) != IFF_TAP)
+ tun->flags = tun->flags & ~IFF_NAPI_FRAGS;
+
/* Make sure persistent devices do not get stuck in
* xoff state.
*/
diff --git a/include/uapi/linux/if_tun.h b/include/uapi/linux/if_tun.h
index 3cb5e1d85ddd..1eb1eb42f151 100644
--- a/include/uapi/linux/if_tun.h
+++ b/include/uapi/linux/if_tun.h
@@ -60,6 +60,7 @@
/* TUNSETIFF ifr flags */
#define IFF_TUN 0x0001
#define IFF_TAP 0x0002
+#define IFF_NAPI_FRAGS 0x0010
#define IFF_NO_PI 0x1000
/* This flag has no real effect */
#define IFF_ONE_QUEUE 0x2000
--
2.14.1.581.gf28d330327-goog
^ permalink raw reply related
* [PATCH net-next RFC 1/2] tun: enable NAPI for TUN/TAP driver
From: Petar Penkov @ 2017-09-05 22:35 UTC (permalink / raw)
To: netdev
Cc: Petar Penkov, Eric Dumazet, Mahesh Bandewar, Willem de Bruijn,
davem, ppenkov
In-Reply-To: <20170905223551.27925-1-ppenkov@google.com>
Changes TUN driver to use napi_gro_receive() upon receiving packets
rather than netif_rx_ni(). Adds flag CONFIG_TUN_NAPI that enables
these changes and operation is not affected if the flag is disabled.
SKBs are constructed upon packet arrival and are queued to be
processed later.
The new path was evaluated with a benchmark with the following setup:
Open two tap devices and a receiver thread that reads in a loop for
each device. Start one sender thread and pin all threads to different
CPUs. Send 1M minimum UDP packets to each device and measure sending
time for each of the sending methods:
napi_gro_receive(): 4.90s
netif_rx_ni(): 4.90s
netif_receive_skb(): 7.20s
Signed-off-by: Petar Penkov <ppenkov@google.com>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Mahesh Bandewar <maheshb@google.com>
Cc: Willem de Bruijn <willemb@google.com>
Cc: davem@davemloft.net
Cc: ppenkov@stanford.edu
---
drivers/net/Kconfig | 8 ++++
drivers/net/tun.c | 120 +++++++++++++++++++++++++++++++++++++++++++++++-----
2 files changed, 118 insertions(+), 10 deletions(-)
diff --git a/drivers/net/Kconfig b/drivers/net/Kconfig
index 83a1616903f8..34850d71ddd1 100644
--- a/drivers/net/Kconfig
+++ b/drivers/net/Kconfig
@@ -307,6 +307,14 @@ config TAP
This option is selected by any driver implementing tap user space
interface for a virtual interface to re-use core tap functionality.
+config TUN_NAPI
+ bool "NAPI support on tx path for TUN/TAP driver"
+ default n
+ depends on TUN
+ ---help---
+ This option allows the TUN/TAP driver to use NAPI to pass packets to
+ the kernel when receiving packets from user space via write()/send().
+
config TUN_VNET_CROSS_LE
bool "Support for cross-endian vnet headers on little-endian kernels"
default n
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 06e8f0bb2dab..d5c824e3ec42 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -172,6 +172,7 @@ struct tun_file {
u16 queue_index;
unsigned int ifindex;
};
+ struct napi_struct napi;
struct list_head next;
struct tun_struct *detached;
struct skb_array tx_array;
@@ -229,6 +230,67 @@ struct tun_struct {
struct bpf_prog __rcu *xdp_prog;
};
+static int tun_napi_receive(struct napi_struct *napi, int budget)
+{
+ struct tun_file *tfile = container_of(napi, struct tun_file, napi);
+ struct sk_buff_head *queue = &tfile->sk.sk_write_queue;
+ struct sk_buff_head process_queue;
+ struct sk_buff *skb;
+ int received = 0;
+
+ __skb_queue_head_init(&process_queue);
+
+ spin_lock(&queue->lock);
+ skb_queue_splice_tail_init(queue, &process_queue);
+ spin_unlock(&queue->lock);
+
+ while (received < budget && (skb = __skb_dequeue(&process_queue))) {
+ napi_gro_receive(napi, skb);
+ ++received;
+ }
+
+ if (!skb_queue_empty(&process_queue)) {
+ spin_lock(&queue->lock);
+ skb_queue_splice(&process_queue, queue);
+ spin_unlock(&queue->lock);
+ }
+
+ return received;
+}
+
+static int tun_napi_poll(struct napi_struct *napi, int budget)
+{
+ unsigned int received;
+
+ received = tun_napi_receive(napi, budget);
+
+ if (received < budget)
+ napi_complete_done(napi, received);
+
+ return received;
+}
+
+static void tun_napi_init(struct tun_struct *tun, struct tun_file *tfile)
+{
+ if (IS_ENABLED(CONFIG_TUN_NAPI)) {
+ netif_napi_add(tun->dev, &tfile->napi, tun_napi_poll,
+ NAPI_POLL_WEIGHT);
+ napi_enable(&tfile->napi);
+ }
+}
+
+static void tun_napi_disable(struct tun_file *tfile)
+{
+ if (IS_ENABLED(CONFIG_TUN_NAPI))
+ napi_disable(&tfile->napi);
+}
+
+static void tun_napi_del(struct tun_file *tfile)
+{
+ if (IS_ENABLED(CONFIG_TUN_NAPI))
+ netif_napi_del(&tfile->napi);
+}
+
#ifdef CONFIG_TUN_VNET_CROSS_LE
static inline bool tun_legacy_is_little_endian(struct tun_struct *tun)
{
@@ -541,6 +603,11 @@ static void __tun_detach(struct tun_file *tfile, bool clean)
tun = rtnl_dereference(tfile->tun);
+ if (tun && clean) {
+ tun_napi_disable(tfile);
+ tun_napi_del(tfile);
+ }
+
if (tun && !tfile->detached) {
u16 index = tfile->queue_index;
BUG_ON(index >= tun->numqueues);
@@ -598,6 +665,7 @@ static void tun_detach_all(struct net_device *dev)
for (i = 0; i < n; i++) {
tfile = rtnl_dereference(tun->tfiles[i]);
BUG_ON(!tfile);
+ tun_napi_disable(tfile);
tfile->socket.sk->sk_shutdown = RCV_SHUTDOWN;
tfile->socket.sk->sk_data_ready(tfile->socket.sk);
RCU_INIT_POINTER(tfile->tun, NULL);
@@ -613,6 +681,7 @@ static void tun_detach_all(struct net_device *dev)
synchronize_net();
for (i = 0; i < n; i++) {
tfile = rtnl_dereference(tun->tfiles[i]);
+ tun_napi_del(tfile);
/* Drop read queue */
tun_queue_purge(tfile);
sock_put(&tfile->sk);
@@ -677,10 +746,12 @@ static int tun_attach(struct tun_struct *tun, struct file *file, bool skip_filte
rcu_assign_pointer(tun->tfiles[tun->numqueues], tfile);
tun->numqueues++;
- if (tfile->detached)
+ if (tfile->detached) {
tun_enable_queue(tfile);
- else
+ } else {
sock_hold(&tfile->sk);
+ tun_napi_init(tun, tfile);
+ }
tun_set_real_num_queues(tun);
@@ -956,13 +1027,28 @@ static void tun_poll_controller(struct net_device *dev)
* Tun only receives frames when:
* 1) the char device endpoint gets data from user space
* 2) the tun socket gets a sendmsg call from user space
- * Since both of those are synchronous operations, we are guaranteed
- * never to have pending data when we poll for it
- * so there is nothing to do here but return.
+ * If NAPI is not enabled, since both of those are synchronous
+ * operations, we are guaranteed never to have pending data when we poll
+ * for it so there is nothing to do here but return.
* We need this though so netpoll recognizes us as an interface that
* supports polling, which enables bridge devices in virt setups to
* still use netconsole
+ * If NAPI is enabled, however, we need to schedule polling for all
+ * queues.
*/
+
+ if (IS_ENABLED(CONFIG_TUN_NAPI)) {
+ struct tun_struct *tun = netdev_priv(dev);
+ struct tun_file *tfile;
+ int i;
+
+ rcu_read_lock();
+ for (i = 0; i < tun->numqueues; i++) {
+ tfile = rcu_dereference(tun->tfiles[i]);
+ napi_schedule(&tfile->napi);
+ }
+ rcu_read_unlock();
+ }
return;
}
#endif
@@ -1535,11 +1621,25 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
}
rxhash = __skb_get_hash_symmetric(skb);
-#ifndef CONFIG_4KSTACKS
- tun_rx_batched(tun, tfile, skb, more);
-#else
- netif_rx_ni(skb);
-#endif
+
+ if (IS_ENABLED(CONFIG_TUN_NAPI)) {
+ struct sk_buff_head *queue = &tfile->sk.sk_write_queue;
+ int queue_len;
+
+ spin_lock_bh(&queue->lock);
+ __skb_queue_tail(queue, skb);
+ queue_len = skb_queue_len(queue);
+ spin_unlock(&queue->lock);
+
+ if (!more || queue_len > NAPI_POLL_WEIGHT)
+ napi_schedule(&tfile->napi);
+
+ local_bh_enable();
+ } else if (!IS_ENABLED(CONFIG_4KSTACKS)) {
+ tun_rx_batched(tun, tfile, skb, more);
+ } else {
+ netif_rx_ni(skb);
+ }
stats = get_cpu_ptr(tun->pcpu_stats);
u64_stats_update_begin(&stats->syncp);
--
2.14.1.581.gf28d330327-goog
^ permalink raw reply related
* [PATCH net-next RFC 0/2] Improve code coverage of syzkaller
From: Petar Penkov @ 2017-09-05 22:35 UTC (permalink / raw)
To: netdev; +Cc: Petar Penkov
This patch series is intended to improve code coverage of syzkaller on
the early receive path, specifically including flow dissector, GRO,
and GRO with frags parts of the networking stack. Syzkaller exercises
the stack through the TUN driver and this is therefore where changes
reside. Current coverage through netif_receive_skb() is limited as it
does not touch on any of the aforementioned code paths. Furthermore,
for full coverage, it is necessary to have more flexibility over the
linear and non-linear data of the skbs.
The following patches address this by providing the user(syzkaller)
with the ability to send via napi_gro_receive() and napi_gro_frags().
Additionally, syzkaller can specify how many fragments there are and
how much data per fragment there is. This is done by exploiting the
convenient structure of iovecs. Finally, this patch series adds
support for exercising the flow dissector during fuzzing.
The code path including napi_gro_receive() can be enabled via the
CONFIG_TUN_NAPI compile-time flag, and can be used by users other than
syzkaller. The remainder of the changes in this patch series give the
user significantly more control over packets entering the kernel. To
avoid potential security vulnerabilities, hide the ability to send
custom skbs and the flow dissector code paths behind a run-time flag
IFF_NAPI_FRAGS that is advertised and accepted only if CONFIG_TUN_NAPI
is enabled.
The patch series will be followed with changes to packetdrill, where
these additions to the TUN driver are exercised and demonstrated.
This will give the ability to write regression tests for specific
parts of the early networking stack.
Patch 1/ Add NAPI struct per receive queue, enable NAPI, and use
napi_gro_receive()
Patch 2/ Use NAPI skb and napi_gro_frags(), exercise flow dissector,
and allow custom skbs.
Petar Penkov (2):
tun: enable NAPI for TUN/TAP driver
tun: enable napi_gro_frags() for TUN/TAP driver
drivers/net/Kconfig | 8 ++
drivers/net/tun.c | 251 +++++++++++++++++++++++++++++++++++++++++---
include/uapi/linux/if_tun.h | 1 +
3 files changed, 246 insertions(+), 14 deletions(-)
--
2.14.1.581.gf28d330327-goog
^ permalink raw reply
* Re: [PATCH net-next] bpf: fix numa_node validation
From: Martin KaFai Lau @ 2017-09-05 22:30 UTC (permalink / raw)
To: Eric Dumazet; +Cc: David Miller, netdev, Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <1504590062.15310.36.camel@edumazet-glaptop3.roam.corp.google.com>
On Mon, Sep 04, 2017 at 10:41:02PM -0700, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> syzkaller reported crashes in bpf map creation or map update [1]
>
> Problem is that nr_node_ids is a signed integer,
> NUMA_NO_NODE is also an integer, so it is very tempting
> to declare numa_node as a signed integer.
>
> This means the typical test to validate a user provided value :
>
> if (numa_node != NUMA_NO_NODE &&
> (numa_node >= nr_node_ids ||
> !node_online(numa_node)))
>
> must be written :
>
> if (numa_node != NUMA_NO_NODE &&
> ((unsigned int)numa_node >= nr_node_ids ||
Thanks for fixing it.
> !node_online(numa_node)))
>
>
> [1]
> kernel BUG at mm/slab.c:3256!
> invalid opcode: 0000 [#1] SMP KASAN
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Modules linked in:
> CPU: 0 PID: 2946 Comm: syzkaller916108 Not tainted 4.13.0-rc7+ #35
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> task: ffff8801d2bc60c0 task.stack: ffff8801c0c90000
> RIP: 0010:____cache_alloc_node+0x1d4/0x1e0 mm/slab.c:3292
> RSP: 0018:ffff8801c0c97638 EFLAGS: 00010096
> RAX: ffffffffffff8b7b RBX: 0000000001080220 RCX: 0000000000000000
> RDX: 00000000ffff8b7b RSI: 0000000001080220 RDI: ffff8801dac00040
> RBP: ffff8801c0c976c0 R08: 0000000000000000 R09: 0000000000000000
> R10: ffff8801c0c97620 R11: 0000000000000001 R12: ffff8801dac00040
> R13: ffff8801dac00040 R14: 0000000000000000 R15: 00000000ffff8b7b
> FS: 0000000002119940(0000) GS:ffff8801db200000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000020001fec CR3: 00000001d2980000 CR4: 00000000001406f0
> Call Trace:
> __do_kmalloc_node mm/slab.c:3688 [inline]
> __kmalloc_node+0x33/0x70 mm/slab.c:3696
> kmalloc_node include/linux/slab.h:535 [inline]
> alloc_htab_elem+0x2a8/0x480 kernel/bpf/hashtab.c:740
> htab_map_update_elem+0x740/0xb80 kernel/bpf/hashtab.c:820
> map_update_elem kernel/bpf/syscall.c:587 [inline]
> SYSC_bpf kernel/bpf/syscall.c:1468 [inline]
> SyS_bpf+0x20c5/0x4c40 kernel/bpf/syscall.c:1443
> entry_SYSCALL_64_fastpath+0x1f/0xbe
> RIP: 0033:0x440409
> RSP: 002b:00007ffd1f1792b8 EFLAGS: 00000246 ORIG_RAX: 0000000000000141
> RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 0000000000440409
> RDX: 0000000000000020 RSI: 0000000020006000 RDI: 0000000000000002
> RBP: 0000000000000086 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000401d70
> R13: 0000000000401e00 R14: 0000000000000000 R15: 0000000000000000
> Code: 83 c2 01 89 50 18 4c 03 70 08 e8 38 f4 ff ff 4d 85 f6 0f 85 3e ff ff ff 44 89 fe 4c 89 ef e8 94 fb ff ff 49 89 c6 e9 2b ff ff ff <0f> 0b 0f 0b 0f 0b 66 0f 1f 44 00 00 55 48 89 e5 41 57 41 56 41
> RIP: ____cache_alloc_node+0x1d4/0x1e0 mm/slab.c:3292 RSP: ffff8801c0c97638
> ---[ end trace d745f355da2e33ce ]---
> Kernel panic - not syncing: Fatal exception
>
> Fixes: 96eabe7a40aa ("bpf: Allow selecting numa node during map creation")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Martin KaFai Lau <kafai@fb.com>
> Cc: Alexei Starovoitov <ast@fb.com>
> Cc: Daniel Borkmann <daniel@iogearbox.net>
> ---
> kernel/bpf/syscall.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 021a05d9d80095303bdfed51ee85bd9067582774..70ad8e220343c7825c8e331f19c1f65c78fdb796 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -323,7 +323,8 @@ static int map_create(union bpf_attr *attr)
> return -EINVAL;
>
> if (numa_node != NUMA_NO_NODE &&
> - (numa_node >= nr_node_ids || !node_online(numa_node)))
> + ((unsigned int)numa_node >= nr_node_ids ||
> + !node_online(numa_node)))
> return -EINVAL;
>
> /* find map type and init map: hashtable vs rbtree vs bloom vs ... */
>
>
^ permalink raw reply
* Re: [RFC net-next] net: sch_clsact: add support for global per-netns classifier mode
From: Jamal Hadi Salim @ 2017-09-05 22:25 UTC (permalink / raw)
To: Roopa Prabhu, Cong Wang
Cc: Nikolay Aleksandrov, Linux Kernel Network Developers, David Ahern,
Jiri Pirko
In-Reply-To: <CAJieiUj_-o-Cg2AzN5b0vrdnBD69ryMmpu4fcC64-MHEmQ-u4g@mail.gmail.com>
On 17-09-05 06:01 PM, Roopa Prabhu wrote:
>
> yes, like Nikolay says we have been discussing this as well. Nikolay's
> patch is a cleaver and most importantly non-invasive
> way today given the anchor point for tc rules is a netdev. we have
> also considered a separate implicit tc anchor device.
> lo seemed like a good fit for all global rules given it is already
> available. And it is not uncommon to hang off global
> network config on the loopback interface.
>
IMO, Jiri has done all the necessary work already with the concept of
blocks. We dont really need the netdev to be the attachment point.
You can add to a block in many locations in the kernel by
constructing the proper "coordinates" in the tcmsg.
i.e this:
tcmsg {
unsigned char tcm_family;
unsigned char tcm__pad1;
unsigned short tcm__pad2;
int tcm_ifindex;
__u32 tcm_handle;
__u32 tcm_parent;
}
If you were to set tcm_ifindex to -1 (since that is not a legit
ifindex) then all we need to do is define a parent for a
different location. Current locations tied to netdevs are:
-----
#define TC_H_ROOT (0xFFFFFFFFU)
#define TC_H_INGRESS (0xFFFFFFF1U)
#define TC_H_CLSACT TC_H_INGRESS
#define TC_H_MIN_INGRESS 0xFFF2U
#define TC_H_MIN_EGRESS 0xFFF3U
-----
You should be able to say add a location which maps to a pre-routing
or post-routing etc; and this would work as well...
cheers,
jamal
^ permalink raw reply
* Re: [PATCH 1/3] security: bpf: Add eBPF LSM hooks to security module
From: Chenbo Feng @ 2017-09-05 22:24 UTC (permalink / raw)
To: Stephen Smalley
Cc: Chenbo Feng, linux-security-module, SELinux, netdev,
Alexei Starovoitov, Lorenzo Colitti
In-Reply-To: <1504270223.8240.2.camel@tycho.nsa.gov>
On Fri, Sep 1, 2017 at 5:50 AM, Stephen Smalley <sds@tycho.nsa.gov> wrote:
> On Thu, 2017-08-31 at 13:56 -0700, Chenbo Feng wrote:
>> From: Chenbo Feng <fengc@google.com>
>>
>> Introduce 5 LSM hooks to provide finer granularity controls on eBPF
>> related operations including create eBPF maps, modify and read eBPF
>> maps
>> content and load eBPF programs to the kernel. Hooks use the new
>> security
>> pointer inside the eBPF map struct to store the owner's security
>> information and the different security modules can perform different
>> checks based on the information stored inside the security field.
>>
>> Signed-off-by: Chenbo Feng <fengc@google.com>
>> ---
>> include/linux/lsm_hooks.h | 41
>> +++++++++++++++++++++++++++++++++++++++++
>> include/linux/security.h | 36 ++++++++++++++++++++++++++++++++++++
>> security/security.c | 28 ++++++++++++++++++++++++++++
>> 3 files changed, 105 insertions(+)
>>
>> diff --git a/include/linux/lsm_hooks.h b/include/linux/lsm_hooks.h
>> index ce02f76a6188..3aaf9a08a983 100644
>> --- a/include/linux/lsm_hooks.h
>> +++ b/include/linux/lsm_hooks.h
>> @@ -1353,6 +1353,32 @@
>> * @inode we wish to get the security context of.
>> * @ctx is a pointer in which to place the allocated security
>> context.
>> * @ctxlen points to the place to put the length of @ctx.
>> + *
>> + * Security hooks for using the eBPF maps and programs
>> functionalities through
>> + * eBPF syscalls.
>> + *
>> + * @bpf_map_create:
>> + * Check permissions prior to creating a new bpf map.
>> + * Return 0 if the permission is granted.
>> + *
>> + * @bpf_map_modify:
>> + * Check permission prior to insert, update and delete map
>> content.
>> + * @map pointer to the struct bpf_map that contains map
>> information.
>> + * Return 0 if the permission is granted.
>> + *
>> + * @bpf_map_read:
>> + * Check permission prior to read a bpf map content.
>> + * @map pointer to the struct bpf_map that contains map
>> information.
>> + * Return 0 if the permission is granted.
>> + *
>> + * @bpf_prog_load:
>> + * Check permission prior to load eBPF program.
>> + * Return 0 if the permission is granted.
>> + *
>> + * @bpf_post_create:
>> + * Initialize the bpf object security field inside struct
>> bpf_maps and
>> + * it is used for future security checks.
>> + *
>> */
>> union security_list_options {
>> int (*binder_set_context_mgr)(struct task_struct *mgr);
>> @@ -1685,6 +1711,14 @@ union security_list_options {
>> struct audit_context *actx);
>> void (*audit_rule_free)(void *lsmrule);
>> #endif /* CONFIG_AUDIT */
>> +
>> +#ifdef CONFIG_BPF_SYSCALL
>> + int (*bpf_map_create)(void);
>> + int (*bpf_map_read)(struct bpf_map *map);
>> + int (*bpf_map_modify)(struct bpf_map *map);
>> + int (*bpf_prog_load)(void);
>> + int (*bpf_post_create)(struct bpf_map *map);
>> +#endif /* CONFIG_BPF_SYSCALL */
>> };
>>
>> struct security_hook_heads {
>> @@ -1905,6 +1939,13 @@ struct security_hook_heads {
>> struct list_head audit_rule_match;
>> struct list_head audit_rule_free;
>> #endif /* CONFIG_AUDIT */
>> +#ifdef CONFIG_BPF_SYSCALL
>> + struct list_head bpf_map_create;
>> + struct list_head bpf_map_read;
>> + struct list_head bpf_map_modify;
>> + struct list_head bpf_prog_load;
>> + struct list_head bpf_post_create;
>> +#endif /* CONFIG_BPF_SYSCALL */
>> } __randomize_layout;
>>
>> /*
>> diff --git a/include/linux/security.h b/include/linux/security.h
>> index 458e24bea2d4..0656a4f74d14 100644
>> --- a/include/linux/security.h
>> +++ b/include/linux/security.h
>> @@ -31,6 +31,7 @@
>> #include <linux/string.h>
>> #include <linux/mm.h>
>> #include <linux/fs.h>
>> +#include <linux/bpf.h>
>>
>> struct linux_binprm;
>> struct cred;
>> @@ -1735,6 +1736,41 @@ static inline void securityfs_remove(struct
>> dentry *dentry)
>>
>> #endif
>>
>> +#ifdef CONFIG_BPF_SYSCALL
>> +#ifdef CONFIG_SECURITY
>> +int security_map_create(void);
>> +int security_map_modify(struct bpf_map *map);
>> +int security_map_read(struct bpf_map *map);
>> +int security_prog_load(void);
>> +int security_post_create(struct bpf_map *map);
>> +#else
>> +static inline int security_map_create(void)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline int security_map_read(struct bpf_map *map)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline int security_map_modify(struct bpf_map *map)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline int security_prog_load(void)
>> +{
>> + return 0;
>> +}
>> +
>> +static inline int security_post_create(struct bpf_map *map)
>> +{
>> + return 0;
>> +}
>> +#endif /* CONFIG_SECURITY */
>> +#endif /* CONFIG_BPF_SYSCALL */
>
> These should be named consistently with the ones in lsm_hooks.h and
> should unambiguously indicate that these are hooks for bpf
> objects/operations, i.e. security_bpf_map_create(),
> security_bpf_map_read(), etc.
>
Thanks for pointing out, will fix this.
> Do you need this level of granularity?
>
The cover letter of this patch series described a possible use cases of
these lsm hooks and this level of granularity would be ideal to reach that
goal. We can also implement two hooks such as bpf_obj_create and
bpf_obj_use to restrict the creation and using when get the bpf fd from
kernel. But that will be less powerful and flexible.
> Could you coalesce the map_create() and post_map_create() hooks into
> one hook and just unwind the create in that case?
>
Okay, I will take a look on how to fix this.
> Why do you label bpf maps but not bpf progs? Should we be controlling
> the ability to attach/detach a bpf prog (partly controlled by
> CAP_NET_ADMIN, but also somewhat broad in scope and doesn't allow
> control based on who created the prog)?
>
> Should there be a top-level security_bpf_use() hook and permission
> check that limits ability to use bpf() at all?
>
This could be useful but having additional lsm hooks check when reading
and write to eBPF maps may cause performance issue. Instead maybe we
could have a hook for creating eBPF object and retrieve object fd to restrict
the access.
>> +
>> #ifdef CONFIG_SECURITY
>>
>> static inline char *alloc_secdata(void)
>> diff --git a/security/security.c b/security/security.c
>> index 55b5997e4b72..02272f93a89e 100644
>> --- a/security/security.c
>> +++ b/security/security.c
>> @@ -12,6 +12,7 @@
>> * (at your option) any later version.
>> */
>>
>> +#include <linux/bpf.h>
>> #include <linux/capability.h>
>> #include <linux/dcache.h>
>> #include <linux/module.h>
>> @@ -1708,3 +1709,30 @@ int security_audit_rule_match(u32 secid, u32
>> field, u32 op, void *lsmrule,
>> actx);
>> }
>> #endif /* CONFIG_AUDIT */
>> +
>> +#ifdef CONFIG_BPF_SYSCALL
>> +int security_map_create(void)
>> +{
>> + return call_int_hook(bpf_map_create, 0);
>> +}
>> +
>> +int security_map_modify(struct bpf_map *map)
>> +{
>> + return call_int_hook(bpf_map_modify, 0, map);
>> +}
>> +
>> +int security_map_read(struct bpf_map *map)
>> +{
>> + return call_int_hook(bpf_map_read, 0, map);
>> +}
>> +
>> +int security_prog_load(void)
>> +{
>> + return call_int_hook(bpf_prog_load, 0);
>> +}
>> +
>> +int security_post_create(struct bpf_map *map)
>> +{
>> + return call_int_hook(bpf_post_create, 0, map);
>> +}
>> +#endif /* CONFIG_BPF_SYSCALL */
^ permalink raw reply
* Re: [RFC net-next] net: sch_clsact: add support for global per-netns classifier mode
From: Roopa Prabhu @ 2017-09-05 22:01 UTC (permalink / raw)
To: Cong Wang
Cc: Nikolay Aleksandrov, Linux Kernel Network Developers, David Ahern,
Jiri Pirko, Jamal Hadi Salim
In-Reply-To: <CAM_iQpUYGH4BQ-ZUShbRAy6aJD2fYuM+oc_kWYyQ51n+===CQw@mail.gmail.com>
On Tue, Sep 5, 2017 at 11:18 AM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Tue, Sep 5, 2017 at 5:48 AM, Nikolay Aleksandrov
> <nikolay@cumulusnetworks.com> wrote:
>> Hi all,
>> This RFC adds a new mode for clsact which designates a device's egress
>> classifier as global per netns. The packets that are not classified for
>> a particular device will be classified using the global classifier.
>> We have needed a global classifier for some time now for various
>> purposes and setting the single bridge or loopback/vrf device as the
>> global classifier device is acceptable for us. Doing it this way avoids
>> the act/cls device and queue dependencies.
>>
>> This is strictly an RFC patch just to show the intent, if we agree on
>> the details the proposed patch will have support for both ingress and
>> egress, and will be using a static key to avoid the fast path test when no
>> global classifier has been configured.
>>
>> Example (need a modified tc that adds TCA_OPTIONS when using q_clsact):
>> $ tc qdisc add dev lo clsact global
>> $ tc filter add dev lo egress protocol ip u32 match ip dst 4.3.2.1/32 action drop
>>
>> the last filter will be global for all devices that don't have a
>> specific egress_cl_list (i.e. have clsact configured).
>
> Sorry this is too ugly.
>
> netdevice is still implied in your command line even if you treat it
> as global. It is essentially hard to bypass netdevice layer since
> netdevice is the core of L2 and also where everything begins.
>
> Maybe the best we can do here is make tc filters standalone
> as tc actions so that filters can exist before qdisc's and netdevices.
> But this probably requires significant works to make it working
> with both existing non-standalone and bindings standalones
> with qdisc's.
yes, like Nikolay says we have been discussing this as well. Nikolay's
patch is a cleaver and most importantly non-invasive
way today given the anchor point for tc rules is a netdev. we have
also considered a separate implicit tc anchor device.
lo seemed like a good fit for all global rules given it is already
available. And it is not uncommon to hang off global
network config on the loopback interface.
^ permalink raw reply
* Re: [PATCH 2/3] security: bpf: Add eBPF LSM hooks and security field to eBPF map
From: Chenbo Feng @ 2017-09-05 21:59 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Chenbo Feng, Daniel Borkmann, linux-security-module,
Jeffrey Vander Stoep, netdev, SELinux, Lorenzo Colitti
In-Reply-To: <20170901020520.uifv6b7tvelgxumf@ast-mbp>
On Thu, Aug 31, 2017 at 7:05 PM, Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
> On Thu, Aug 31, 2017 at 01:56:34PM -0700, Chenbo Feng wrote:
>> From: Chenbo Feng <fengc@google.com>
>>
>> Introduce a pointer into struct bpf_map to hold the security information
>> about the map. The actual security struct varies based on the security
>> models implemented. Place the LSM hooks before each of the unrestricted
>> eBPF operations, the map_update_elem and map_delete_elem operations are
>> checked by security_map_modify. The map_lookup_elem and map_get_next_key
>> operations are checked by securtiy_map_read.
>>
>> Signed-off-by: Chenbo Feng <fengc@google.com>
>
> ...
>
>> @@ -410,6 +418,10 @@ static int map_lookup_elem(union bpf_attr *attr)
>> if (IS_ERR(map))
>> return PTR_ERR(map);
>>
>> + err = security_map_read(map);
>> + if (err)
>> + return -EACCES;
>> +
>> key = memdup_user(ukey, map->key_size);
>> if (IS_ERR(key)) {
>> err = PTR_ERR(key);
>> @@ -490,6 +502,10 @@ static int map_update_elem(union bpf_attr *attr)
>> if (IS_ERR(map))
>> return PTR_ERR(map);
>>
>> + err = security_map_modify(map);
>
> I don't feel these extra hooks are really thought through.
> With such hook you'll disallow map_update for given map. That's it.
> The key/values etc won't be used in such security decision.
> In such case you don't need such hooks in update/lookup at all.
> Only in map_creation and object_get calls where FD can be received.
> In other words I suggest to follow standard unix practices:
> Do permissions checks in open() and allow read/write() if FD is valid.
> Same here. Do permission checks in prog_load/map_create/obj_pin/get
> and that will be enough to jail bpf subsystem.
> bpf cmds that need to be fast (like lookup and update) should not
> have security hooks.
>
Thanks for pointing out this. I agree we should only do checks on
creating and passing
the object from one processes to another. And if we can still
distinguish the read/write operation
when obtaining the file fd, that would be great. But that may require
us to add a new mode
field into bpf_map struct and change the attribute struct when doing
the bpf syscall. How do you
think about this approach? Or we can do simple checks for
bpf_obj_create and bpf_obj_use when
creating the object and passing the object respectively but this
solution cannot distinguish map modify and
read.
^ permalink raw reply
* Re: [PATCH 0/3] drivers: net: xgene: Misc bug fixes
From: David Miller @ 2017-09-05 21:58 UTC (permalink / raw)
To: isubramanian; +Cc: netdev, dnelson, patches, linux-arm-kernel
In-Reply-To: <1504635392-19089-1-git-send-email-isubramanian@apm.com>
From: Iyappan Subramanian <isubramanian@apm.com>
Date: Tue, 5 Sep 2017 11:16:29 -0700
> This patch set fixes bugs related to handling the case for ACPI for,
> reading and programming tx/rx delay values.
>
> Signed-off-by: Iyappan Subramanian <isubramanian@apm.com>
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH][next v2] rocker: fix kcalloc parameter order
From: David Miller @ 2017-09-05 21:57 UTC (permalink / raw)
To: zahari.doychev; +Cc: jiri, netdev
In-Reply-To: <20170905194958.3436-1-zahari.doychev@linux.com>
From: Zahari Doychev <zahari.doychev@linux.com>
Date: Tue, 5 Sep 2017 21:49:58 +0200
> The function calls to kcalloc use wrong parameter order and incorrect flags
> values. GFP_KERNEL is used instead of flags now and the order is corrected.
>
> The change was done using the following coccinelle script:
>
> @@
> expression E1,E2;
> type T;
> @@
>
> -kcalloc(E1, E2, sizeof(T))
> +kcalloc(E2, sizeof(T), GFP_KERNEL)
>
> Signed-off-by: Zahari Doychev <zahari.doychev@linux.com>
Applied, thank you.
^ permalink raw reply
* Re: [PATCH net] rds: Fix non-atomic operation on shared flag variable
From: David Miller @ 2017-09-05 21:50 UTC (permalink / raw)
To: Haakon.Bugge-QHcLZuEGTsvQT0dZR+AlfA
Cc: santosh.shilimkar-QHcLZuEGTsvQT0dZR+AlfA,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-rdma-u79uwXL29TY76Z2rM5mHXA,
rds-devel-N0ozoZBvEnrZJqsBc5GL+g,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
knut.omang-QHcLZuEGTsvQT0dZR+AlfA,
wei.lin.guay-QHcLZuEGTsvQT0dZR+AlfA
In-Reply-To: <20170905154201.30198-1-Haakon.Bugge-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
From: Håkon Bugge <Haakon.Bugge-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Date: Tue, 5 Sep 2017 17:42:01 +0200
> The bits in m_flags in struct rds_message are used for a plurality of
> reasons, and from different contexts. To avoid any missing updates to
> m_flags, use the atomic set_bit() instead of the non-atomic equivalent.
>
> Signed-off-by: Håkon Bugge <haakon.bugge-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Knut Omang <knut.omang-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
> Reviewed-by: Wei Lin Guay <wei.lin.guay-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>
Applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH net v2] net: sched: don't use GFP_KERNEL under spin lock
From: David Miller @ 2017-09-05 21:49 UTC (permalink / raw)
To: jakub.kicinski
Cc: netdev, eric.dumazet, jiri, chrism, xiyou.wangcong, jhs,
oss-drivers
In-Reply-To: <20170905153123.13209-1-jakub.kicinski@netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Tue, 5 Sep 2017 08:31:23 -0700
> The new TC IDR code uses GFP_KERNEL under spin lock. Which leads
> to:
...
> Try to preallocate the memory with idr_prealloc(GFP_KERNEL)
> (as suggested by Eric Dumazet), and change the allocation
> flags under spin lock.
>
> Fixes: 65a206c01e8e ("net/sched: Change act_api and act_xxx modules to use IDR")
> Signed-off-by: Jakub Kicinski <jakub.kicinski@netronome.com>
> Reviewed-by: Simon Horman <simon.horman@netronome.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net V2] vhost_net: correctly check tx avail during rx busy polling
From: David Miller @ 2017-09-05 21:48 UTC (permalink / raw)
To: jasowang; +Cc: mst, netdev, kvm, virtualization, linux-kernel
In-Reply-To: <1504574525-4711-1-git-send-email-jasowang@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Tue, 5 Sep 2017 09:22:05 +0800
> We check tx avail through vhost_enable_notify() in the past which is
> wrong since it only checks whether or not guest has filled more
> available buffer since last avail idx synchronization which was just
> done by vhost_vq_avail_empty() before. What we really want is checking
> pending buffers in the avail ring. Fix this by calling
> vhost_vq_avail_empty() instead.
>
> This issue could be noticed by doing netperf TCP_RR benchmark as
> client from guest (but not host). With this fix, TCP_RR from guest to
> localhost restores from 1375.91 trans per sec to 55235.28 trans per
> sec on my laptop (Intel(R) Core(TM) i7-5600U CPU @ 2.60GHz).
>
> Fixes: 030881372460 ("vhost_net: basic polling support")
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Applied and queued up for -stable, thanks.
^ 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