Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH] qlcnic: remove redundant zero check on retries counter
From: David Miller @ 2017-09-01 17:34 UTC (permalink / raw)
  To: colin.king
  Cc: harish.patil, manish.chopra, Dept-GELinuxNICDev, netdev,
	kernel-janitors, linux-kernel
In-Reply-To: <20170901134431.23179-1-colin.king@canonical.com>

From: Colin King <colin.king@canonical.com>
Date: Fri,  1 Sep 2017 14:44:31 +0100

> From: Colin Ian King <colin.king@canonical.com>
> 
> At the end of the do while loop the integer counter retries will
> always be zero and so the subsequent check to see if it is zero
> is always true and therefore redundant.  Remove the redundant check
> and always return -EIO on this return path.  Also unbreak the literal
> string in dev_err message to clean up a checkpatch warning.
> 
> Detected by CoverityScan, CID#744279 ("Logically dead code")
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>

Applied to net-next, thanks.

^ permalink raw reply

* Re: [RESEND PATCH] Allow passing tid or pid in SCM_CREDENTIALS without CAP_SYS_ADMIN
From: Prakash Sangappa @ 2017-09-01 17:30 UTC (permalink / raw)
  To: Eric W. Biederman; +Cc: David Miller, linux-kernel, netdev, drepper
In-Reply-To: <87inh5ymv2.fsf@xmission.com>



On 8/30/17 10:41 AM, ebiederm@xmission.com wrote:
> Prakash Sangappa <prakash.sangappa@oracle.com> writes:
>
>
>> With regards to security, the question basically is what is the consequence
>> of passing the wrong id. As I understand it, Interpreting the id to be pid
>> or tid, the effective uid and gid will be the same. It would be a problem
>> only if the incorrect interpretation of the id would refer a different process.
>> But that cannot happen as the the global tid(gettid() of a thread is
>> unique.
> There is also the issue that the receiving process could look, not see
> the pid in proc and assume the sending process is dead.  That I suspect
> is the larger danger.
>

Will this not be a bug in the application, if it is sending the wrong id?

>> As long as the thread is alive, that id cannot reference another process / thread.
>> Unless the thread were to exit and the id gets recycled and got used for another
>> thread or process. This would be no different from a process exiting and its
>> pid getting recycled which is the case now.
> Largely I agree.
>
> If all you want are pid translations I suspect the are far easier ways
> thant updating the SCM_CREDENTIALS code.

What would be an another easier & efficient way of doing pid translation?

Should a new API/mechanism be considered mainly for pid translation purpose
for use with pid namespaces, say based on 'pipe' something similar to 
I_SENDFD?

Thanks,
-Prakash.

> Eric
>

^ permalink raw reply

* Re: [PATCH net] udp: fix secpath leak
From: David Miller @ 2017-09-01 17:30 UTC (permalink / raw)
  To: pabeni; +Cc: netdev, yossiku, paul
In-Reply-To: <5bd7a6e643340d833718722c0508474c6c3a0a3a.1504260470.git.pabeni@redhat.com>

From: Paolo Abeni <pabeni@redhat.com>
Date: Fri,  1 Sep 2017 14:42:30 +0200

> From: Yossi Kuperman <yossiku@mellanox.com>
> 
> After commit dce4551cb2ad ("udp: preserve head state for IP_CMSG_PASSSEC")
> we preserve the secpath for the whole skb lifecycle, but we also
> end up leaking a reference to it.
> 
> We must clear the head state on skb reception, if secpath is
> present.
> 
> Fixes: dce4551cb2ad ("udp: preserve head state for IP_CMSG_PASSSEC")
> Signed-off-by: Yossi Kuperman <yossiku@mellanox.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Applied.

^ permalink raw reply

* Re: [PATCH v2 0/5] net: mdio-mux: Misc fix
From: David Miller @ 2017-09-01 17:26 UTC (permalink / raw)
  To: clabbe.montjoie; +Cc: andrew, f.fainelli, netdev, linux-kernel
In-Reply-To: <20170901115604.27513-1-clabbe.montjoie@gmail.com>

From: Corentin Labbe <clabbe.montjoie@gmail.com>
Date: Fri,  1 Sep 2017 13:55:59 +0200

> This patch series fix minor problems found when working on the
> dwmac-sun8i syscon mdio-mux.
 ...
> Changes since v1:
> - Removed obsolete comment about of_mdio_find_bus/put_device
> - removed more DRV_VERSION

Series applied to net-next.

^ permalink raw reply

* Re: [net 0/3] gianfar: Tx flow control fix (adjust_link)
From: David Miller @ 2017-09-01 17:15 UTC (permalink / raw)
  To: claudiu.manoil; +Cc: netdev
In-Reply-To: <1504258863-2058-1-git-send-email-claudiu.manoil@nxp.com>

From: Claudiu Manoil <claudiu.manoil@nxp.com>
Date: Fri, 1 Sep 2017 12:41:00 +0300

> Fix a small blunder in the Tx pause frame settings, that
> went unnoticed in the tangled code of adjust_link().
> I followed up with a couple of simple refactoring patches,
> aiming to make adjust_link() more manageable.
> 
> (The last 2 patches may be postponed if they are too much
> for the current stage of net.)

You need to fix some things up here.

First, do not mix bug fixes with cleanups.  Refactoring is a cleanup.

Submit the bug fix for 'net' and then later you can submit the
cleanups for 'net-next'.

Second, do not CC: stable for networking bug fixes, instead explicitly
ask me to queue up the fix for -stable.

Third, you need to fix how you specify your Fixes tag, it must
be exactly:

Fixes: $(SHA1_ID) ("Commit header text.")

And no matter how long the line is, do not break it up.

Thank you.

^ permalink raw reply

* Re: [PATCH v2 1/3] dt-bindings: add SFF vendor prefix
From: Rob Herring @ 2017-09-01 17:13 UTC (permalink / raw)
  To: Baruch Siach
  Cc: Mark Rutland, Andrew Lunn, Florian Fainelli, David S. Miller,
	Russell King, netdev, devicetree
In-Reply-To: <d1e64c5752ef0dd5c5b543c2d4c3ef1783318173.1504086672.git.baruch@tkos.co.il>

On Wed, Aug 30, 2017 at 12:51:10PM +0300, Baruch Siach wrote:
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
> v2: New patch in this series
> ---
>  Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>  1 file changed, 1 insertion(+)

Acked-by: Rob Herring <robh@kernel.org>

^ permalink raw reply

* Re: tip -ENOBOOT - bisected to locking/refcounts, x86/asm: Implement fast refcount overflow protection
From: Kees Cook @ 2017-09-01 17:12 UTC (permalink / raw)
  To: Mike Galbraith
  Cc: David S. Miller, Peter Zijlstra, LKML, Ingo Molnar,
	Reshetova, Elena, Network Development
In-Reply-To: <1504271369.332.29.camel@gmx.de>

On Fri, Sep 1, 2017 at 6:09 AM, Mike Galbraith <efault@gmx.de> wrote:
> On Fri, 2017-09-01 at 08:57 +0200, Mike Galbraith wrote:
>> On Thu, 2017-08-31 at 11:45 -0700, Kees Cook wrote:
>> > On Thu, Aug 31, 2017 at 10:19 AM, Mike Galbraith <efault@gmx.de> wrote:
>> > > On Thu, 2017-08-31 at 10:00 -0700, Kees Cook wrote:
>> > >>
>> > >> Oh! So it's gcc-version sensitive? That's alarming. Is this mapping correct:
>> > >>
>> > >> 4.8.5: WARN, eventual kernel hang
>> > >> 6.3.1, 7.0.1: WARN, but continues working
>> > >
>> > > Yeah, that's correct.  I find that troubling, simply because this gcc
>> > > version has been through one hell of a lot of kernels with me.  Yeah, I
>> > > know, that doesn't exempt it from having bugs, but color me suspicious.
>> >
>> > I still can't hit this with a 4.8.5 build. :(
>> >
>> > With _RATELIMIT removed, this should, in theory, report whatever goes
>> > negative first...
>>
>> I applied the other patch you posted, and built with gcc-6.3.1 to
>> remove the gcc-4.8.5 aspect.  Look below the resulting splat.
>
> Grr, that one has a in6_dev_getx() line missing for the first
> increment, where things go pear shaped.
>
> With that added, looking at counter both before, and after incl, with a
> trace_printk() in the exception handler showing it doing its saturate
> thing, irqs disabled across the whole damn refcount_inc(), and even
> booting box nr_cpus=1 for extra credit...
>
> HTH can that first refcount_inc() get there?
>
> # tracer: nop
> #
> #                              _-----=> irqs-off
> #                             / _----=> need-resched
> #                            | / _---=> hardirq/softirq
> #                            || / _--=> preempt-depth
> #                            ||| /     delay
> #           TASK-PID   CPU#  ||||    TIMESTAMP  FUNCTION
> #              | |       |   ||||       |         |
>          systemd-1     [000] d..1     1.937284: in6_dev_getx: PRE refs.counter:3
>          systemd-1     [000] d..1     1.937295: ex_handler_refcount: *(int *)regs->cx = -1073741824
>          systemd-1     [000] d..1     1.937296: in6_dev_getx: POST refs.counter:-1073741824

O_o

Can you paste the disassembly of in6_dev_getx? I can't understand how
we're landing in the exception handler.

>          systemd-1     [000] d..1     1.937296: in6_dev_getx: PRE refs.counter:-1073741824
>          systemd-1     [000] d..1     1.937297: ex_handler_refcount: *(int *)regs->cx = -1073741824
>          systemd-1     [000] d..1     1.937297: in6_dev_getx: POST refs.counter:-1073741824
>          systemd-1     [000] d..1     1.937297: in6_dev_getx: PRE refs.counter:-1073741824
>          systemd-1     [000] d..1     1.937298: ex_handler_refcount: *(int *)regs->cx = -1073741824
>          systemd-1     [000] d..1     1.937299: in6_dev_getx: POST refs.counter:-1073741824
>
> ---
>  arch/x86/include/asm/refcount.h |   14 ++++++++++++++
>  arch/x86/mm/extable.c           |    1 +
>  include/net/addrconf.h          |   12 ++++++++++++
>  net/ipv6/route.c                |    6 +++---
>  4 files changed, 30 insertions(+), 3 deletions(-)
>
> --- a/arch/x86/include/asm/refcount.h
> +++ b/arch/x86/include/asm/refcount.h
> @@ -55,6 +55,20 @@ static __always_inline void refcount_inc
>                 : : "cc", "cx");
>  }
>
> +static __always_inline void refcount_inc_x(refcount_t *r)
> +{
> +       unsigned long flags;
> +
> +       local_irq_save(flags);
> +       trace_printk("PRE refs.counter:%d\n", r->refs.counter);
> +       asm volatile(LOCK_PREFIX "incl %0\n\t"
> +               REFCOUNT_CHECK_LT_ZERO
> +               : [counter] "+m" (r->refs.counter)
> +               : : "cc", "cx");

Does this need an explicit "memory" added to the clobbers line here?
This isn't present in the atomic_inc() implementation, but maybe
something confuses gcc in this case into ignoring the "+m" marking?

> +       trace_printk("POST refs.counter:%d\n", r->refs.counter);
> +       local_irq_restore(flags);
> +}
> +
>  static __always_inline void refcount_dec(refcount_t *r)
>  {
>         asm volatile(LOCK_PREFIX "decl %0\n\t"
> --- a/arch/x86/mm/extable.c
> +++ b/arch/x86/mm/extable.c
> @@ -45,6 +45,7 @@ bool ex_handler_refcount(const struct ex
>  {
>         /* First unconditionally saturate the refcount. */
>         *(int *)regs->cx = INT_MIN / 2;
> +       trace_printk("*(int *)regs->cx = %d\n", *(int *)regs->cx);

Just for fun, can you print out *(int *)regs->cx before the assignment too?

>
>         /*
>          * Strictly speaking, this reports the fixup destination, not
> --- a/include/net/addrconf.h
> +++ b/include/net/addrconf.h
> @@ -321,6 +321,18 @@ static inline struct inet6_dev *in6_dev_
>         return idev;
>  }
>
> +static inline struct inet6_dev *in6_dev_getx(const struct net_device *dev)
> +{
> +       struct inet6_dev *idev;
> +
> +       rcu_read_lock();
> +       idev = rcu_dereference(dev->ip6_ptr);
> +       if (idev)
> +               refcount_inc_x(&idev->refcnt);
> +       rcu_read_unlock();
> +       return idev;
> +}
> +
>  static inline struct neigh_parms *__in6_dev_nd_parms_get_rcu(const struct net_device *dev)
>  {
>         struct inet6_dev *idev = __in6_dev_get(dev);
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -4041,12 +4041,12 @@ void __init ip6_route_init_special_entri
>          * the loopback reference in rt6_info will not be taken, do it
>          * manually for init_net */
>         init_net.ipv6.ip6_null_entry->dst.dev = init_net.loopback_dev;
> -       init_net.ipv6.ip6_null_entry->rt6i_idev = in6_dev_get(init_net.loopback_dev);
> +       init_net.ipv6.ip6_null_entry->rt6i_idev = in6_dev_getx(init_net.loopback_dev);
>    #ifdef CONFIG_IPV6_MULTIPLE_TABLES
>         init_net.ipv6.ip6_prohibit_entry->dst.dev = init_net.loopback_dev;
> -       init_net.ipv6.ip6_prohibit_entry->rt6i_idev = in6_dev_get(init_net.loopback_dev);
> +       init_net.ipv6.ip6_prohibit_entry->rt6i_idev = in6_dev_getx(init_net.loopback_dev);
>         init_net.ipv6.ip6_blk_hole_entry->dst.dev = init_net.loopback_dev;
> -       init_net.ipv6.ip6_blk_hole_entry->rt6i_idev = in6_dev_get(init_net.loopback_dev);
> +       init_net.ipv6.ip6_blk_hole_entry->rt6i_idev = in6_dev_getx(init_net.loopback_dev);
>    #endif
>  }
>

-Kees

-- 
Kees Cook
Pixel Security

^ permalink raw reply

* Re: [PATCH v2 2/3] dt-binding: net: sfp binding documentation
From: Rob Herring @ 2017-09-01 17:12 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Baruch Siach, Sergei Shtylyov, Mark Rutland, Florian Fainelli,
	David S. Miller, Russell King, netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170830145829.GA22289-g2DYL2Zd6BY@public.gmane.org>

On Wed, Aug 30, 2017 at 04:58:29PM +0200, Andrew Lunn wrote:
> > > > >     Your example shows there's GPIO phandle *and* specifier.
> > > > 
> > > > Would "GPIO specifier" be enough here?
> > > 
> > >    No, specifier is the cells following GPIO (or any other) phandle.
> > 
> > So this should be "GPIO phandle and specifier of ...", is that correct?
> > 
> > I have found very few (< 4) occurrences of this language in (lots of) '-gpios' 
> > property descriptions under Documentation/devicetree/bindings/. Is this a new 
> > requirement?
> 
> Sometimes it is just easier to refer to another document:
> 
> GPIO, as defined in Documentation/devicetree/binding/gpio/gpio.txt

Yes, and what I care about here is how many GPIOs, direction and active 
state. IOW, worry about the information necessary to validate a specific 
instance is correct. And hopefully someday we'll have a format parseable 
to do that checking, and all the free form text will be gone.

Rob
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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] bridge: switchdev: Clear forward mark when transmitting packet
From: David Miller @ 2017-09-01 17:12 UTC (permalink / raw)
  To: idosch; +Cc: mlxsw, yotamg, nikolay, netdev, bridge, jiri
In-Reply-To: <20170901092225.31597-1-idosch@mellanox.com>

From: Ido Schimmel <idosch@mellanox.com>
Date: Fri,  1 Sep 2017 12:22:25 +0300

> Commit 6bc506b4fb06 ("bridge: switchdev: Add forward mark support for
> stacked devices") added the 'offload_fwd_mark' bit to the skb in order
> to allow drivers to indicate to the bridge driver that they already
> forwarded the packet in L2.
> 
> In case the bit is set, before transmitting the packet from each port,
> the port's mark is compared with the mark stored in the skb's control
> block. If both marks are equal, we know the packet arrived from a switch
> device that already forwarded the packet and it's not re-transmitted.
> 
> However, if the packet is transmitted from the bridge device itself
> (e.g., br0), we should clear the 'offload_fwd_mark' bit as the mark
> stored in the skb's control block isn't valid.
> 
> This scenario can happen in rare cases where a packet was trapped during
> L3 forwarding and forwarded by the kernel to a bridge device.
> 
> Fixes: 6bc506b4fb06 ("bridge: switchdev: Add forward mark support for stacked devices")
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Reported-by: Yotam Gigi <yotamg@mellanox.com>
> Tested-by: Yotam Gigi <yotamg@mellanox.com>
> Reviewed-by: Jiri Pirko <jiri@mellanox.com>

Applied and queued up for -stable, thanks.

^ permalink raw reply

* Re: [PATCH net-next v2 0/4] net: mvpp2: optional PHYs and GoP link irq
From: David Miller @ 2017-09-01 17:09 UTC (permalink / raw)
  To: antoine.tenart
  Cc: andrew, gregory.clement, thomas.petazzoni, nadavh, linux,
	linux-kernel, mw, stefanc, miquel.raynal, netdev
In-Reply-To: <20170901090455.32316-1-antoine.tenart@free-electrons.com>

From: Antoine Tenart <antoine.tenart@free-electrons.com>
Date: Fri,  1 Sep 2017 11:04:51 +0200

> This series aims at making the driver work when no PHY is connected
> between a port and the physical layer and not described as a fixed-phy.
> This is useful for some usecases such as when a switch is connected
> directly to the serdes lanes. It can also be used for SFP ports on the
> 7k-db and 8k-db while waiting for the phylink support to land in (which
> should be part of another series).
> 
> This series makes the phy optional in the PPv2 driver, and then adds
> the support for the GoP port link interrupt to handle link status
> changes on such ports.
> 
> This was tested using the SFP ports on the 7k-db and 8k-db boards.

Series applied.

^ permalink raw reply

* Re: [PATCH net-next 2/2] flow_dissector: Add limits for encapsulation and EH
From: Hannes Frederic Sowa @ 2017-09-01 17:05 UTC (permalink / raw)
  To: Tom Herbert
  Cc: Tom Herbert, David S . Miller, Linux Kernel Network Developers,
	alex.popov
In-Reply-To: <CALx6S37304j28=TdDYaj8dS6G=bhjzMkaVsieCcK3hLWNSVKzA@mail.gmail.com>

Tom Herbert <tom@herbertland.com> writes:

> On Fri, Sep 1, 2017 at 9:35 AM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
>> Hello Tom,
>>
>> Tom Herbert <tom@quantonium.net> writes:
>>
>>> On Fri, Sep 1, 2017 at 6:32 AM, Hannes Frederic Sowa
>>> <hannes@stressinduktion.org> wrote:
>>>> Tom Herbert <tom@quantonium.net> writes:
>>>>
>>>>> In flow dissector there are no limits to the number of nested
>>>>> encapsulations that might be dissected which makes for a nice DOS
>>>>> attack. This patch limits for dissecting nested encapsulations
>>>>> as well as for dissecting over extension headers.
>>>>
>>>> I was actually more referring to your patch, because the flow dissector
>>>> right now is not stack recursive. Your changes would make it doing
>>>> recursion on the stack.
>>>
>>> I don't believe those patches had any recursion.
>>
>> I was wrong with stack recursion, you handle that using the
>> FLOW_DISSECT_RET_PROTO_AGAIN return value thus leaving the stack frame
>> again, sorry.
>>
>> But otherwise the walk would be unlimited (based on the packet size) in
>> your first patchset, correct? See this malicious example:
>>
>> | IP1 | UDP1 | VXLAN1 | Ethernet | IP2 | UDP2 | VXLAN2 | ...
>>
> Without the limits patch I subsequently proposed, yes. However, this
> is true for all the other encapsulations anyway; there's is nothing
> unique about UDP encapsulations in this regard (hence with the limit
> patch should generally apply to all encapsulations).

I used this example to show its possible security implications. Other
encaps definitely have the same problems.

>> where IP1 == IP2, UDP1 == UDP2 and VXLAN1 != VXLAN2?
>>
>> Notice that because IP1 == IP2 and UDP1 == UDP2 it seems to me it would
>> hit the same socket again. We would be prone to overwrite vxlan id 1
>> with vxlan id 2 in the key thus the key would be malicious and traffic
>> could be injected into other tenant networks, if the encapsulated
>> packets within VXLAN1 could be generated by a malicious user?
>>
> This is why flow dissection is not an authoritative parsing of the
> packet. It can be wrong or misleading because it doesn't have all the
> context, doesn't necessarily parse the whole chain, and only returns
> one set of information (for instance only one pair of IP addresses
> when there may be more in a packet). It's just a best effort mechanism
> that is great for computing a hash for instance. If someone is
> steering a packet to a VM based on the output of flow dissector that
> is a bug; the only correct way to do this is to go through the normal
> receive protocol processing path.

I think it must be agreed upon what flow dissector is. Especially I am
concerned about its use in cls_flower (or vice versa this patch).

>> I was actually not concerned about the "recursion" but merely about
>> updating the values to the innermost values.
>>
> See my previous comment about use STOP_AT_ENCAP.

For an authorative parser, for which it gets used in flower right now
STOP_AT_ENCAP might not make too much sense, because it might want to
look at one additional level of encapsulation information. But if you
don't consider the dissector to be an authorative parser for packets, it
would be okay.

Btw., I fear this recursion problem exists right now also with flower's
use of lwt.

[...]

Thanks,
Hannes

^ permalink raw reply

* Re: [patch net-next 0/2] mlxsw: spectrum_router: Couple of fixes
From: David Miller @ 2017-09-01 17:01 UTC (permalink / raw)
  To: jiri; +Cc: netdev, idosch, mlxsw
In-Reply-To: <20170901085856.2286-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@resnulli.us>
Date: Fri,  1 Sep 2017 10:58:54 +0200

> From: Jiri Pirko <jiri@mellanox.com>
> 
> Ido Schimmel (2):
>   mlxsw: spectrum_router: Trap packets hitting anycast routes
>   mlxsw: spectrum_router: Set abort trap in all virtual routers

Series applied.

^ permalink raw reply

* Re: [patch net] mlxsw: spectrum: Forbid linking to devices that have uppers
From: David Miller @ 2017-09-01 17:00 UTC (permalink / raw)
  To: jiri; +Cc: netdev, idosch, nogahf, mlxsw
In-Reply-To: <20170901085231.1396-1-jiri@resnulli.us>

From: Jiri Pirko <jiri@resnulli.us>
Date: Fri,  1 Sep 2017 10:52:31 +0200

> From: Ido Schimmel <idosch@mellanox.com>
> 
> The mlxsw driver relies on NETDEV_CHANGEUPPER events to configure the
> device in case a port is enslaved to a master netdev such as bridge or
> bond.
> 
> Since the driver ignores events unrelated to its ports and their
> uppers, it's possible to engineer situations in which the device's data
> path differs from the kernel's.
> 
> One example to such a situation is when a port is enslaved to a bond
> that is already enslaved to a bridge. When the bond was enslaved the
> driver ignored the event - as the bond wasn't one of its uppers - and
> therefore a bridge port instance isn't created in the device.
> 
> Until such configurations are supported forbid them by checking that the
> upper device doesn't have uppers of its own.
> 
> Fixes: 0d65fc13042f ("mlxsw: spectrum: Implement LAG port join/leave")
> Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> Reported-by: Nogah Frankel <nogahf@mellanox.com>
> Tested-by: Nogah Frankel <nogahf@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>

Applied and queued up for -stable, thanks.

^ permalink raw reply

* Re: [PATCH net-next 0/3] bpf: Improve LRU map lookup performance
From: David Miller @ 2017-09-01 16:57 UTC (permalink / raw)
  To: kafai; +Cc: netdev, ast, daniel, kernel-team
In-Reply-To: <20170901062713.1842249-1-kafai@fb.com>

From: Martin KaFai Lau <kafai@fb.com>
Date: Thu, 31 Aug 2017 23:27:10 -0700

> This patchset improves the lookup performance of the LRU map.
> Please see individual patch for details.

Series applied, thanks Martin.

^ permalink raw reply

* Re: [iproute PATCH 50/51] Check user supplied interface name lengths
From: Phil Sutter @ 2017-09-01 16:56 UTC (permalink / raw)
  To: Stephen Hemminger, netdev
In-Reply-To: <20170815165132.GI16375@orbyte.nwl.cc>

Hi Stephen,

On Tue, Aug 15, 2017 at 06:51:32PM +0200, Phil Sutter wrote:
> On Tue, Aug 15, 2017 at 09:09:45AM -0700, Stephen Hemminger wrote:
> > On Sat, 12 Aug 2017 14:05:09 +0200
> > Phil Sutter <phil@nwl.cc> wrote:
> > 
> > > +void assert_valid_dev_name(const char *, const char *);
> > 
> > Not a fan of long function names.
> > “I have only made this letter longer because I have not had the time to make it shorter."
> 
> :)
> 
> > Maybe just add a new function addattr_ifname() and add the checking there?
> 
> It is not only about netlink attributes - these are in fact
> unproblematic, since they allow for interface names longer than
> IFNAMSIZ-1 and the kernel will then reject. The length check has to
> happen before copying into any IFNAMSIZ-sized buffer takes place (e.g.
> ifr_name of struct ifreq). Logically I would prefer to perform the
> checks right at the point user input parsing takes place since it
> belongs there.
> 
> I could rename the function to 'ifname_valid' instead? I liked having
> 'assert' in the name though, since the function calls exit() on error.

Any thoughts on this?

Cheers, Phil

^ permalink raw reply

* Re: pull request (net-next): ipsec-next 2017-09-01
From: David Miller @ 2017-09-01 16:54 UTC (permalink / raw)
  To: steffen.klassert; +Cc: herbert, netdev
In-Reply-To: <1504251046-17106-1-git-send-email-steffen.klassert@secunet.com>

From: Steffen Klassert <steffen.klassert@secunet.com>
Date: Fri, 1 Sep 2017 09:30:44 +0200

> This should be the last ipsec-next pull request for this
> release cycle:
> 
> 1) Support netdevice ESP trailer removal when decryption
>    is offloaded. From Yossi Kuperman.
> 
> 2) Fix overwritten return value of copy_sec_ctx().
> 
> Please pull or let me know if there are problems.

Pulled, thanks Steffen.

^ permalink raw reply

* [PATCH net-next 2/4] bpf: add a test case for helper bpf_perf_read_counter_time
From: Yonghong Song @ 2017-09-01 16:53 UTC (permalink / raw)
  To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170901165357.465121-1-yhs@fb.com>

The bpf sample program tracex6 is enhanced to use the new
helper to read enabled/running time.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 samples/bpf/tracex6_kern.c                | 26 ++++++++++++++++++++++++++
 samples/bpf/tracex6_user.c                | 13 ++++++++++++-
 tools/testing/selftests/bpf/bpf_helpers.h |  4 ++++
 3 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/samples/bpf/tracex6_kern.c b/samples/bpf/tracex6_kern.c
index e7d1803..46acfef 100644
--- a/samples/bpf/tracex6_kern.c
+++ b/samples/bpf/tracex6_kern.c
@@ -15,6 +15,12 @@ struct bpf_map_def SEC("maps") values = {
 	.value_size = sizeof(u64),
 	.max_entries = 64,
 };
+struct bpf_map_def SEC("maps") values2 = {
+	.type = BPF_MAP_TYPE_HASH,
+	.key_size = sizeof(int),
+	.value_size = sizeof(struct bpf_perf_counter_time),
+	.max_entries = 64,
+};
 
 SEC("kprobe/htab_map_get_next_key")
 int bpf_prog1(struct pt_regs *ctx)
@@ -37,5 +43,25 @@ int bpf_prog1(struct pt_regs *ctx)
 	return 0;
 }
 
+SEC("kprobe/htab_map_lookup_elem")
+int bpf_prog2(struct pt_regs *ctx)
+{
+	u32 key = bpf_get_smp_processor_id();
+	struct bpf_perf_counter_time *val, buf;
+	int error;
+
+	error = bpf_perf_read_counter_time(&counters, key, &buf, sizeof(buf));
+	if (error)
+		return 0;
+
+	val = bpf_map_lookup_elem(&values2, &key);
+	if (val)
+		*val = buf;
+	else
+		bpf_map_update_elem(&values2, &key, &buf, BPF_NOEXIST);
+
+	return 0;
+}
+
 char _license[] SEC("license") = "GPL";
 u32 _version SEC("version") = LINUX_VERSION_CODE;
diff --git a/samples/bpf/tracex6_user.c b/samples/bpf/tracex6_user.c
index a05a99a..2a0c5d8 100644
--- a/samples/bpf/tracex6_user.c
+++ b/samples/bpf/tracex6_user.c
@@ -22,6 +22,7 @@
 
 static void check_on_cpu(int cpu, struct perf_event_attr *attr)
 {
+	struct bpf_perf_counter_time value2;
 	int pmu_fd, error = 0;
 	cpu_set_t set;
 	__u64 value;
@@ -46,8 +47,18 @@ static void check_on_cpu(int cpu, struct perf_event_attr *attr)
 		fprintf(stderr, "Value missing for CPU %d\n", cpu);
 		error = 1;
 		goto on_exit;
+	} else {
+		fprintf(stderr, "CPU %d: %llu\n", cpu, value);
+	}
+	/* The above bpf_map_lookup_elem should trigger the second kprobe */
+	if (bpf_map_lookup_elem(map_fd[2], &cpu, &value2)) {
+		fprintf(stderr, "Value2 missing for CPU %d\n", cpu);
+		error = 1;
+		goto on_exit;
+	} else {
+		fprintf(stderr, "CPU %d: counter: %llu, enabled: %llu, running: %llu\n", cpu,
+			value2.counter, value2.time.enabled, value2.time.running);
 	}
-	fprintf(stderr, "CPU %d: %llu\n", cpu, value);
 
 on_exit:
 	assert(bpf_map_delete_elem(map_fd[0], &cpu) == 0 || error);
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index 36fb916..fe41852 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -70,6 +70,10 @@ static int (*bpf_sk_redirect_map)(void *map, int key, int flags) =
 static int (*bpf_sock_map_update)(void *map, void *key, void *value,
 				  unsigned long long flags) =
 	(void *) BPF_FUNC_sock_map_update;
+static int (*bpf_perf_read_counter_time)(void *map, unsigned long long flags,
+				       void *counter_time_buf,
+				       unsigned int buf_size) =
+	(void *) BPF_FUNC_perf_read_counter_time;
 
 
 /* llvm builtin functions that eBPF C program may use to
-- 
2.9.5

^ permalink raw reply related

* [PATCH net-next 3/4] bpf: add helper bpf_perf_prog_read_time
From: Yonghong Song @ 2017-09-01 16:53 UTC (permalink / raw)
  To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170901165357.465121-1-yhs@fb.com>

This patch adds helper bpf_perf_prog_read_time for perf event based bpf
programs, to read event enabled/running time.
The enabled/running time is accumulated since the perf event open.

The typical use case for perf event based bpf program is to attach itself
to a single event. In such cases, if it is desirable to get scaling factor
between two bpf invocations, users can can save the time values in a map,
and use the value from the map and the current value to calculate
the scaling factor.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 include/linux/perf_event.h |  1 +
 include/uapi/linux/bpf.h   |  8 ++++++++
 kernel/events/core.c       |  1 +
 kernel/trace/bpf_trace.c   | 24 ++++++++++++++++++++++++
 4 files changed, 34 insertions(+)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 7fd5e94..92955fc 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -821,6 +821,7 @@ struct perf_output_handle {
 struct bpf_perf_event_data_kern {
 	struct pt_regs *regs;
 	struct perf_sample_data *data;
+	struct perf_event *event;
 };
 
 #ifdef CONFIG_CGROUP_PERF
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 9c23bef..1ae55c8 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -590,6 +590,13 @@ union bpf_attr {
  *     @counter_time_buf: buf to fill
  *     @buf_size: size of the counter_time_buf
  *     Return: 0 on success or negative error code
+ *
+ * int bpf_perf_prog_read_time(ctx, time_buf, buf_size)
+ *     Read perf event enabled and running time
+ *     @ctx: pointer to ctx
+ *     @time_buf: buf to fill
+ *     @buf_size: size of the time_buf
+ *     Return : 0 on success or negative error code
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -647,6 +654,7 @@ union bpf_attr {
 	FN(sk_redirect_map),		\
 	FN(sock_map_update),		\
 	FN(perf_read_counter_time),		\
+	FN(perf_prog_read_time),		\
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
diff --git a/kernel/events/core.c b/kernel/events/core.c
index ef5c7fb..1f16f1f 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -8019,6 +8019,7 @@ static void bpf_overflow_handler(struct perf_event *event,
 	struct bpf_perf_event_data_kern ctx = {
 		.data = data,
 		.regs = regs,
+		.event = event,
 	};
 	int ret = 0;
 
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index b807b1a..e97620a 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -608,6 +608,19 @@ BPF_CALL_3(bpf_get_stackid_tp, void *, tp_buff, struct bpf_map *, map,
 			       flags, 0, 0);
 }
 
+BPF_CALL_3(bpf_perf_prog_read_time_tp, void *, ctx, struct bpf_perf_time *,
+	time_buf, u32, size)
+{
+	struct bpf_perf_event_data_kern *kctx = (struct bpf_perf_event_data_kern *)ctx;
+	u64 now;
+
+	if (size != sizeof(struct bpf_perf_time))
+		return -EINVAL;
+
+	calc_timer_values(kctx->event, &now, &time_buf->enabled, &time_buf->running);
+	return 0;
+}
+
 static const struct bpf_func_proto bpf_get_stackid_proto_tp = {
 	.func		= bpf_get_stackid_tp,
 	.gpl_only	= true,
@@ -617,6 +630,15 @@ static const struct bpf_func_proto bpf_get_stackid_proto_tp = {
 	.arg3_type	= ARG_ANYTHING,
 };
 
+static const struct bpf_func_proto bpf_perf_prog_read_time_proto_tp = {
+         .func           = bpf_perf_prog_read_time_tp,
+         .gpl_only       = true,
+         .ret_type       = RET_INTEGER,
+         .arg1_type      = ARG_PTR_TO_CTX,
+         .arg2_type      = ARG_PTR_TO_UNINIT_MEM,
+         .arg3_type      = ARG_CONST_SIZE,
+};
+
 static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id)
 {
 	switch (func_id) {
@@ -624,6 +646,8 @@ static const struct bpf_func_proto *tp_prog_func_proto(enum bpf_func_id func_id)
 		return &bpf_perf_event_output_proto_tp;
 	case BPF_FUNC_get_stackid:
 		return &bpf_get_stackid_proto_tp;
+	case BPF_FUNC_perf_prog_read_time:
+		return &bpf_perf_prog_read_time_proto_tp;
 	default:
 		return tracing_func_proto(func_id);
 	}
-- 
2.9.5

^ permalink raw reply related

* [PATCH net-next 4/4] bpf: add a test case for helper bpf_perf_prog_read_time
From: Yonghong Song @ 2017-09-01 16:53 UTC (permalink / raw)
  To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170901165357.465121-1-yhs@fb.com>

The bpf sample program trace_event is enhanced to use the new
helper to print out enabled/running time.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 samples/bpf/trace_event_kern.c            | 5 +++++
 tools/testing/selftests/bpf/bpf_helpers.h | 3 +++
 2 files changed, 8 insertions(+)

diff --git a/samples/bpf/trace_event_kern.c b/samples/bpf/trace_event_kern.c
index 41b6115..f372660 100644
--- a/samples/bpf/trace_event_kern.c
+++ b/samples/bpf/trace_event_kern.c
@@ -37,8 +37,10 @@ struct bpf_map_def SEC("maps") stackmap = {
 SEC("perf_event")
 int bpf_prog1(struct bpf_perf_event_data *ctx)
 {
+	char time_fmt[] = "Time Enabled: %lld, Time Running: %lld";
 	char fmt[] = "CPU-%d period %lld ip %llx";
 	u32 cpu = bpf_get_smp_processor_id();
+	struct bpf_perf_time time_buf;
 	struct key_t key;
 	u64 *val, one = 1;
 
@@ -54,6 +56,9 @@ int bpf_prog1(struct bpf_perf_event_data *ctx)
 		return 0;
 	}
 
+	bpf_perf_prog_read_time(ctx, (void *)&time_buf, sizeof(struct bpf_perf_time));
+	bpf_trace_printk(time_fmt, sizeof(time_fmt), time_buf.enabled, time_buf.running);
+
 	val = bpf_map_lookup_elem(&counts, &key);
 	if (val)
 		(*val)++;
diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
index fe41852..ddad690 100644
--- a/tools/testing/selftests/bpf/bpf_helpers.h
+++ b/tools/testing/selftests/bpf/bpf_helpers.h
@@ -74,6 +74,9 @@ static int (*bpf_perf_read_counter_time)(void *map, unsigned long long flags,
 				       void *counter_time_buf,
 				       unsigned int buf_size) =
 	(void *) BPF_FUNC_perf_read_counter_time;
+static int (*bpf_perf_prog_read_time)(void *ctx, void *time_buf,
+				      unsigned int size) =
+	(void *) BPF_FUNC_perf_prog_read_time;
 
 
 /* llvm builtin functions that eBPF C program may use to
-- 
2.9.5

^ permalink raw reply related

* [PATCH net-next 1/4] bpf: add helper bpf_perf_read_counter_time for perf event array map
From: Yonghong Song @ 2017-09-01 16:53 UTC (permalink / raw)
  To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team
In-Reply-To: <20170901165357.465121-1-yhs@fb.com>

Hardware pmu counters are limited resources. When there are more
pmu based perf events opened than available counters, kernel will
multiplex these events so each event gets certain percentage
(but not 100%) of the pmu time. In case that multiplexing happens,
the number of samples or counter value will not reflect the
case compared to no multiplexing. This makes comparison between
different runs difficult.

Typically, the number of samples or counter value should be
normalized before comparing to other experiments. The typical
normalization is done like:
  normalized_num_samples = num_samples * time_enabled / time_running
  normalized_counter_value = counter_value * time_enabled / time_running
where time_enabled is the time enabled for event and time_running is
the time running for event since last normalization.

This patch adds helper bpf_perf_read_counter_time for kprobed based perf
event array map, to read perf counter and enabled/running time.
The enabled/running time is accumulated since the perf event open.
To achieve scaling factor between two bpf invocations, users
can can use cpu_id as the key (which is typical for perf array usage model)
to remember the previous value and do the calculation inside the
bpf program.

Signed-off-by: Yonghong Song <yhs@fb.com>
---
 include/linux/perf_event.h |  2 ++
 include/uapi/linux/bpf.h   | 21 +++++++++++++++++++-
 kernel/bpf/verifier.c      |  4 +++-
 kernel/events/core.c       |  2 +-
 kernel/trace/bpf_trace.c   | 49 ++++++++++++++++++++++++++++++++++++++++++----
 5 files changed, 71 insertions(+), 7 deletions(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index b14095b..7fd5e94 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -901,6 +901,8 @@ extern void perf_pmu_migrate_context(struct pmu *pmu,
 int perf_event_read_local(struct perf_event *event, u64 *value);
 extern u64 perf_event_read_value(struct perf_event *event,
 				 u64 *enabled, u64 *running);
+extern void calc_timer_values(struct perf_event *event, u64 *now,
+         u64 *enabled, u64 *running);
 
 
 struct perf_sample_data {
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index ba848b7..9c23bef 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -582,6 +582,14 @@ union bpf_attr {
  *	@map: pointer to sockmap to update
  *	@key: key to insert/update sock in map
  *	@flags: same flags as map update elem
+ *
+ * int bpf_perf_read_counter_time(map, flags, counter_time_buf, buf_size)
+ *     read perf event counter value and perf event enabled/running time
+ *     @map: pointer to perf_event_array map
+ *     @flags: index of event in the map or bitmask flags
+ *     @counter_time_buf: buf to fill
+ *     @buf_size: size of the counter_time_buf
+ *     Return: 0 on success or negative error code
  */
 #define __BPF_FUNC_MAPPER(FN)		\
 	FN(unspec),			\
@@ -638,6 +646,7 @@ union bpf_attr {
 	FN(redirect_map),		\
 	FN(sk_redirect_map),		\
 	FN(sock_map_update),		\
+	FN(perf_read_counter_time),		\
 
 /* integer value in 'imm' field of BPF_CALL instruction selects which helper
  * function eBPF program intends to call
@@ -681,7 +690,8 @@ enum bpf_func_id {
 #define BPF_F_ZERO_CSUM_TX		(1ULL << 1)
 #define BPF_F_DONT_FRAGMENT		(1ULL << 2)
 
-/* BPF_FUNC_perf_event_output and BPF_FUNC_perf_event_read flags. */
+/* BPF_FUNC_perf_event_output, BPF_FUNC_perf_event_read and
+ * BPF_FUNC_perf_read_counter_time flags. */
 #define BPF_F_INDEX_MASK		0xffffffffULL
 #define BPF_F_CURRENT_CPU		BPF_F_INDEX_MASK
 /* BPF_FUNC_perf_event_output for sk_buff input context. */
@@ -864,4 +874,13 @@ enum {
 #define TCP_BPF_IW		1001	/* Set TCP initial congestion window */
 #define TCP_BPF_SNDCWND_CLAMP	1002	/* Set sndcwnd_clamp */
 
+struct bpf_perf_time {
+	__u64 enabled;
+	__u64 running;
+};
+struct bpf_perf_counter_time {
+	__u64 counter;
+	struct bpf_perf_time time;
+};
+
 #endif /* _UAPI__LINUX_BPF_H__ */
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index d690c7d..c4d29e3 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -1494,7 +1494,8 @@ static int check_map_func_compatibility(struct bpf_map *map, int func_id)
 		break;
 	case BPF_MAP_TYPE_PERF_EVENT_ARRAY:
 		if (func_id != BPF_FUNC_perf_event_read &&
-		    func_id != BPF_FUNC_perf_event_output)
+		    func_id != BPF_FUNC_perf_event_output &&
+		    func_id != BPF_FUNC_perf_read_counter_time)
 			goto error;
 		break;
 	case BPF_MAP_TYPE_STACK_TRACE:
@@ -1537,6 +1538,7 @@ static int check_map_func_compatibility(struct bpf_map *map, int func_id)
 		break;
 	case BPF_FUNC_perf_event_read:
 	case BPF_FUNC_perf_event_output:
+	case BPF_FUNC_perf_read_counter_time:
 		if (map->map_type != BPF_MAP_TYPE_PERF_EVENT_ARRAY)
 			goto error;
 		break;
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 8c01572..ef5c7fb 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -4883,7 +4883,7 @@ static int perf_event_index(struct perf_event *event)
 	return event->pmu->event_idx(event);
 }
 
-static void calc_timer_values(struct perf_event *event,
+void calc_timer_values(struct perf_event *event,
 				u64 *now,
 				u64 *enabled,
 				u64 *running)
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index dc498b6..b807b1a 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -255,13 +255,13 @@ const struct bpf_func_proto *bpf_get_trace_printk_proto(void)
 	return &bpf_trace_printk_proto;
 }
 
-BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
-{
+static __always_inline int
+get_map_perf_counter(struct bpf_map *map, u64 flags,
+		u64 *value, struct perf_event **pe) {
 	struct bpf_array *array = container_of(map, struct bpf_array, map);
 	unsigned int cpu = smp_processor_id();
 	u64 index = flags & BPF_F_INDEX_MASK;
 	struct bpf_event_entry *ee;
-	u64 value = 0;
 	int err;
 
 	if (unlikely(flags & ~(BPF_F_INDEX_MASK)))
@@ -275,7 +275,19 @@ BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
 	if (!ee)
 		return -ENOENT;
 
-	err = perf_event_read_local(ee->event, &value);
+	err = perf_event_read_local(ee->event, value);
+	if (!err && pe)
+		*pe = ee->event;
+	return err;
+}
+
+
+BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
+{
+	u64 value = 0;
+	int err;
+
+	err = get_map_perf_counter(map, flags, &value, NULL);
 	/*
 	 * this api is ugly since we miss [-22..-2] range of valid
 	 * counter values, but that's uapi
@@ -285,6 +297,23 @@ BPF_CALL_2(bpf_perf_event_read, struct bpf_map *, map, u64, flags)
 	return value;
 }
 
+BPF_CALL_4(bpf_perf_read_counter_time, struct bpf_map *, map, u64, flags,
+	struct bpf_perf_counter_time *, buf, u32, size)
+{
+	struct perf_event *pe;
+	u64 now;
+	int err;
+
+	if (unlikely(size != sizeof(struct bpf_perf_counter_time)))
+		return -EINVAL;
+	err = get_map_perf_counter(map, flags, &buf->counter, &pe);
+	if (err)
+		return err;
+
+	calc_timer_values(pe, &now, &buf->time.enabled, &buf->time.running);
+	return 0;
+}
+
 static const struct bpf_func_proto bpf_perf_event_read_proto = {
 	.func		= bpf_perf_event_read,
 	.gpl_only	= true,
@@ -293,6 +322,16 @@ static const struct bpf_func_proto bpf_perf_event_read_proto = {
 	.arg2_type	= ARG_ANYTHING,
 };
 
+static const struct bpf_func_proto bpf_perf_read_counter_time_proto = {
+	.func		= bpf_perf_read_counter_time,
+	.gpl_only	= true,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_CONST_MAP_PTR,
+	.arg2_type	= ARG_ANYTHING,
+	.arg3_type	= ARG_PTR_TO_UNINIT_MEM,
+	.arg4_type	= ARG_CONST_SIZE,
+};
+
 static DEFINE_PER_CPU(struct perf_sample_data, bpf_sd);
 
 static __always_inline u64
@@ -499,6 +538,8 @@ static const struct bpf_func_proto *kprobe_prog_func_proto(enum bpf_func_id func
 		return &bpf_perf_event_output_proto;
 	case BPF_FUNC_get_stackid:
 		return &bpf_get_stackid_proto;
+	case BPF_FUNC_perf_read_counter_time:
+		return &bpf_perf_read_counter_time_proto;
 	default:
 		return tracing_func_proto(func_id);
 	}
-- 
2.9.5

^ permalink raw reply related

* [PATCH net-next 0/4] bpf: add two helpers to read perf event enabled/running time
From: Yonghong Song @ 2017-09-01 16:53 UTC (permalink / raw)
  To: peterz, rostedt, ast, daniel, netdev; +Cc: kernel-team

Hardware pmu counters are limited resources. When there are more
pmu based perf events opened than available counters, kernel will
multiplex these events so each event gets certain percentage
(but not 100%) of the pmu time. In case that multiplexing happens,
the number of samples or counter value will not reflect the
case compared to no multiplexing. This makes comparison between
different runs difficult.
                                
Typically, the number of samples or counter value should be
normalized before comparing to other experiments. The typical
normalization is done like:
  normalized_num_samples = num_samples * time_enabled / time_running
  normalized_counter_value = counter_value * time_enabled / time_running
where time_enabled is the time enabled for event and time_running is
the time running for event since last normalization.

This patch set implements two helper functions.
The helper bpf_perf_read_counter_time reads counter/time_enabled/time_running
for perf event array map. The helper bpf_perf_prog_read_time read
time_enabled/time_running for bpf prog with type BPF_PROG_TYPE_PERF_EVENT.

Yonghong Song (4):
  bpf: add helper bpf_perf_read_counter_time for perf event array map
  bpf: add a test case for helper bpf_perf_read_counter_time
  bpf: add helper bpf_perf_prog_read_time
  bpf: add a test case for helper bpf_perf_prog_read_time

 include/linux/perf_event.h                |  3 ++
 include/uapi/linux/bpf.h                  | 29 +++++++++++-
 kernel/bpf/verifier.c                     |  4 +-
 kernel/events/core.c                      |  3 +-
 kernel/trace/bpf_trace.c                  | 73 +++++++++++++++++++++++++++++--
 samples/bpf/trace_event_kern.c            |  5 +++
 samples/bpf/tracex6_kern.c                | 26 +++++++++++
 samples/bpf/tracex6_user.c                | 13 +++++-
 tools/testing/selftests/bpf/bpf_helpers.h |  7 +++
 9 files changed, 155 insertions(+), 8 deletions(-)

-- 
2.9.5

^ permalink raw reply

* [iproute PATCH 3/6] Convert harmful calls to strncpy() to strlcpy()
From: Phil Sutter @ 2017-09-01 16:52 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170901165256.21459-1-phil@nwl.cc>

This patch converts spots where manual buffer termination was missing to
strlcpy() since that does what is needed.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 genl/ctrl.c     | 2 +-
 ip/ipvrf.c      | 2 +-
 ip/xfrm_state.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/genl/ctrl.c b/genl/ctrl.c
index 6abd52582d0d3..448988eb90e2b 100644
--- a/genl/ctrl.c
+++ b/genl/ctrl.c
@@ -313,7 +313,7 @@ static int ctrl_list(int cmd, int argc, char **argv)
 
 		if (matches(*argv, "name") == 0) {
 			NEXT_ARG();
-			strncpy(d, *argv, sizeof (d) - 1);
+			strlcpy(d, *argv, sizeof(d));
 			addattr_l(nlh, 128, CTRL_ATTR_FAMILY_NAME,
 				  d, strlen(d) + 1);
 		} else if (matches(*argv, "id") == 0) {
diff --git a/ip/ipvrf.c b/ip/ipvrf.c
index b74c501e0970c..f9277e1e66bdf 100644
--- a/ip/ipvrf.c
+++ b/ip/ipvrf.c
@@ -74,7 +74,7 @@ static int vrf_identify(pid_t pid, char *name, size_t len)
 			if (end)
 				*end = '\0';
 
-			strncpy(name, vrf, len - 1);
+			strlcpy(name, vrf, len);
 			break;
 		}
 	}
diff --git a/ip/xfrm_state.c b/ip/xfrm_state.c
index e11c93bf1c3b5..4483fb8f71d2c 100644
--- a/ip/xfrm_state.c
+++ b/ip/xfrm_state.c
@@ -125,7 +125,7 @@ static int xfrm_algo_parse(struct xfrm_algo *alg, enum xfrm_attr_type_t type,
 	fprintf(stderr, "warning: ALGO-NAME/ALGO-KEYMAT values will be sent to the kernel promiscuously! (verifying them isn't implemented yet)\n");
 #endif
 
-	strncpy(alg->alg_name, name, sizeof(alg->alg_name));
+	strlcpy(alg->alg_name, name, sizeof(alg->alg_name));
 
 	if (slen > 2 && strncmp(key, "0x", 2) == 0) {
 		/* split two chars "0x" from the top */
-- 
2.13.1

^ permalink raw reply related

* Re: [RFC net-next 0/5] TSN: Add qdisc-based config interfaces for traffic shapers
From: Richard Cochran @ 2017-09-01 16:53 UTC (permalink / raw)
  To: Jesus Sanchez-Palencia
  Cc: Vinicius Costa Gomes, netdev, jhs, xiyou.wangcong, jiri,
	intel-wired-lan, andre.guedes, ivan.briano, boon.leong.ong
In-Reply-To: <1519366e-5418-4dda-db00-5bf50a1c67c4@intel.com>

On Fri, Sep 01, 2017 at 09:12:17AM -0700, Jesus Sanchez-Palencia wrote:
> Is it correct to assume you are referring to an interface for Launchtime here?

Yes.

Thanks,
Richard

^ permalink raw reply

* [iproute PATCH 5/6] tc_util: No need to terminate an snprintf'ed buffer
From: Phil Sutter @ 2017-09-01 16:52 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170901165256.21459-1-phil@nwl.cc>

snprintf() won't leave the buffer unterminated, so manually terminating
is not necessary here.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 tc/tc_util.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tc/tc_util.c b/tc/tc_util.c
index 371046839ba9f..50d355046bdad 100644
--- a/tc/tc_util.c
+++ b/tc/tc_util.c
@@ -434,7 +434,6 @@ static const char *action_n2a(int action)
 		return "trap";
 	default:
 		snprintf(buf, 64, "%d", action);
-		buf[63] = '\0';
 		return buf;
 	}
 }
-- 
2.13.1

^ permalink raw reply related

* [iproute PATCH 6/6] lnstat_util: Make sure buffer is NUL-terminated
From: Phil Sutter @ 2017-09-01 16:52 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20170901165256.21459-1-phil@nwl.cc>

Can't use strlcpy() here since lnstat is not linked against libutil.

While being at it, fix coding style in that chunk as well.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 misc/lnstat_util.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/misc/lnstat_util.c b/misc/lnstat_util.c
index ec19238c24b94..c2dc42ec1ff12 100644
--- a/misc/lnstat_util.c
+++ b/misc/lnstat_util.c
@@ -150,7 +150,8 @@ static int lnstat_scan_compat_rtstat_fields(struct lnstat_file *lf)
 {
 	char buf[FGETS_BUF_SIZE];
 
-	strncpy(buf, RTSTAT_COMPAT_LINE, sizeof(buf)-1);
+	strncpy(buf, RTSTAT_COMPAT_LINE, sizeof(buf) - 1);
+	buf[sizeof(buf) - 1] = '\0';
 
 	return __lnstat_scan_fields(lf, buf);
 }
-- 
2.13.1

^ permalink raw reply related


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