Netdev List
 help / color / mirror / Atom feed
* Re: Potential issues (security and otherwise) with the current cgroup-bpf API
From: Mickaël Salaün @ 2017-01-19  1:01 UTC (permalink / raw)
  To: Andy Lutomirski, Tejun Heo
  Cc: Michal Hocko, Peter Zijlstra, David Ahern, Alexei Starovoitov,
	Andy Lutomirski, Daniel Mack, Kees Cook, Jann Horn,
	David S. Miller, Thomas Graf, Michael Kerrisk, Linux API,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Network Development
In-Reply-To: <CALCETrXv3k+=C=9D5YGypzPk8_f4yo8ShQ3tM2fOo++gJFBM4A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 2265 bytes --]


On 19/01/2017 01:18, Andy Lutomirski wrote:
>>> it explicitly respects the cgroup hierarchy.  It shows up in
>>> /proc/cgroups, and I had no problem mounting a cgroupfs instance with
>>> perf_event enabled.  So I'm not sure what you mean.
>>
>> That all it's doing is providing membership information.
> 
> But it's doing it wrong!  Even perf_event tests for membership in a
> given cgroup *or one of its descendents*.  This code does not.
> 
> I think the moral of the story here is that there are lots of open
> questions and design work to be done and that this feature really
> isn't ready to be stable.  For Landlock, I believe that it really
> needs to be done right and I will put my foot down and NAK any effort
> to have Landlock available in a released kernel without resolving
> these types of issues first.  Does anyone really want Landlock to work
> differently than the net hooks simply because the net hooks were in a
> rush?

About Landlock, there is two use cases:

The first is to allow unprivileged users to tie eBPF programs (rules) to
processes. This is the (final) goal. In this case, a (cgroup) hierarchy
is mandatory, otherwise it would be trivial to defeat any access rule.
This is the same issue with namespaces.

The second use case is to only allow privileged users to tie eBPF
programs to processes. As discussed, this will be the next series,
preceding the unprivileged series. In this privilege case, only root
(global CAP_SYS_ADMIN, no namespaces) may be able to use Landlock. Not
having a hierarchy is not a security issue (only a practical one).

The first/next Landlock series (in February) will focus on process
hierarchies (without cgroup), a la seccomp-bpf. It would be too
confusing to not use an inheritable hierarchy like seccomp does, even in
a privileged-only first approach. The inherited rules should then behave
similarly to the seccomp-bpf filters.

However, the following series focusing on cgroup could keep the current
cgroup-bpf behavior, without hierarchy. I don't like the non-hierarchy
approach very much because it add another (less flexible and more
confusing) way to do things (for Landlock at least), but I'm willing to
do it if needed.

Regards,
 Mickaël


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply

* Re: [PATCH] net: add regs attribute to phy device for user diagnose
From: Andrew Lunn @ 2017-01-19  1:01 UTC (permalink / raw)
  To: YUAN Linyu
  Cc: Zefir Kurtisi, David Miller, f.fainelli@gmail.com, cugyly@163.com,
	netdev@vger.kernel.org
In-Reply-To: <8729016553E3654398EA69218DA29EEF15A5A289@cnshjmbx04>

> I will add two ethtool command in kernel to read and write register in PHY. 

Write access will get NACKed by me. Read only please.

> ethtool can use these command to dump what user want, there is no
> more work to PHY driver.

Please think about how you handle PHYs with pages. This needs to be
part of the API.

     Andrew

^ permalink raw reply

* prb_retire_rx_blk_timer_expired use-after-free
From: Dave Jones @ 2017-01-19  0:22 UTC (permalink / raw)
  To: netdev

RSI looks kinda like slab poison here, so re-using a free'd ptr ?

general protection fault: 0000 [#1] PREEMPT SMP DEBUG_PAGEALLOC
CPU: 0 PID: 0 Comm: swapper/0 Not tainted 4.10.0-rc4-think+ #2 
task: ffffffff81e16500 task.stack: ffffffff81e00000
RIP: 0010:prb_retire_rx_blk_timer_expired+0x42/0x130
RSP: 0018:ffff880507803e30 EFLAGS: 00010246
RAX: ffffffff81e16500 RBX: ffff8804bc751158 RCX: 0000000000000000
RDX: ffff8804fb6e8008 RSI: a56b6b6b6b6b6b6b RDI: 0000000000000001
RBP: ffff880507803e48 R08: 0000000000000000 R09: 0000000000000001
R10: 0000000061f74469 R11: 0000000000000054 R12: ffff8804bc751338
R13: ffff8804bc7516d8 R14: ffffffff818ab6a0 R15: ffff8804bc751158
FS:  0000000000000000(0000) GS:ffff880507800000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00005578f64a0130 CR3: 0000000003e11000 CR4: 00000000001406f0
DR0: 00007f539ba38000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000600
Call Trace:
 <IRQ>
 call_timer_fn+0xd2/0x340
 ? call_timer_fn+0x5/0x340
 ? prb_retire_current_block+0x100/0x100
 run_timer_softirq+0x284/0x650
 ? 0xffffffffa035c077
 ? run_timer_softirq+0x5/0x650
 ? lapic_next_deadline+0x5/0x40
 __do_softirq+0x143/0x431
 irq_exit+0xa5/0xb0
 smp_apic_timer_interrupt+0x3d/0x50
 apic_timer_interrupt+0x8d/0xa0
RIP: 0010:cpuidle_enter_state+0x129/0x360
RSP: 0018:ffffffff81e03db8 EFLAGS: 00000246
  ORIG_RAX: ffffffffffffff10
RAX: 0000000000000000 RBX: ffffe8ffff603cc8 RCX: 000000000000001f
RDX: 20c49ba5e353f7cf RSI: ffffffff81c5e743 RDI: ffffffff81c48102
RBP: ffffffff81e03df8 R08: cccccccccccccccd R09: 0000000000000018
R10: 000000000000022e R11: 0000000000000a2c R12: 0000000000000005
R13: ffffffff81eaf918 R14: 0000000000000005 R15: ffffffff81eaf900
 </IRQ>
 ? cpuidle_enter_state+0x113/0x360
 cpuidle_enter+0x17/0x20
 call_cpuidle+0x23/0x40
 do_idle+0xf6/0x1f0
 cpu_startup_entry+0x71/0x80
 rest_init+0xb8/0xc0
 start_kernel+0x432/0x453
 x86_64_start_reservations+0x2a/0x2c
 x86_64_start_kernel+0x178/0x18b
 start_cpu+0x14/0x14
 ? start_cpu+0x14/0x14
Code: fb 4c 89 e7 e8 b0 f1 01 00 0f b7 8b 2a 05 00 00 48 8b 93 18 05 00 00 80 bb 29 05 00 00 00 0f b6 bb 28 05 00 00 48 8b 34 ca 75 58 <8b> 56 0c 48 89 c8 85 d2 74 1d 8b 93 70 05 00 00 85 d2 74 13 f3 

All code
========
   0:	fb                   	sti    
   1:	4c 89 e7             	mov    %r12,%rdi
   4:	e8 b0 f1 01 00       	callq  0x1f1b9
   9:	0f b7 8b 2a 05 00 00 	movzwl 0x52a(%rbx),%ecx
  10:	48 8b 93 18 05 00 00 	mov    0x518(%rbx),%rdx
  17:	80 bb 29 05 00 00 00 	cmpb   $0x0,0x529(%rbx)
  1e:	0f b6 bb 28 05 00 00 	movzbl 0x528(%rbx),%edi
  25:	48 8b 34 ca          	mov    (%rdx,%rcx,8),%rsi
  29:	75 58                	jne    0x83
  2b:*	8b 56 0c             	mov    0xc(%rsi),%edx     <-- trapping instruction
  2e:	48 89 c8             	mov    %rcx,%rax
  31:	85 d2                	test   %edx,%edx
  33:	74 1d                	je     0x52
  35:	8b 93 70 05 00 00    	mov    0x570(%rbx),%edx
  3b:	85 d2                	test   %edx,%edx
  3d:	74 13                	je     0x52
  3f:	f3                   	repz

Code starting with the faulting instruction
===========================================
   0:	8b 56 0c             	mov    0xc(%rsi),%edx
   3:	48 89 c8             	mov    %rcx,%rax
   6:	85 d2                	test   %edx,%edx
   8:	74 1d                	je     0x27
   a:	8b 93 70 05 00 00    	mov    0x570(%rbx),%edx
  10:	85 d2                	test   %edx,%edx
  12:	74 13                	je     0x27
  14:	f3                   	repz

That code is the BLOCK_NUM_PKTS line here..

 677         spin_lock(&po->sk.sk_receive_queue.lock);
 678 
 679         frozen = prb_queue_frozen(pkc);
 680         pbd = GET_CURR_PBLOCK_DESC_FROM_CORE(pkc);
 681 
 682         if (unlikely(pkc->delete_blk_timer))
 683                 goto out;
 684 
 685         /* We only need to plug the race when the block is partially filled.
 686          * tpacket_rcv:
 687          *              lock(); increment BLOCK_NUM_PKTS; unlock()
 688          *              copy_bits() is in progress ...
 689          *              timer fires on other cpu:
 690          *              we can't retire the current block because copy_bits
 691          *              is in progress.
 692          *
 693          */
 694         if (BLOCK_NUM_PKTS(pbd)) {

^ permalink raw reply

* Re: Potential issues (security and otherwise) with the current cgroup-bpf API
From: Tejun Heo @ 2017-01-19  0:59 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Michal Hocko, Peter Zijlstra, David Ahern, Alexei Starovoitov,
	Andy Lutomirski, Daniel Mack, Mickaël Salaün, Kees Cook,
	Jann Horn, David S. Miller, Thomas Graf, Michael Kerrisk,
	Linux API, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Network Development
In-Reply-To: <CALCETrXv3k+=C=9D5YGypzPk8_f4yo8ShQ3tM2fOo++gJFBM4A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Hello, Andy.

On Wed, Jan 18, 2017 at 04:18:04PM -0800, Andy Lutomirski wrote:
> To map cgroup -> hook, a simple array in each cgroup structure works.
> To map (cgroup, netns) -> hook function, the natural approach would be
> to have some kind of hash, and that would be slower.  This code is
> intended to be *fast*.

Updating these programs isn't a frequent operation.  We can easily
calculate a perfect (or acceptable) hash per-cgroup and rcu swap the
new hashtable.

> I suppose you could have each cgroup structure contain an array where
> each element is (netns, hook function) and just skip the ones that are
> the wrong netns.  Perhaps it would be rare to have many of those.

Yeah, or maybe even that.  It just isn't a fundamentally difficult
problem.

> I think it's currently a broken cgroup feature.  I think it could be
> made into a properly working cgroup feature (which I favor) or a
> properly working net feature.  Can you articulate why you prefer
> having it be a net feature instead of a cgroup feature?  I tend to

I thought that's what I was doing in the previous message.

> favor making it be a working cgroup feature (by giving it a file or
> files in cgroupfs and maybe even a controller) because the result
> would have very obvious semantics.

I'm fine with both directions but one seems far out.

> But maybe this is just a false dichotomy.  Could this feature be a
> per-netns configuration where you can give instructions like "run this
> hook if you're in such-and-such netns and in a given cgroup or one of
> its descendents"?  This would prevent it from being a direct analogue
> to systemd's RestrictAddressFamilies= option, but that may be okay.
> This would side-step issues in the current code where a hook can't
> rely on ifindex meaning what the hook thinks it means.

How is this different from making the current code netns aware?

> Actually, I think I like that approach.  What if it we had a "socket"
> controller and files like "socket.create_socket", "socket.ingress" and
> "socket.egress"?  You could use the bpf() syscall to install a bpf
> filter into "socket.egress" and that filter would filter egress for
> the target cgroup and its descendents on the current netns.  As a
> first pass, the netns issue could be sidestepped by making it only
> work in the root netns (i.e. the bpf() call would fail if you're not
> in the root netns and the hooks wouldn't run if you're not in the root
> netns.)  It *might* even be possible to retrofit in the socket
> controller by saying that the default hierarchy is used if the socket
> controller isn't mounted.

I don't know.  You're throwing out too many ideas too fast and it's
difficult to keep up with what the differences are between those
ideas.  But if we're doing cgroup controllers, shouldn't cgroup ns
support be sufficient?  We can consider the product of cgroup and net
namespaces but that doesn't seem necessary given that people usually
set up these namespaces in conjunction.

> What *isn't* possible to cleanly fix after the fact is the current
> semantics that cgroup hooks override the hooks in their ancestors.
> IMO that is simply broken.  The example you gave (perf_event) is very
> careful not to have this problem.

That's like saying installing an iptables rule for a more specific
target is broken.  As a cgroup controller, it is not an acceptable
behavior given how delegation works.  As something similar to
iptables, it is completely fine.

> > * My impression with bpf is that while delegation is something
> >   theoretically possible it is not something which is gonna take place
> >   in any immediate time frame.  If I'm wrong on this, please feel free
> >   to correct me.
> 
> But the issue isn't *BPF* delegation.  It's cgroup delegation or netns
> creation, both of which exist today.

No, the issue is bpf delegation.  If bpf were fully delegatable in
practical sense, we could just do straight forward cgroup bpf
controller.  Well, we'll have to think about how to chain the programs
which would differ on program type but that shouldn't be too hard.

> > * There are a lot of use cases which can immediately take advantage of
> >   cgroup-aware bpf programs operating as proposed.  The model is
> >   semantically equivalent to iptables (let's address the netns part if
> >   that's an issue) which net people are familiar with.
> 
> That sounds like a great argument for those users to patch their
> kernels to gain experience with this feature.

I don't get why this would particularly point to out-of-tree usage.
Why is that?

> > * It isn't exclusive with adding cgroup bpf controller down the road
> >   if necessary and practical.  Sure, this isn't the perfect situation
> >   but it seems like an acceptable trade-off to me.  What ever is
> >   perfect?
> 
> I think it more or less is exclusive.  I can imagine davem accepting a
> patch to make the sock_cgroup_data think point at a new "socket"
> cgroup type.  I can't imagine him accepting a patch to have sockets
> have more than one pointer to a cgroup, with good reason.

Why would it ever need multiple pointers?  Socket is associated with
one cgroup whether it's this or controller thing.  Membership part
doesn't change.  It can share everything.

> I don't see what's added by having a "bpf" cgroup controller, though
> -- it's just too broad.  If the Landlock stuff goes in, that should
> presumably be either tied to the default hierarchy or use an "lsm"
> controller.  A "bpf" controller for that makes no sense to me.
>
> I can see *huge* value in having some combination of BPF and the
> perf_event controller deciding whether to log perf events, for
> example.  But this would be the perf_event controller, not a
> hypothetical "bpf" controller.

Let's not spiral out.  This isn't relevant to the discussion.  Call it
whatever you want.

> But it's doing it wrong!  Even perf_event tests for membership in a
> given cgroup *or one of its descendents*.  This code does not.
> 
> I think the moral of the story here is that there are lots of open
> questions and design work to be done and that this feature really
> isn't ready to be stable.  For Landlock, I believe that it really
> needs to be done right and I will put my foot down and NAK any effort
> to have Landlock available in a released kernel without resolving
> these types of issues first.  Does anyone really want Landlock to work
> differently than the net hooks simply because the net hooks were in a
> rush?

No idea about landlock but as for the cgroup aware network bpf
programs, let's please try to narrow down the arguments.  Here's one
question.

* If we make the current thing netns aware, how is it different from
  iptables?  Note that at least future-proofing for netns is trivial.
  We can simply skip running the bpf programs for non-root ns for now.

If the answer is "they aren't that different", is the reason that
you're against the current code because you think that it being a part
of cgroup would be more useful?  I'm not necessarily against that
point, I just think that this is a reasonable trade-off given the
circumstances especially given that this doens't block having the
cgroup things in the future.

I'd really appreciate some inputs from bpf folks.  I'm basing my
judgement primarily on the impression that it is very difficult to
make bpf programs practically delegatable.  If that's not the case, we
can easily go the cgroup controller route.

Thanks.

-- 
tejun

^ permalink raw reply

* [PATCH net-next v3] bridge: multicast to unicast
From: Linus Lüssing @ 2017-01-19  0:23 UTC (permalink / raw)
  To: netdev
  Cc: Nikolay Aleksandrov, bridge, linux-wireless, linux-kernel,
	David S . Miller, Felix Fietkau

Implements an optional, per bridge port flag and feature to deliver
multicast packets to any host on the according port via unicast
individually. This is done by copying the packet per host and
changing the multicast destination MAC to a unicast one accordingly.

multicast-to-unicast works on top of the multicast snooping feature of
the bridge. Which means unicast copies are only delivered to hosts which
are interested in it and signalized this via IGMP/MLD reports
previously.

This feature is intended for interface types which have a more reliable
and/or efficient way to deliver unicast packets than broadcast ones
(e.g. wifi).

However, it should only be enabled on interfaces where no IGMPv2/MLDv1
report suppression takes place. This feature is disabled by default.

The initial patch and idea is from Felix Fietkau.

Signed-off-by: Felix Fietkau <nbd@nbd.name>
[linus.luessing@c0d3.blue: various bug + style fixes, commit message]
Signed-off-by: Linus Lüssing <linus.luessing@c0d3.blue>

---

This feature is used and enabled by default in OpenWRT and LEDE for AP
interfaces for more than a year now to allow both a more robust multicast
delivery and multicast at higher rates (e.g. multicast streaming).

In OpenWRT/LEDE the IGMP/MLD report suppression issue is overcome by
the network daemon enabling AP isolation and by that separating all STAs.
Delivery of STA-to-STA IP mulitcast is made possible again by
enabling and utilizing the bridge hairpin mode, which considers the
incoming port as a potential outgoing port, too.

Hairpin-mode is performed after multicast snooping, therefore leading to
only deliver reports to STAs running a multicast router.

Changes in v3:
* fix an uninitialized variable bug introduced in br_multicast_flood()
  in v2, found by kbuild test bot

Changes in v2:
* netlink support (thanks Nik!)
* fixed switching between multicast_to_unicast on/off
  -> even after toggling an already existing entry would
     stale in its mode and would never be replaced
  -> new extra check in br_port_group_equal)
* reduced checks in br_multicast_flood() from two to one
  to address fast-path concerns (thanks Nik!)
* rev-christmas tree ordering (thanks Nik!)
* removed "net_bridge_port_group::unicast", using
  ::flags instead (thanks Nik!)
* BR_MULTICAST_TO_UCAST -> BR_MULTICAST_TO_UNICAST
  (BR_MULTICAST_FLAST_LEAVE has the same length anyway)
* simplified maybe_deliver_addr()
  (no return, no "prev" paramater -> was a NOP anyway)
* added "From: Felix Fietkau [...]"
* added "Signed-off-by: Felix Fietkau [...]"
---
 include/linux/if_bridge.h    |  1 +
 include/uapi/linux/if_link.h |  1 +
 net/bridge/br_forward.c      | 37 ++++++++++++++++-
 net/bridge/br_mdb.c          |  2 +-
 net/bridge/br_multicast.c    | 96 ++++++++++++++++++++++++++++++++------------
 net/bridge/br_netlink.c      |  5 +++
 net/bridge/br_private.h      |  8 ++--
 net/bridge/br_sysfs_if.c     |  2 +
 8 files changed, 121 insertions(+), 31 deletions(-)

diff --git a/include/linux/if_bridge.h b/include/linux/if_bridge.h
index c6587c0..debc9d5 100644
--- a/include/linux/if_bridge.h
+++ b/include/linux/if_bridge.h
@@ -46,6 +46,7 @@ struct br_ip_list {
 #define BR_LEARNING_SYNC	BIT(9)
 #define BR_PROXYARP_WIFI	BIT(10)
 #define BR_MCAST_FLOOD		BIT(11)
+#define BR_MULTICAST_TO_UNICAST	BIT(12)
 
 #define BR_DEFAULT_AGEING_TIME	(300 * HZ)
 
diff --git a/include/uapi/linux/if_link.h b/include/uapi/linux/if_link.h
index 6b13e59..4e59565 100644
--- a/include/uapi/linux/if_link.h
+++ b/include/uapi/linux/if_link.h
@@ -321,6 +321,7 @@ enum {
 	IFLA_BRPORT_MULTICAST_ROUTER,
 	IFLA_BRPORT_PAD,
 	IFLA_BRPORT_MCAST_FLOOD,
+	IFLA_BRPORT_MCAST_TO_UCAST,
 	__IFLA_BRPORT_MAX
 };
 #define IFLA_BRPORT_MAX (__IFLA_BRPORT_MAX - 1)
diff --git a/net/bridge/br_forward.c b/net/bridge/br_forward.c
index 7cb41ae..a6c8a27 100644
--- a/net/bridge/br_forward.c
+++ b/net/bridge/br_forward.c
@@ -174,6 +174,29 @@ static struct net_bridge_port *maybe_deliver(
 	return p;
 }
 
+static void maybe_deliver_addr(struct net_bridge_port *p, struct sk_buff *skb,
+			       const unsigned char *addr, bool local_orig)
+{
+	struct net_device *dev = BR_INPUT_SKB_CB(skb)->brdev;
+	const unsigned char *src = eth_hdr(skb)->h_source;
+
+	if (!should_deliver(p, skb))
+		return;
+
+	/* Even with hairpin, no soliloquies - prevent breaking IPv6 DAD */
+	if (skb->dev == p->dev && ether_addr_equal(src, addr))
+		return;
+
+	skb = skb_copy(skb, GFP_ATOMIC);
+	if (!skb) {
+		dev->stats.tx_dropped++;
+		return;
+	}
+
+	memcpy(eth_hdr(skb)->h_dest, addr, ETH_ALEN);
+	__br_forward(p, skb, local_orig);
+}
+
 /* called under rcu_read_lock */
 void br_flood(struct net_bridge *br, struct sk_buff *skb,
 	      enum br_pkt_type pkt_type, bool local_rcv, bool local_orig)
@@ -241,10 +264,20 @@ void br_multicast_flood(struct net_bridge_mdb_entry *mdst,
 		rport = rp ? hlist_entry(rp, struct net_bridge_port, rlist) :
 			     NULL;
 
-		port = (unsigned long)lport > (unsigned long)rport ?
-		       lport : rport;
+		if ((unsigned long)lport > (unsigned long)rport) {
+			port = lport;
+
+			if (p->flags & MDB_PG_FLAGS_MCAST_TO_UCAST) {
+				maybe_deliver_addr(lport, skb, p->eth_addr,
+						   local_orig);
+				goto delivered;
+			}
+		} else {
+			port = rport;
+		}
 
 		prev = maybe_deliver(prev, port, skb, local_orig);
+delivered:
 		if (IS_ERR(prev))
 			goto out;
 		if (prev == port)
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index 7dbc80d..056e6ac 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -531,7 +531,7 @@ static int br_mdb_add_group(struct net_bridge *br, struct net_bridge_port *port,
 			break;
 	}
 
-	p = br_multicast_new_port_group(port, group, *pp, state);
+	p = br_multicast_new_port_group(port, group, *pp, state, NULL);
 	if (unlikely(!p))
 		return -ENOMEM;
 	rcu_assign_pointer(*pp, p);
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index b30e77e..b127bb7 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -43,12 +43,14 @@ static void br_multicast_add_router(struct net_bridge *br,
 static void br_ip4_multicast_leave_group(struct net_bridge *br,
 					 struct net_bridge_port *port,
 					 __be32 group,
-					 __u16 vid);
+					 __u16 vid,
+					 const unsigned char *src);
+
 #if IS_ENABLED(CONFIG_IPV6)
 static void br_ip6_multicast_leave_group(struct net_bridge *br,
 					 struct net_bridge_port *port,
 					 const struct in6_addr *group,
-					 __u16 vid);
+					 __u16 vid, const unsigned char *src);
 #endif
 unsigned int br_mdb_rehash_seq;
 
@@ -711,7 +713,8 @@ struct net_bridge_port_group *br_multicast_new_port_group(
 			struct net_bridge_port *port,
 			struct br_ip *group,
 			struct net_bridge_port_group __rcu *next,
-			unsigned char flags)
+			unsigned char flags,
+			const unsigned char *src)
 {
 	struct net_bridge_port_group *p;
 
@@ -726,12 +729,39 @@ struct net_bridge_port_group *br_multicast_new_port_group(
 	hlist_add_head(&p->mglist, &port->mglist);
 	setup_timer(&p->timer, br_multicast_port_group_expired,
 		    (unsigned long)p);
+
+	if ((port->flags & BR_MULTICAST_TO_UNICAST) && src) {
+		memcpy(p->eth_addr, src, ETH_ALEN);
+		p->flags |= MDB_PG_FLAGS_MCAST_TO_UCAST;
+	}
+
 	return p;
 }
 
+static bool br_port_group_equal(struct net_bridge_port_group *p,
+				struct net_bridge_port *port,
+				const unsigned char *src)
+{
+	if (p->port != port)
+		return false;
+
+	if (!(p->flags & MDB_PG_FLAGS_MCAST_TO_UCAST) !=
+	    !(port->flags & BR_MULTICAST_TO_UNICAST))
+		return false;
+
+	if (!(p->flags & MDB_PG_FLAGS_MCAST_TO_UCAST))
+		return true;
+
+	if (!src)
+		return false;
+
+	return ether_addr_equal(src, p->eth_addr);
+}
+
 static int br_multicast_add_group(struct net_bridge *br,
 				  struct net_bridge_port *port,
-				  struct br_ip *group)
+				  struct br_ip *group,
+				  const unsigned char *src)
 {
 	struct net_bridge_port_group __rcu **pp;
 	struct net_bridge_port_group *p;
@@ -758,13 +788,13 @@ static int br_multicast_add_group(struct net_bridge *br,
 	for (pp = &mp->ports;
 	     (p = mlock_dereference(*pp, br)) != NULL;
 	     pp = &p->next) {
-		if (p->port == port)
+		if (br_port_group_equal(p, port, src))
 			goto found;
 		if ((unsigned long)p->port < (unsigned long)port)
 			break;
 	}
 
-	p = br_multicast_new_port_group(port, group, *pp, 0);
+	p = br_multicast_new_port_group(port, group, *pp, 0, src);
 	if (unlikely(!p))
 		goto err;
 	rcu_assign_pointer(*pp, p);
@@ -783,7 +813,8 @@ static int br_multicast_add_group(struct net_bridge *br,
 static int br_ip4_multicast_add_group(struct net_bridge *br,
 				      struct net_bridge_port *port,
 				      __be32 group,
-				      __u16 vid)
+				      __u16 vid,
+				      const unsigned char *src)
 {
 	struct br_ip br_group;
 
@@ -794,14 +825,15 @@ static int br_ip4_multicast_add_group(struct net_bridge *br,
 	br_group.proto = htons(ETH_P_IP);
 	br_group.vid = vid;
 
-	return br_multicast_add_group(br, port, &br_group);
+	return br_multicast_add_group(br, port, &br_group, src);
 }
 
 #if IS_ENABLED(CONFIG_IPV6)
 static int br_ip6_multicast_add_group(struct net_bridge *br,
 				      struct net_bridge_port *port,
 				      const struct in6_addr *group,
-				      __u16 vid)
+				      __u16 vid,
+				      const unsigned char *src)
 {
 	struct br_ip br_group;
 
@@ -812,7 +844,7 @@ static int br_ip6_multicast_add_group(struct net_bridge *br,
 	br_group.proto = htons(ETH_P_IPV6);
 	br_group.vid = vid;
 
-	return br_multicast_add_group(br, port, &br_group);
+	return br_multicast_add_group(br, port, &br_group, src);
 }
 #endif
 
@@ -1081,6 +1113,7 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
 					 struct sk_buff *skb,
 					 u16 vid)
 {
+	const unsigned char *src;
 	struct igmpv3_report *ih;
 	struct igmpv3_grec *grec;
 	int i;
@@ -1121,12 +1154,14 @@ static int br_ip4_multicast_igmp3_report(struct net_bridge *br,
 			continue;
 		}
 
+		src = eth_hdr(skb)->h_source;
 		if ((type == IGMPV3_CHANGE_TO_INCLUDE ||
 		     type == IGMPV3_MODE_IS_INCLUDE) &&
 		    ntohs(grec->grec_nsrcs) == 0) {
-			br_ip4_multicast_leave_group(br, port, group, vid);
+			br_ip4_multicast_leave_group(br, port, group, vid, src);
 		} else {
-			err = br_ip4_multicast_add_group(br, port, group, vid);
+			err = br_ip4_multicast_add_group(br, port, group, vid,
+							 src);
 			if (err)
 				break;
 		}
@@ -1141,6 +1176,7 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
 					struct sk_buff *skb,
 					u16 vid)
 {
+	const unsigned char *src = eth_hdr(skb)->h_source;
 	struct icmp6hdr *icmp6h;
 	struct mld2_grec *grec;
 	int i;
@@ -1192,10 +1228,11 @@ static int br_ip6_multicast_mld2_report(struct net_bridge *br,
 		     grec->grec_type == MLD2_MODE_IS_INCLUDE) &&
 		    ntohs(*nsrcs) == 0) {
 			br_ip6_multicast_leave_group(br, port, &grec->grec_mca,
-						     vid);
+						     vid, src);
 		} else {
 			err = br_ip6_multicast_add_group(br, port,
-							 &grec->grec_mca, vid);
+							 &grec->grec_mca, vid,
+							 src);
 			if (err)
 				break;
 		}
@@ -1511,7 +1548,8 @@ br_multicast_leave_group(struct net_bridge *br,
 			 struct net_bridge_port *port,
 			 struct br_ip *group,
 			 struct bridge_mcast_other_query *other_query,
-			 struct bridge_mcast_own_query *own_query)
+			 struct bridge_mcast_own_query *own_query,
+			 const unsigned char *src)
 {
 	struct net_bridge_mdb_htable *mdb;
 	struct net_bridge_mdb_entry *mp;
@@ -1535,7 +1573,7 @@ br_multicast_leave_group(struct net_bridge *br,
 		for (pp = &mp->ports;
 		     (p = mlock_dereference(*pp, br)) != NULL;
 		     pp = &p->next) {
-			if (p->port != port)
+			if (!br_port_group_equal(p, port, src))
 				continue;
 
 			rcu_assign_pointer(*pp, p->next);
@@ -1566,7 +1604,7 @@ br_multicast_leave_group(struct net_bridge *br,
 		for (p = mlock_dereference(mp->ports, br);
 		     p != NULL;
 		     p = mlock_dereference(p->next, br)) {
-			if (p->port != port)
+			if (!br_port_group_equal(p, port, src))
 				continue;
 
 			if (!hlist_unhashed(&p->mglist) &&
@@ -1617,7 +1655,8 @@ br_multicast_leave_group(struct net_bridge *br,
 static void br_ip4_multicast_leave_group(struct net_bridge *br,
 					 struct net_bridge_port *port,
 					 __be32 group,
-					 __u16 vid)
+					 __u16 vid,
+					 const unsigned char *src)
 {
 	struct br_ip br_group;
 	struct bridge_mcast_own_query *own_query;
@@ -1632,14 +1671,15 @@ static void br_ip4_multicast_leave_group(struct net_bridge *br,
 	br_group.vid = vid;
 
 	br_multicast_leave_group(br, port, &br_group, &br->ip4_other_query,
-				 own_query);
+				 own_query, src);
 }
 
 #if IS_ENABLED(CONFIG_IPV6)
 static void br_ip6_multicast_leave_group(struct net_bridge *br,
 					 struct net_bridge_port *port,
 					 const struct in6_addr *group,
-					 __u16 vid)
+					 __u16 vid,
+					 const unsigned char *src)
 {
 	struct br_ip br_group;
 	struct bridge_mcast_own_query *own_query;
@@ -1654,7 +1694,7 @@ static void br_ip6_multicast_leave_group(struct net_bridge *br,
 	br_group.vid = vid;
 
 	br_multicast_leave_group(br, port, &br_group, &br->ip6_other_query,
-				 own_query);
+				 own_query, src);
 }
 #endif
 
@@ -1712,6 +1752,7 @@ static int br_multicast_ipv4_rcv(struct net_bridge *br,
 				 u16 vid)
 {
 	struct sk_buff *skb_trimmed = NULL;
+	const unsigned char *src;
 	struct igmphdr *ih;
 	int err;
 
@@ -1731,13 +1772,14 @@ static int br_multicast_ipv4_rcv(struct net_bridge *br,
 	}
 
 	ih = igmp_hdr(skb);
+	src = eth_hdr(skb)->h_source;
 	BR_INPUT_SKB_CB(skb)->igmp = ih->type;
 
 	switch (ih->type) {
 	case IGMP_HOST_MEMBERSHIP_REPORT:
 	case IGMPV2_HOST_MEMBERSHIP_REPORT:
 		BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
-		err = br_ip4_multicast_add_group(br, port, ih->group, vid);
+		err = br_ip4_multicast_add_group(br, port, ih->group, vid, src);
 		break;
 	case IGMPV3_HOST_MEMBERSHIP_REPORT:
 		err = br_ip4_multicast_igmp3_report(br, port, skb_trimmed, vid);
@@ -1746,7 +1788,7 @@ static int br_multicast_ipv4_rcv(struct net_bridge *br,
 		err = br_ip4_multicast_query(br, port, skb_trimmed, vid);
 		break;
 	case IGMP_HOST_LEAVE_MESSAGE:
-		br_ip4_multicast_leave_group(br, port, ih->group, vid);
+		br_ip4_multicast_leave_group(br, port, ih->group, vid, src);
 		break;
 	}
 
@@ -1766,6 +1808,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
 				 u16 vid)
 {
 	struct sk_buff *skb_trimmed = NULL;
+	const unsigned char *src;
 	struct mld_msg *mld;
 	int err;
 
@@ -1785,8 +1828,10 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
 
 	switch (mld->mld_type) {
 	case ICMPV6_MGM_REPORT:
+		src = eth_hdr(skb)->h_source;
 		BR_INPUT_SKB_CB(skb)->mrouters_only = 1;
-		err = br_ip6_multicast_add_group(br, port, &mld->mld_mca, vid);
+		err = br_ip6_multicast_add_group(br, port, &mld->mld_mca, vid,
+						 src);
 		break;
 	case ICMPV6_MLD2_REPORT:
 		err = br_ip6_multicast_mld2_report(br, port, skb_trimmed, vid);
@@ -1795,7 +1840,8 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
 		err = br_ip6_multicast_query(br, port, skb_trimmed, vid);
 		break;
 	case ICMPV6_MGM_REDUCTION:
-		br_ip6_multicast_leave_group(br, port, &mld->mld_mca, vid);
+		src = eth_hdr(skb)->h_source;
+		br_ip6_multicast_leave_group(br, port, &mld->mld_mca, vid, src);
 		break;
 	}
 
diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
index 71c7453..6c087cd 100644
--- a/net/bridge/br_netlink.c
+++ b/net/bridge/br_netlink.c
@@ -123,6 +123,7 @@ static inline size_t br_port_info_size(void)
 		+ nla_total_size(1)	/* IFLA_BRPORT_GUARD */
 		+ nla_total_size(1)	/* IFLA_BRPORT_PROTECT */
 		+ nla_total_size(1)	/* IFLA_BRPORT_FAST_LEAVE */
+		+ nla_total_size(1)	/* IFLA_BRPORT_MCAST_TO_UCAST */
 		+ nla_total_size(1)	/* IFLA_BRPORT_LEARNING */
 		+ nla_total_size(1)	/* IFLA_BRPORT_UNICAST_FLOOD */
 		+ nla_total_size(1)	/* IFLA_BRPORT_PROXYARP */
@@ -173,6 +174,8 @@ static int br_port_fill_attrs(struct sk_buff *skb,
 		       !!(p->flags & BR_ROOT_BLOCK)) ||
 	    nla_put_u8(skb, IFLA_BRPORT_FAST_LEAVE,
 		       !!(p->flags & BR_MULTICAST_FAST_LEAVE)) ||
+	    nla_put_u8(skb, IFLA_BRPORT_MCAST_TO_UCAST,
+		       !!(p->flags & BR_MULTICAST_TO_UNICAST)) ||
 	    nla_put_u8(skb, IFLA_BRPORT_LEARNING, !!(p->flags & BR_LEARNING)) ||
 	    nla_put_u8(skb, IFLA_BRPORT_UNICAST_FLOOD,
 		       !!(p->flags & BR_FLOOD)) ||
@@ -586,6 +589,7 @@ static const struct nla_policy br_port_policy[IFLA_BRPORT_MAX + 1] = {
 	[IFLA_BRPORT_PROXYARP]	= { .type = NLA_U8 },
 	[IFLA_BRPORT_PROXYARP_WIFI] = { .type = NLA_U8 },
 	[IFLA_BRPORT_MULTICAST_ROUTER] = { .type = NLA_U8 },
+	[IFLA_BRPORT_MCAST_TO_UCAST] = { .type = NLA_U8 },
 };
 
 /* Change the state of the port and notify spanning tree */
@@ -636,6 +640,7 @@ static int br_setport(struct net_bridge_port *p, struct nlattr *tb[])
 	br_set_port_flag(p, tb, IFLA_BRPORT_LEARNING, BR_LEARNING);
 	br_set_port_flag(p, tb, IFLA_BRPORT_UNICAST_FLOOD, BR_FLOOD);
 	br_set_port_flag(p, tb, IFLA_BRPORT_MCAST_FLOOD, BR_MCAST_FLOOD);
+	br_set_port_flag(p, tb, IFLA_BRPORT_MCAST_TO_UCAST, BR_MULTICAST_TO_UNICAST);
 	br_set_port_flag(p, tb, IFLA_BRPORT_PROXYARP, BR_PROXYARP);
 	br_set_port_flag(p, tb, IFLA_BRPORT_PROXYARP_WIFI, BR_PROXYARP_WIFI);
 
diff --git a/net/bridge/br_private.h b/net/bridge/br_private.h
index 8ce621e..f7db79f 100644
--- a/net/bridge/br_private.h
+++ b/net/bridge/br_private.h
@@ -166,8 +166,9 @@ struct net_bridge_fdb_entry
 	struct rcu_head			rcu;
 };
 
-#define MDB_PG_FLAGS_PERMANENT	BIT(0)
-#define MDB_PG_FLAGS_OFFLOAD	BIT(1)
+#define MDB_PG_FLAGS_PERMANENT		BIT(0)
+#define MDB_PG_FLAGS_OFFLOAD		BIT(1)
+#define MDB_PG_FLAGS_MCAST_TO_UCAST	BIT(2)
 
 struct net_bridge_port_group {
 	struct net_bridge_port		*port;
@@ -177,6 +178,7 @@ struct net_bridge_port_group {
 	struct timer_list		timer;
 	struct br_ip			addr;
 	unsigned char			flags;
+	unsigned char			eth_addr[ETH_ALEN];
 };
 
 struct net_bridge_mdb_entry
@@ -599,7 +601,7 @@ void br_multicast_free_pg(struct rcu_head *head);
 struct net_bridge_port_group *
 br_multicast_new_port_group(struct net_bridge_port *port, struct br_ip *group,
 			    struct net_bridge_port_group __rcu *next,
-			    unsigned char flags);
+			    unsigned char flags, const unsigned char *src);
 void br_mdb_init(void);
 void br_mdb_uninit(void);
 void br_mdb_notify(struct net_device *dev, struct net_bridge_port *port,
diff --git a/net/bridge/br_sysfs_if.c b/net/bridge/br_sysfs_if.c
index 8bd5696..05e8946 100644
--- a/net/bridge/br_sysfs_if.c
+++ b/net/bridge/br_sysfs_if.c
@@ -188,6 +188,7 @@ static BRPORT_ATTR(multicast_router, S_IRUGO | S_IWUSR, show_multicast_router,
 		   store_multicast_router);
 
 BRPORT_ATTR_FLAG(multicast_fast_leave, BR_MULTICAST_FAST_LEAVE);
+BRPORT_ATTR_FLAG(multicast_to_unicast, BR_MULTICAST_TO_UNICAST);
 #endif
 
 static const struct brport_attribute *brport_attrs[] = {
@@ -214,6 +215,7 @@ static const struct brport_attribute *brport_attrs[] = {
 #ifdef CONFIG_BRIDGE_IGMP_SNOOPING
 	&brport_attr_multicast_router,
 	&brport_attr_multicast_fast_leave,
+	&brport_attr_multicast_to_unicast,
 #endif
 	&brport_attr_proxyarp,
 	&brport_attr_proxyarp_wifi,
-- 
2.1.4

^ permalink raw reply related

* Re: Potential issues (security and otherwise) with the current cgroup-bpf API
From: Andy Lutomirski @ 2017-01-19  0:18 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Michal Hocko, Peter Zijlstra, David Ahern, Alexei Starovoitov,
	Andy Lutomirski, Daniel Mack, Mickaël Salaün, Kees Cook,
	Jann Horn, David S. Miller, Thomas Graf, Michael Kerrisk,
	Linux API, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Network Development
In-Reply-To: <20170118224120.GG9171-qYNAdHglDFBN0TnZuCh8vA@public.gmane.org>

On Wed, Jan 18, 2017 at 2:41 PM, Tejun Heo <tj-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> Helloo, Andy.
>
> On Mon, Jan 16, 2017 at 09:18:36PM -0800, Andy Lutomirski wrote:
>> Perhaps this is a net feature, though, not a cgroup feature.  This
>> would IMO make a certain amount of sense.  Iptables cgroup matches,
>> for example, logically are an iptables (i.e., net) feature.  The
>
> Yeap.
>
>> problem here is that network configuration (and this explicitly
>> includes iptables) is done on a per-netns level, whereas these new
>> hooks entirely ignore network namespaces.  I've pointed out that
>> trying to enable them in a namespaced world is problematic (e.g.
>> switching to ns_capable() will cause serious problems), and Alexei
>> seems to think that this will never happen.  So I don't think we can
>> really call this a net feature that works the way that other net
>> features work.
>>
>> (Suppose that this actually tried to be netns-enabled.  Then you'd
>> have to map from (netns, cgroup) -> hook, and this would probably be
>> slow and messy.)
>
> I'm not sure how important netns support for these bpf programs
> actually is but I have no objection to making it behave in an
> equivalent manner to iptables where appropriate.  This is kinda
> trivial to do too.  For now, we can simply not run the programs if the
> associated socket belongs to non-root netns.  Later, if necessary, we
> can add per-ns bpf programs.  And I can't think of a reason why
> (netns, cgroup) -> function mapping would be slow and messy.  Why
> would that be?

To map cgroup -> hook, a simple array in each cgroup structure works.
To map (cgroup, netns) -> hook function, the natural approach would be
to have some kind of hash, and that would be slower.  This code is
intended to be *fast*.

I suppose you could have each cgroup structure contain an array where
each element is (netns, hook function) and just skip the ones that are
the wrong netns.  Perhaps it would be rare to have many of those.

>
>> So I think that leaves it being a cgroup feature.  And it definitely
>> does *not* play by the rules of cgroups right now.
>
> Because this in no way is a cgroup feature.  As you wrote above, it is
> something similar to iptables with lacking netns considerations.
> Let's address that and make it a proper network thing.

I think it's currently a broken cgroup feature.  I think it could be
made into a properly working cgroup feature (which I favor) or a
properly working net feature.  Can you articulate why you prefer
having it be a net feature instead of a cgroup feature?  I tend to
favor making it be a working cgroup feature (by giving it a file or
files in cgroupfs and maybe even a controller) because the result
would have very obvious semantics.

But maybe this is just a false dichotomy.  Could this feature be a
per-netns configuration where you can give instructions like "run this
hook if you're in such-and-such netns and in a given cgroup or one of
its descendents"?  This would prevent it from being a direct analogue
to systemd's RestrictAddressFamilies= option, but that may be okay.
This would side-step issues in the current code where a hook can't
rely on ifindex meaning what the hook thinks it means.

Actually, I think I like that approach.  What if it we had a "socket"
controller and files like "socket.create_socket", "socket.ingress" and
"socket.egress"?  You could use the bpf() syscall to install a bpf
filter into "socket.egress" and that filter would filter egress for
the target cgroup and its descendents on the current netns.  As a
first pass, the netns issue could be sidestepped by making it only
work in the root netns (i.e. the bpf() call would fail if you're not
in the root netns and the hooks wouldn't run if you're not in the root
netns.)  It *might* even be possible to retrofit in the socket
controller by saying that the default hierarchy is used if the socket
controller isn't mounted.

What *isn't* possible to cleanly fix after the fact is the current
semantics that cgroup hooks override the hooks in their ancestors.
IMO that is simply broken.  The example you gave (perf_event) is very
careful not to have this problem.

>
>> > I'm sure we'll
>> > eventually get there but from what I hear it isn't something we can
>> > pull off in a restricted timeframe.
>>
>> To me, this sounds like "the API isn't that great, we know how to do
>> better, but we really really want this feature ASAP so we want to ship
>> it with a sub-par API."  I think that's a bad idea.
>
> I see your point but this isn't something which is black and white.
> There are arguments for both directions.  I'm leaning the other
> direction because
>
> * My impression with bpf is that while delegation is something
>   theoretically possible it is not something which is gonna take place
>   in any immediate time frame.  If I'm wrong on this, please feel free
>   to correct me.

But the issue isn't *BPF* delegation.  It's cgroup delegation or netns
creation, both of which exist today.

>
> * There are a lot of use cases which can immediately take advantage of
>   cgroup-aware bpf programs operating as proposed.  The model is
>   semantically equivalent to iptables (let's address the netns part if
>   that's an issue) which net people are familiar with.

That sounds like a great argument for those users to patch their
kernels to gain experience with this feature.

>
> * It isn't exclusive with adding cgroup bpf controller down the road
>   if necessary and practical.  Sure, this isn't the perfect situation
>   but it seems like an acceptable trade-off to me.  What ever is
>   perfect?

I think it more or less is exclusive.  I can imagine davem accepting a
patch to make the sock_cgroup_data think point at a new "socket"
cgroup type.  I can't imagine him accepting a patch to have sockets
have more than one pointer to a cgroup, with good reason.

I don't see what's added by having a "bpf" cgroup controller, though
-- it's just too broad.  If the Landlock stuff goes in, that should
presumably be either tied to the default hierarchy or use an "lsm"
controller.  A "bpf" controller for that makes no sense to me.

I can see *huge* value in having some combination of BPF and the
perf_event controller deciding whether to log perf events, for
example.  But this would be the perf_event controller, not a
hypothetical "bpf" controller.

>
>> > This also holds true for the perf controller.  While it is implemented
>> > as a controller, it isn't visible to cgroup users in any way and the
>> > only function it serves is providing the membership test to perf
>> > subsystem.  perf is the one which decides whether and how it is to be
>> > used.  cgroup providing membership test to other subsystems is
>> > completely acceptable and established.
>>
>> Unless I'm mistaken, "perf_event" is an actual cgroup controller, and
>
> Yeah, it is implemented as a controller but in essence what it does is
> just tagging the hierarchy to tell perf to use that hierarchy for
> membership test purposes.

It's also a rather innocuous controller in that it doesn't change the
behavior of the controlled tasks.  The bpf hooks definitely do change
behavior.

>
>> it explicitly respects the cgroup hierarchy.  It shows up in
>> /proc/cgroups, and I had no problem mounting a cgroupfs instance with
>> perf_event enabled.  So I'm not sure what you mean.
>
> That all it's doing is providing membership information.

But it's doing it wrong!  Even perf_event tests for membership in a
given cgroup *or one of its descendents*.  This code does not.

I think the moral of the story here is that there are lots of open
questions and design work to be done and that this feature really
isn't ready to be stable.  For Landlock, I believe that it really
needs to be done right and I will put my foot down and NAK any effort
to have Landlock available in a released kernel without resolving
these types of issues first.  Does anyone really want Landlock to work
differently than the net hooks simply because the net hooks were in a
rush?

--Andy

^ permalink raw reply

* sales--【充电站+新能源汽车展】 2017第八届中国(广州)国际新能源汽车工业展  (G-YD2-C50-Me)
From: eaebtqp @ 2017-01-18 19:52 UTC (permalink / raw)


中华人民共和国商务部引导支持展会
国家级国际性汽车充电设备展贸平台
 
2017第八届中国(广州)国际新能源汽车工业展览会 (NEA 2017)
The 8th Guangzhou International New Energy Automobiles Industry Expo 2017
 
2017第八届中国(广州)国际充电站技术装备展览会 (EVE 2017)
The 8th Guangzhou International Electric Vehicle Supply Equipments Expo 2017
 
 
【 展 会 信 息 】
 
■ 展览日期
2017年06月02—04日
 
■ 展览场馆
中国•广州琶洲保利世贸博览馆
 
■ 批准举办
中华人民共和国商务部
批文号:商服贸批(2015)823号
 
■ 支持单位
中华人民共和国国家能源局
中华人民共和国国家发展和改革委员会
广东省人民政府
广州市人民政府
国家电网
南方电网 •••
 
■ 主办单位
中国对外贸易经济合作企业协会
 
■ 联合承办
映德会展(北京)有限公司
广州中汽展览有限公司
 
■ 官方网站
http://www.CAPE-china.com
 
■ 在线客服: 
邮箱/QQ:12809395#qq.com;  微信:yondexpo;  微博:http://weibo.com/yingdehuizhan
 
■ 咨询电话: 
4000-580-850(转8144或8699); 139-1031-8144; 010―8699-7155、 8084-2128; 
 
 
【 宣 传 推 广 】
 
  2017第八届广州国际新能源汽车工业展览会将是一次立足泛珠三角,依托内地、辐射亚太、放眼世界具有进出口双向导向型的现代化国际性专业品牌盛会。主办方将保证所有参展商能透过本次专业展会有效的产品推介及形象展示,并最终达成市场成功推广销售之目的。专业观众的组织和邀请是组委会工作重点之一,主办方将引进国际先进的CRM客户关系管理系统,整合政府、协会、生产企业、媒体资源及主办方庞大的买家数据库,通过直邮、传真、电子邮件、拜访、VIP邀请、即时推广、广告传媒、展前预览、专业观众在线登记、行业展会推介、专业媒体推介、展商邀请、行业协会组团和国际合作等多种灵活有效的方式组织专业人士到场参观采购。
  组委会将在上百家新闻媒体、专业杂志、网站上发布本次展会信息,刊登广告;坚持以主流媒体面对消费者,专业媒体面对厂商、经销商的传播导向,结合大型活动的影响力、权威性,以注意力营消的新观念,全力打造具有国际影响力的专业展会。
  主办方将充分发挥自身优势,与全国范围内超过1000家大型新能源汽车及充电桩市场携手对展会进行宣传。邀请组织全国各地区所有新能源汽车市场及全国各地新能源汽车行业相关协会会员单位参展参观。
  具体举措如下: 
■ 联手海内外各大知名媒体、专业媒体对展会进行强势宣传和深入报道,与行业权威媒体强强合作,共同打造新能源汽车行业的专业展会;
■ 在国内外各大汽车行业展会上推广本次展会、广发参观券;
■ 对海内外9万多专业观众邮寄参观券邀请,对主办方20多万行业买家数据发送电子邮件邀请,对海内外100000家以上的新能源汽车市场销售企业发传真邀请,对海内外行业内超过1000位专业VIP人士发送请柬邀请;
■ 通过外国驻华使领馆、商务处和贸易机构向其本国相关新能源汽车行业协会和机构推介本次展会,邀请有关企业参展参观,同时通过我国驻外使领馆和商务处推介展会。
■ 从往届展会观众数据库中,挖掘专业观众,继续邀请他们参观本届展会;同时委托海内外各专业协(学)会、行业媒体分发展会宣传品并组织参观;完整的赠票计划将协助展商将请柬送至最合适的目标观众群体;
■ 积极为展商提供不断更新的市场资料和政策方面的指导与咨询,开展新建、在建项目发布,将展览会的贸易活动进一步延伸,引导参展商制定适宜的营销策略;并将利用自身的社会资源和卓越的组织能力,协助参展商组织举办产品推广会,使参展商最大限度地扩大参展的效益。
 
 
【 参 展 细 则 】    
 
■ 展位规格: 
  1、特装展位:36平方米起租,仅提供相应面积室内外空地。展台搭建、展览器具、用电用水等自理。 
  2、标准展位:9平方米(3m×3m)每个,2.5m高壁板、一条楣板(展商名称)、一张洽谈桌、两把椅子、两盏射灯、220V/5A电源插座一处。 
 
■ 展位费用:      
  特装展位:境内企业RMB1200/平方米;  境外企业USD350/平方米; 
  标准展位:境内企业RMB12000/个;  境外企业USD3500/个; (双面开口标准展位另加收10%费用)
 
■ 会刊广告: (大会《会刊》将帮助您在展会后找到客户!除在展会期间广为发送外,还通过各种有关渠道发送给未能前来参观展会的各地专业人士手中,他们可利用会刊迅速查找服务内容与联络方法。 会刊尺寸:130mm*210mm,进口铜板纸彩色精印,发行量10万册。)
  封面 CNY 30000;     封二封三 CNY 22000;     扉页 CNY 18000;     黑白页 CNY 5000;
  封底 CNY 20000;     彩页跨版 CNY 18000;     彩页 CNY 12000;     300字简介 CNY 2000;
 
■ 会议论坛:
  如技术交流会/产品推广发布会,CNY9000/小时/场,用于会场及相关设备租金(包括场地、扩音设施、灯具、投影机、投影仪,桌椅、空调、茶水并协助主讲企业组织听众)。
 
 
【 筹 展 联 络 】    
  
全国统一客服热线: 4000-580-850.(分机:8144、5206或5220)
全国统一报名专线: 4000-680-860.(兼传真)
官方网站: http://www.CAPE-china.com 
参展顾问: 王先生、 李先生、 段小姐、 梁小姐
参展热线: 131-2662-5206、 139-1031-8144;  010— 8699- 7155、 8084- 2128.
在线客服: QQ/邮箱:12809395#qq.com;  微信/ yondexpo; 微博/ http://weibo.com/yondexpo
 
 
              〔 映德会展 / 诚邀参加2017年度权威实效品牌盛会﹞
 
 
【 公 众 平 台 】
 
微信: 参展消息 (ID:CanZhanXiaoXi)—— 品牌扩张的平台 市场开拓的桥梁
微信: 展商之家 (ID:ZhanShangZhiJia)—— 为展商提供最佳营地 为阁下营造参展价值
 
-----------------------------------------------------------------------------------------------------------
百万群发系统|为您发送|如不希望再收到此行业资讯|请回复“退订+NEA2017”至邮箱1055800812@qq.com

^ permalink raw reply

* Re: [PATCH 1/2] qed: Add support for hardware offloaded FCoE.
From: Arun Easi @ 2017-01-18 23:26 UTC (permalink / raw)
  To: kbuild test robot
  Cc: Dupuis, Chad, kbuild-all, martin.petersen, linux-scsi, fcoe-devel,
	netdev, yuval.mintz, QLogic-Storage-Upstream
In-Reply-To: <201701172240.wF4OEtiC%fengguang.wu@intel.com>

This will be addressed in v2 of the series. The warning pops up when 
CONFIG_DCB is not set. Thanks.

Regards,
-Arun

On Tue, 17 Jan 2017, 6:35am, kbuild test robot wrote:

> Hi Arun,
> 
> [auto build test WARNING on net-next/master]
> [also build test WARNING on v4.10-rc4 next-20170117]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Dupuis-Chad/Add-QLogic-FastLinQ-FCoE-qedf-driver/20170117-052438
> config: i386-randconfig-c0-01172134 (attached as .config)
> compiler: gcc-4.9 (Debian 4.9.4-2) 4.9.4
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386 
> 
> All warnings (new ones prefixed by >>):
> 
>    In file included from drivers/net/ethernet/qlogic/qed/qed.h:49:0,
>                     from drivers/net/ethernet/qlogic/qed/qed_cxt.c:44:
> >> include/linux/qed/qed_if.h:428:37: warning: 'struct qed_dcbx_get' declared inside parameter list
>      void (*dcbx_aen)(void *dev, struct qed_dcbx_get *get, u32 mib_type);
>                                         ^
> >> include/linux/qed/qed_if.h:428:37: warning: its scope is only this definition or declaration, which is probably not what you want
> --
>    In file included from drivers/net/ethernet/qlogic/qed/qed.h:49:0,
>                     from drivers/net/ethernet/qlogic/qed/qed_dcbx.c:41:
> >> include/linux/qed/qed_if.h:428:37: warning: 'struct qed_dcbx_get' declared inside parameter list
>      void (*dcbx_aen)(void *dev, struct qed_dcbx_get *get, u32 mib_type);
>                                         ^
> >> include/linux/qed/qed_if.h:428:37: warning: its scope is only this definition or declaration, which is probably not what you want
>    drivers/net/ethernet/qlogic/qed/qed_dcbx.c: In function 'qed_dcbx_aen':
>    drivers/net/ethernet/qlogic/qed/qed_dcbx.c:873:42: error: 'struct qed_dcbx_info' has no member named 'get'
>       op->dcbx_aen(cookie, &hwfn->p_dcbx_info->get, mib_type);
>                                              ^
>    drivers/net/ethernet/qlogic/qed/qed_dcbx.c: In function 'qed_dcbx_mib_update_event':
>    drivers/net/ethernet/qlogic/qed/qed_dcbx.c:902:2: error: implicit declaration of function 'qed_dcbx_get_params' [-Werror=implicit-function-declaration]
>      qed_dcbx_get_params(p_hwfn, p_ptt, &p_hwfn->p_dcbx_info->get, type);
>      ^
>    drivers/net/ethernet/qlogic/qed/qed_dcbx.c:902:57: error: 'struct qed_dcbx_info' has no member named 'get'
>      qed_dcbx_get_params(p_hwfn, p_ptt, &p_hwfn->p_dcbx_info->get, type);
>                                                             ^
>    cc1: some warnings being treated as errors
> 
> vim +428 include/linux/qed/qed_if.h
> 
>    412		u8	name[QED_DRV_VER_STR_SIZE];
>    413	};
>    414	
>    415	#define ILT_PAGE_SIZE_TCFC 0x8000 /* 32KB */
>    416	
>    417	struct qed_int_info {
>    418		struct msix_entry	*msix;
>    419		u8			msix_cnt;
>    420	
>    421		/* This should be updated by the protocol driver */
>    422		u8			used_cnt;
>    423	};
>    424	
>    425	struct qed_common_cb_ops {
>    426		void	(*link_update)(void			*dev,
>    427				       struct qed_link_output	*link);
>  > 428		void	(*dcbx_aen)(void *dev, struct qed_dcbx_get *get, u32 mib_type);
>    429	};
>    430	
>    431	struct qed_selftest_ops {
>    432	/**
>    433	 * @brief selftest_interrupt - Perform interrupt test
>    434	 *
>    435	 * @param cdev
>    436	 *
> 
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
> 

^ permalink raw reply

* [PATCH next 0/3] use netdev_is_rx_handler_busy() in few known cases
From: Mahesh Bandewar @ 2017-01-18 23:02 UTC (permalink / raw)
  To: David Miller; +Cc: Eric Dumazet, netdev, Mahesh Bandewar, Mahesh Bandewar

From: Mahesh Bandewar <maheshb@google.com>

netdev_rx_handler_register() was recently split into two parts - (a) check
if the handler is used, (b) register the new handler, parts. This is 
helpful in scenarios like bonding where at the time of registration there
is too much state to unwind and it should check if the device is free
before building that state. IPvlan and macvlan drivers don't have this
issue however it can make use of the same check instead of using a device
specific check.

Mahesh Bandewar (3):
  net: remove duplicate code.
  ipvlan: use netdev_is_rx_handler_busy instead of checking specific
    type
  macvlan: use netdev_is_rx_handler_busy instead of checking specific
    type

 drivers/net/ipvlan/ipvlan_main.c | 4 ++--
 drivers/net/macvlan.c            | 2 +-
 net/core/dev.c                   | 4 +---
 3 files changed, 4 insertions(+), 6 deletions(-)

-- 
2.11.0.483.g087da7b7c-goog

^ permalink raw reply

* [PATCH next 3/3] macvlan: use netdev_is_rx_handler_busy instead of checking specific type
From: Mahesh Bandewar @ 2017-01-18 23:02 UTC (permalink / raw)
  To: David Miller; +Cc: Eric Dumazet, netdev, Mahesh Bandewar, Mahesh Bandewar

From: Mahesh Bandewar <maheshb@google.com>

netdev_is_rx_handler_busy() check is a superset of netif_is_ipvlan_port()
check and hence should be preferred.

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
 drivers/net/macvlan.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index 440ab3d8adf7..cbfc1be23a0e 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -1110,7 +1110,7 @@ static int macvlan_port_create(struct net_device *dev)
 	if (dev->type != ARPHRD_ETHER || dev->flags & IFF_LOOPBACK)
 		return -EINVAL;
 
-	if (netif_is_ipvlan_port(dev))
+	if (netdev_is_rx_handler_busy(dev))
 		return -EBUSY;
 
 	port = kzalloc(sizeof(*port), GFP_KERNEL);
-- 
2.11.0.483.g087da7b7c-goog

^ permalink raw reply related

* [PATCH next 1/3] net: remove duplicate code.
From: Mahesh Bandewar @ 2017-01-18 23:02 UTC (permalink / raw)
  To: David Miller; +Cc: Eric Dumazet, netdev, Mahesh Bandewar, Mahesh Bandewar

From: Mahesh Bandewar <maheshb@google.com>

netdev_rx_handler_register() checks to see if the handler is already
busy which was recently separated into netdev_is_rx_handler_busy(). So
use the same function inside register() to avoid code duplication.
Essentially this change should be a no-op

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
 net/core/dev.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index ad5959e56116..c8f1f67ff16c 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3961,9 +3961,7 @@ int netdev_rx_handler_register(struct net_device *dev,
 			       rx_handler_func_t *rx_handler,
 			       void *rx_handler_data)
 {
-	ASSERT_RTNL();
-
-	if (dev->rx_handler)
+	if (netdev_is_rx_handler_busy(dev))
 		return -EBUSY;
 
 	/* Note: rx_handler_data must be set before rx_handler */
-- 
2.11.0.483.g087da7b7c-goog

^ permalink raw reply related

* [PATCH next 2/3] ipvlan: use netdev_is_rx_handler_busy instead of checking specific type
From: Mahesh Bandewar @ 2017-01-18 23:02 UTC (permalink / raw)
  To: David Miller; +Cc: Eric Dumazet, netdev, Mahesh Bandewar, Mahesh Bandewar

From: Mahesh Bandewar <maheshb@google.com>

IPvlan checks if the master device is already used by checking a
specific device (here it's macvlan device). This is technically not
sufficient and it should just ensure the rx_handler is busy or not.
This would be a super check that includes macvlan and any other that
has already registered rx-handler.

Signed-off-by: Mahesh Bandewar <maheshb@google.com>
---
 drivers/net/ipvlan/ipvlan_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
index b5c390f0f2b3..95b18f4602cf 100644
--- a/drivers/net/ipvlan/ipvlan_main.c
+++ b/drivers/net/ipvlan/ipvlan_main.c
@@ -102,8 +102,8 @@ static int ipvlan_port_create(struct net_device *dev)
 		return -EINVAL;
 	}
 
-	if (netif_is_macvlan_port(dev)) {
-		netdev_err(dev, "Master is a macvlan port.\n");
+	if (netdev_is_rx_handler_busy(dev)) {
+		netdev_err(dev, "Device is already in use.\n");
 		return -EBUSY;
 	}
 
-- 
2.11.0.483.g087da7b7c-goog

^ permalink raw reply related

* Re: [PATCH v3 1/2] bpf: add a longest prefix match trie map implementation
From: Alexei Starovoitov @ 2017-01-18 22:29 UTC (permalink / raw)
  To: David Herrmann
  Cc: Alexei Starovoitov, Daniel Mack, Daniel Borkmann, netdev,
	David S. Miller
In-Reply-To: <CANq1E4TY1BZfa1da1DB6=M==+JJUuATxT5iCJAAn2_Bi=yuiJw@mail.gmail.com>

On Wed, Jan 18, 2017 at 03:30:14PM +0100, David Herrmann wrote:
> Hi
> 
> On Sat, Jan 14, 2017 at 5:55 PM, Alexei Starovoitov <ast@fb.com> wrote:
> > Another alternative is to extend samples/bpf/map_perf_test
> > It has perf tests for most map types today (including lru)
> > and trie would be natural addition there.
> > I would prefer this latter option.
> 
> I hooked into gettid() and installed a simple kprobe bpf program that
> searches for an entry in an lpm trie. The bpf program does either 0,
> 1, 8, or 32 lookups in a row (always the same element). The trie has
> size 0, 1, or 8192. The data is below. The results vary by roughly 5%
> on every run.
> 
> A single gettid() syscall with an empty bpf program takes roughly
> 6.5us on my system. Lookups in empty tries take ~1.8us on first try,
> ~0.9us on retries. Lookups in tries with 8192 entries take ~7.1us (on
> the first _and_ any subsequent try).

thanks. please add them to commit log.

> https://gist.github.com/dvdhrm/4c90e61a1c39746d5c55ab9e0e29315e

looks good. please add it as an extra patch to this set.
and add
#pragma clang loop unroll(full)
before
+       for (i = 0; i < 8; ++i)
+               bpf_map_lookup_elem(&lpm_trie_map_alloc, &key);
to make sure it's unrolled regardless of the version of llvm.

> Trie-size: 0
> #Lookups: 0
>     0:lpm_perf kmalloc 9,230,321 events per sec
>     -> 6.5us / syscall
> 
> Trie-size: 1
> #Lookups: 1
>     0:lpm_perf kmalloc 7,224,508 events per sec
>     -> 8.3us / syscall
> 
> Trie-size: 1
> #Lookups: 8
>     0:lpm_perf kmalloc 4,152,740 events per sec
>     -> 14.4us / syscall
> 
> Trie-size: 1
> #Lookups: 32
>     0:lpm_perf kmalloc 1,713,415 events per sec
>     -> 35.0us / syscall
> 
> Trie-size: 8192
> #Lookups: 1
>     0:lpm_perf kmalloc 4,369,138 events per sec
>     -> 13.7us / syscall
> 
> Trie-size: 8192
> #Lookups: 8
>     0:lpm_perf kmalloc 943,849 events per sec
>     -> 63.6us / syscall
> 
> Trie-size: 8192
> #Lookups: 32
>     0:lpm_perf kmalloc 271,737 events per sec
>     -> 220.8us / syscall

so it's the same lookup done 32-times per each syscall.
Since the latency of single lookup in 8 vs 32 experiments
are pretty close, it means that the 32 lookup numbers
amortize the cost of syscall pretty well and no need to go higher.
So I would use 32 as a loop count in stress_lpm_trie_map_alloc()

Thanks!

^ permalink raw reply

* linux-next: build warning after merge of the net tree
From: Stephen Rothwell @ 2017-01-18 22:56 UTC (permalink / raw)
  To: David Miller, Networking
  Cc: linux-next, linux-kernel, Eran Ben Elisha, Tariq Toukan

Hi all,

After merging the net tree, today's linux-next build (x86_64 allmodconfig)
produced this warning:

drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c: In function 'mlx5e_set_channels':
drivers/net/ethernet/mellanox/mlx5/core/en_ethtool.c:546:6: warning: unused variable 'ncv' [-Wunused-variable]
  int ncv = mlx5e_get_max_num_channels(priv->mdev);
      ^

Introduced by commit

  639e9e94160e ("net/mlx5e: Remove unnecessary checks when setting num channels")

which removed the last reference to 'ncv'.

-- 
Cheers,
Stephen Rothwell

^ permalink raw reply

* Re: Potential issues (security and otherwise) with the current cgroup-bpf API
From: Tejun Heo @ 2017-01-18 22:41 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Michal Hocko, Peter Zijlstra, David Ahern, Alexei Starovoitov,
	Andy Lutomirski, Daniel Mack, Mickaël Salaün, Kees Cook,
	Jann Horn, David S. Miller, Thomas Graf, Michael Kerrisk,
	Linux API, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Network Development
In-Reply-To: <CALCETrW+ZQ1xMEfdEOzx6RK9ZoCvQiqQSnOyhDJ=-FZMwUbucg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

Helloo, Andy.

On Mon, Jan 16, 2017 at 09:18:36PM -0800, Andy Lutomirski wrote:
> Perhaps this is a net feature, though, not a cgroup feature.  This
> would IMO make a certain amount of sense.  Iptables cgroup matches,
> for example, logically are an iptables (i.e., net) feature.  The

Yeap.

> problem here is that network configuration (and this explicitly
> includes iptables) is done on a per-netns level, whereas these new
> hooks entirely ignore network namespaces.  I've pointed out that
> trying to enable them in a namespaced world is problematic (e.g.
> switching to ns_capable() will cause serious problems), and Alexei
> seems to think that this will never happen.  So I don't think we can
> really call this a net feature that works the way that other net
> features work.
> 
> (Suppose that this actually tried to be netns-enabled.  Then you'd
> have to map from (netns, cgroup) -> hook, and this would probably be
> slow and messy.)

I'm not sure how important netns support for these bpf programs
actually is but I have no objection to making it behave in an
equivalent manner to iptables where appropriate.  This is kinda
trivial to do too.  For now, we can simply not run the programs if the
associated socket belongs to non-root netns.  Later, if necessary, we
can add per-ns bpf programs.  And I can't think of a reason why
(netns, cgroup) -> function mapping would be slow and messy.  Why
would that be?

> So I think that leaves it being a cgroup feature.  And it definitely
> does *not* play by the rules of cgroups right now.

Because this in no way is a cgroup feature.  As you wrote above, it is
something similar to iptables with lacking netns considerations.
Let's address that and make it a proper network thing.

> > I'm sure we'll
> > eventually get there but from what I hear it isn't something we can
> > pull off in a restricted timeframe.
> 
> To me, this sounds like "the API isn't that great, we know how to do
> better, but we really really want this feature ASAP so we want to ship
> it with a sub-par API."  I think that's a bad idea.

I see your point but this isn't something which is black and white.
There are arguments for both directions.  I'm leaning the other
direction because

* My impression with bpf is that while delegation is something
  theoretically possible it is not something which is gonna take place
  in any immediate time frame.  If I'm wrong on this, please feel free
  to correct me.

* There are a lot of use cases which can immediately take advantage of
  cgroup-aware bpf programs operating as proposed.  The model is
  semantically equivalent to iptables (let's address the netns part if
  that's an issue) which net people are familiar with.

* It isn't exclusive with adding cgroup bpf controller down the road
  if necessary and practical.  Sure, this isn't the perfect situation
  but it seems like an acceptable trade-off to me.  What ever is
  perfect?

> > This also holds true for the perf controller.  While it is implemented
> > as a controller, it isn't visible to cgroup users in any way and the
> > only function it serves is providing the membership test to perf
> > subsystem.  perf is the one which decides whether and how it is to be
> > used.  cgroup providing membership test to other subsystems is
> > completely acceptable and established.
> 
> Unless I'm mistaken, "perf_event" is an actual cgroup controller, and

Yeah, it is implemented as a controller but in essence what it does is
just tagging the hierarchy to tell perf to use that hierarchy for
membership test purposes.

> it explicitly respects the cgroup hierarchy.  It shows up in
> /proc/cgroups, and I had no problem mounting a cgroupfs instance with
> perf_event enabled.  So I'm not sure what you mean.

That all it's doing is providing membership information.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH net-next] net: Remove usage of net_device last_rx member
From: David Miller @ 2017-01-18 22:27 UTC (permalink / raw)
  To: tklauser
  Cc: devel, eric.dumazet, netdev, b.a.t.m.a.n, linux-m68k,
	intel-wired-lan, j.vosburgh, vfalico, andy, mlindner
In-Reply-To: <20170118164501.1934-1-tklauser@distanz.ch>

From: Tobias Klauser <tklauser@distanz.ch>
Date: Wed, 18 Jan 2017 17:45:01 +0100

> The network stack no longer uses the last_rx member of struct net_device
> since the bonding driver switched to use its own private last_rx in
> commit 9f242738376d ("bonding: use last_arp_rx in slave_last_rx()").
> 
> However, some drivers still (ab)use the field for their own purposes and
> some driver just update it without actually using it.
> 
> Previously, there was an accompanying comment for the last_rx member
> added in commit 4dc89133f49b ("net: add a comment on netdev->last_rx")
> which asked drivers not to update is, unless really needed. However,
> this commend was removed in commit f8ff080dacec ("bonding: remove
> useless updating of slave->dev->last_rx"), so some drivers added later
> on still did update last_rx.
> 
> Remove all usage of last_rx and switch three drivers (sky2, atp and
> smc91c92_cs) which actually read and write it to use their own private
> copy in netdev_priv.
> 
> Compile-tested with allyesconfig and allmodconfig on x86 and arm.
> 
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Jay Vosburgh <j.vosburgh@gmail.com>
> Cc: Veaceslav Falico <vfalico@gmail.com>
> Cc: Andy Gospodarek <andy@greyhouse.net>
> Cc: Mirko Lindner <mlindner@marvell.com>
> Cc: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net] bpf: don't trigger OOM killer under pressure with map alloc
From: David Miller @ 2017-01-18 22:13 UTC (permalink / raw)
  To: daniel; +Cc: alexei.starovoitov, netdev
In-Reply-To: <1f39f6b28b1ec25a88b90dd95aa4ea7820d0d53a.1484747846.git.daniel@iogearbox.net>

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Wed, 18 Jan 2017 15:14:17 +0100

> This patch adds two helpers, bpf_map_area_alloc() and bpf_map_area_free(),
> that are to be used for map allocations. Using kmalloc() for very large
> allocations can cause excessive work within the page allocator, so i) fall
> back earlier to vmalloc() when the attempt is considered costly anyway,
> and even more importantly ii) don't trigger OOM killer with any of the
> allocators.
> 
> Since this is based on a user space request, for example, when creating
> maps with element pre-allocation, we really want such requests to fail
> instead of killing other user space processes.
> 
> Also, don't spam the kernel log with warnings should any of the allocations
> fail under pressure. Given that, we can make backend selection in
> bpf_map_area_alloc() generic, and convert all maps over to use this API
> for spots with potentially large allocation requests.
> 
> Note, replacing the one kmalloc_array() is fine as overflow checks happen
> earlier in htab_map_alloc(), since it must also protect the multiplication
> for vmalloc() should kmalloc_array() fail.
> 
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> Acked-by: Alexei Starovoitov <ast@kernel.org>

Applied, thanks.

^ permalink raw reply

* Re: Potential issues (security and otherwise) with the current cgroup-bpf API
From: Tejun Heo @ 2017-01-18 22:18 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Peter Zijlstra, Andy Lutomirski, David Ahern, Alexei Starovoitov,
	Andy Lutomirski, Daniel Mack, Mickaël Salaün, Kees Cook,
	Jann Horn, David S. Miller, Thomas Graf, Michael Kerrisk,
	Linux API, linux-kernel@vger.kernel.org, Network Development
In-Reply-To: <20170117135830.GO19699@dhcp22.suse.cz>

Hello, Michal.

On Tue, Jan 17, 2017 at 02:58:30PM +0100, Michal Hocko wrote:
> This would require using hierarchical cgroup iterators to iterate over

It does behave hierarchically.

> tasks. As per Andy's testing this doesn't seem to be the case. I haven't

That's not what Andy's testing showed.  What that showed was that
program in a child can override the one from its ancestor.

> checked the implementation closely but my understanding was that using
> only cgroup specific tasks was intentional.

Maybe I'm misreading you but if you're saying that a bpf program would
only execute for tasks on that specific cgroup, that is not true.

> I do agree that using hierarchy aware cgroup iterators is the right
> approach here and we wouldn't see any issue. But I am still not sure
> I've wrapped my head around this feature completely.

It's really simple.  You can install bpf programs with a cgroup path
associated with them.  When that particular type of bpf program needs
to be executed, the bpf program which is attached to the closest
ancestor (including self) is executed.

Thanks.

-- 
tejun

^ permalink raw reply

* Re: [PATCH net v2] lwtunnel: fix autoload of lwt modules
From: David Miller @ 2017-01-18 22:07 UTC (permalink / raw)
  To: dsa; +Cc: netdev, rshearma
In-Reply-To: <1484693856-26021-1-git-send-email-dsa@cumulusnetworks.com>

From: David Ahern <dsa@cumulusnetworks.com>
Date: Tue, 17 Jan 2017 14:57:36 -0800

> Trying to add an mpls encap route when the MPLS modules are not loaded
> hangs. For example:
 ...
> The problem is that lwtunnel_build_state is called with rtnl lock
> held preventing mpls_init from registering.
> 
> Given the potential references held by the time lwtunnel_build_state it
> can not drop the rtnl lock to the load module. So, extract the module
> loading code from lwtunnel_build_state into a new function to validate
> the encap type. The new function is called while converting the user
> request into a fib_config which is well before any table, device or
> fib entries are examined.
> 
> Fixes: 745041e2aaf1 ("lwtunnel: autoload of lwt modules")
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>
> ---
> v2
> - extract the module load attempt into a separate function that is
>   called early in the newroute code paths

Ok, this looks like it should work.

Assuming it passed my build tests, I'll push this out and queue it for
-stable.

Thanks!

^ permalink raw reply

* Re: [PATCH net-next] lwtunnel: remove device arg to lwtunnel_build_state
From: David Miller @ 2017-01-18 22:08 UTC (permalink / raw)
  To: dsa; +Cc: netdev, roopa
In-Reply-To: <1484694521-17479-1-git-send-email-dsa@cumulusnetworks.com>

From: David Ahern <dsa@cumulusnetworks.com>
Date: Tue, 17 Jan 2017 15:08:41 -0800

> Nothing about lwt state requires a device reference, so remove the
> input argument.
> 
> Signed-off-by: David Ahern <dsa@cumulusnetworks.com>

This is going to conflict with the module loading patch, why don't you
respin this the next time I merge net into net-next?

Thanks.

^ permalink raw reply

* Re: [PATCH net-next v4 00/10] net: dsa: Support for pdata in dsa2
From: Florian Fainelli @ 2017-01-18 22:06 UTC (permalink / raw)
  To: David Miller
  Cc: netdev, jason, andrew, sebastian.hesselbarth, gregory.clement,
	linux, vivien.didelot, gregkh, linux-arm-kernel, linux-kernel
In-Reply-To: <20170118.165050.2165692521961713529.davem@davemloft.net>

On 01/18/2017 01:50 PM, David Miller wrote:
> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Tue, 17 Jan 2017 15:21:42 -0800
> 
>> This is not exactly new, and was sent before, although back then, I did not
>> have an user of the pre-declared MDIO board information, but now we do. Note
>> that I have additional changes queued up to have b53 register platform data for
>> MIPS bcm47xx and bcm63xx.
>  ...
> 
> We're waiting for Greg's further review and Vivien's changes which I just
> applied are going to create some conflicts, so I am presuming that you will
> respin a v5 anyways.

Yes indeed. Thanks for your patience!
-- 
Florian

^ permalink raw reply

* Re: [PATCH net] bnxt_en: Fix "uninitialized variable" bug in TPA code path.
From: David Miller @ 2017-01-18 22:02 UTC (permalink / raw)
  To: michael.chan; +Cc: netdev, andrew.gospodarek
In-Reply-To: <1484708839-11745-1-git-send-email-michael.chan@broadcom.com>

From: Michael Chan <michael.chan@broadcom.com>
Date: Tue, 17 Jan 2017 22:07:19 -0500

> In the TPA GRO code path, initialize the tcp_opt_len variable to 0 so
> that it will be correct for packets without TCP timestamps.  The bug
> caused the SKB fields to be incorrectly set up for packets without
> TCP timestamps, leading to these packets being rejected by the stack.
> 
> Reported-by: Andy Gospodarek <andrew.gospodarek@broadocm.com>
> Acked-by: Andy Gospodarek <andrew.gospodarek@broadocm.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>

Applied, thanks Andy.

^ permalink raw reply

* Re: fq_codel and skb->hash
From: Andrew Collins @ 2017-01-18 20:21 UTC (permalink / raw)
  To: Eric Dumazet, Tom Herbert; +Cc: Linux Kernel Network Developers, Eric Dumazet
In-Reply-To: <1484666094.13165.68.camel@edumazet-glaptop3.roam.corp.google.com>

On 01/17/2017 08:14 AM, Eric Dumazet wrote:
> Right, but that might add overhead in cases we do not need skb->hash
> after IPsec . I've heard IPsec is already quite slow :/

I've been running with the following change locally with good results:

--- a/net/sched/sch_fq_codel.c
+++ b/net/sched/sch_fq_codel.c
@@ -57,7 +57,6 @@ struct fq_codel_sched_data {
         struct fq_codel_flow *flows;    /* Flows table [flows_cnt] */
         u32             *backlogs;      /* backlog table [flows_cnt] */
         u32             flows_cnt;      /* number of flows */
-       u32             perturbation;   /* hash perturbation */
         u32             quantum;        /* psched_mtu(qdisc_dev(sch)); */
         u32             drop_batch_size;
         u32             memory_limit;
@@ -75,9 +74,7 @@ struct fq_codel_sched_data {
  static unsigned int fq_codel_hash(const struct fq_codel_sched_data *q,
                                   struct sk_buff *skb)
  {
-       u32 hash = skb_get_hash_perturb(skb, q->perturbation);
-
-       return reciprocal_scale(hash, q->flows_cnt);
+       return reciprocal_scale(skb_get_hash(skb), q->flows_cnt);
  }

  static unsigned int fq_codel_classify(struct sk_buff *skb, struct Qdisc *sch,
@@ -482,7 +479,6 @@ static int fq_codel_init(struct Qdisc *sch, struct nlattr *opt)
         q->memory_limit = 32 << 20; /* 32 MBytes */
         q->drop_batch_size = 64;
         q->quantum = psched_mtu(qdisc_dev(sch));
-       q->perturbation = prandom_u32();
         INIT_LIST_HEAD(&q->new_flows);
         INIT_LIST_HEAD(&q->old_flows);
         codel_params_init(&q->cparams);

Any interest in me spinning a real patch for this?  I agree that it'd be better
if we were guaranteed to get a pre-encryption flow hash for any IPsec traffic,
but in my particular case I don't care, as I control the HW and can make it give
me a hash. :)

Thanks,
Andrew Collins

^ permalink raw reply

* Re: [PATCH net] net: phy: bcm63xx: Utilize correct config_intr function
From: David Miller @ 2017-01-18 21:57 UTC (permalink / raw)
  To: f.fainelli; +Cc: netdev, jonas.gorski, dgcbueu
In-Reply-To: <20170118002655.6865-1-f.fainelli@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Tue, 17 Jan 2017 16:26:55 -0800

> From: Daniel Gonzalez Cabanelas <dgcbueu@gmail.com>
> 
> Commit a1cba5613edf ("net: phy: Add Broadcom phy library for common
> interfaces") make the BCM63xx PHY driver utilize bcm_phy_config_intr()
> which would appear to do the right thing, except that it does not write
> to the MII_BCM63XX_IR register but to MII_BCM54XX_ECR which is
> different.
> 
> This would be causing invalid link parameters and events from being
> generated by the PHY interrupt.
> 
> Fixes: a1cba5613edf ("net: phy: Add Broadcom phy library for common interfaces")
> Signed-off-by: Daniel Gonzalez Cabanelas <dgcbueu@gmail.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>

Applied and queued up for -stable, thanks!

^ permalink raw reply

* Re: [PATCH net-next v4 00/10] net: dsa: Support for pdata in dsa2
From: David Miller @ 2017-01-18 21:50 UTC (permalink / raw)
  To: f.fainelli
  Cc: netdev, jason, andrew, sebastian.hesselbarth, gregory.clement,
	linux, vivien.didelot, gregkh, linux-arm-kernel, linux-kernel
In-Reply-To: <20170117232152.1661-1-f.fainelli@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Tue, 17 Jan 2017 15:21:42 -0800

> This is not exactly new, and was sent before, although back then, I did not
> have an user of the pre-declared MDIO board information, but now we do. Note
> that I have additional changes queued up to have b53 register platform data for
> MIPS bcm47xx and bcm63xx.
 ...

We're waiting for Greg's further review and Vivien's changes which I just
applied are going to create some conflicts, so I am presuming that you will
respin a v5 anyways.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox