* Re: [PATCH bpf-next 2/3] bpf: emit RECORD_MMAP events for bpf prog load/unload
From: Alexei Starovoitov @ 2018-09-20 0:59 UTC (permalink / raw)
To: Song Liu, Alexei Starovoitov
Cc: David S . Miller, daniel@iogearbox.net, peterz@infradead.org,
acme@kernel.org, netdev@vger.kernel.org, Kernel Team
In-Reply-To: <3FB6EC86-700C-4C80-A467-55B6753B07FE@fb.com>
On 9/19/18 4:44 PM, Song Liu wrote:
>
>
>> On Sep 19, 2018, at 3:39 PM, Alexei Starovoitov <ast@kernel.org> wrote:
>>
>> use perf_event_mmap_bpf_prog() helper to notify user space
>> about JITed bpf programs.
>> Use RECORD_MMAP perf event to tell user space where JITed bpf program was loaded.
>> Use empty program name as unload indication.
>>
>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>> ---
>> kernel/bpf/core.c | 22 ++++++++++++++++++++--
>> 1 file changed, 20 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
>> index 3f5bf1af0826..ddf11fdafd36 100644
>> --- a/kernel/bpf/core.c
>> +++ b/kernel/bpf/core.c
>> @@ -384,7 +384,7 @@ bpf_get_prog_addr_region(const struct bpf_prog *prog,
>> *symbol_end = addr + hdr->pages * PAGE_SIZE;
>> }
>>
>> -static void bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
>> +static char *bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
>> {
>> const char *end = sym + KSYM_NAME_LEN;
>>
>> @@ -402,9 +402,10 @@ static void bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
>> sym += snprintf(sym, KSYM_NAME_LEN, "bpf_prog_");
>> sym = bin2hex(sym, prog->tag, sizeof(prog->tag));
>> if (prog->aux->name[0])
>> - snprintf(sym, (size_t)(end - sym), "_%s", prog->aux->name);
>> + sym += snprintf(sym, (size_t)(end - sym), "_%s", prog->aux->name);
>> else
>> *sym = 0;
>> + return sym;
>> }
>>
>> static __always_inline unsigned long
>> @@ -480,23 +481,40 @@ static bool bpf_prog_kallsyms_verify_off(const struct bpf_prog *fp)
>>
>> void bpf_prog_kallsyms_add(struct bpf_prog *fp)
>> {
>> + unsigned long symbol_start, symbol_end;
>> + char buf[KSYM_NAME_LEN], *sym;
>> +
>> if (!bpf_prog_kallsyms_candidate(fp) ||
>> !capable(CAP_SYS_ADMIN))
>> return;
>>
>> + bpf_get_prog_addr_region(fp, &symbol_start, &symbol_end);
>> + sym = bpf_get_prog_name(fp, buf);
>> + sym++; /* sym - buf is the length of the name including trailing 0 */
>> + while (!IS_ALIGNED(sym - buf, sizeof(u64)))
>> + *sym++ = 0;
>
> nit: This logic feels a little weird to me. How about we wrap the extra logic
> in a separate function:
>
> size_t bpf_get_prog_name_u64_aligned(const struct bpf_prog fp, char *buf)
probably bpf_get_prog_name_u64_padded() ?
would be cleaner indeed.
Will send v2 once Peter and Arnaldo provide their feedback.
Thanks for reviewing!
^ permalink raw reply
* Re: [PATCH bpf-next 1/3] perf/core: introduce perf_event_mmap_bpf_prog
From: Alexei Starovoitov @ 2018-09-20 0:53 UTC (permalink / raw)
To: Song Liu, Alexei Starovoitov
Cc: David S . Miller, daniel@iogearbox.net, peterz@infradead.org,
acme@kernel.org, netdev@vger.kernel.org, Kernel Team
In-Reply-To: <4DBB524C-09B1-46BD-A56E-1E372E87F2E8@fb.com>
On 9/19/18 4:30 PM, Song Liu wrote:
>
>
>> On Sep 19, 2018, at 3:39 PM, Alexei Starovoitov <ast@kernel.org> wrote:
>>
>> introduce perf_event_mmap_bpf_prog() helper to emit RECORD_MMAP events
>> into perf ring buffer.
>> It's used by bpf load/unload logic to notify user space of addresses
>> and names of JITed bpf programs.
>>
>> Note that event->mmap.pid == -1 is an existing indicator of kernel event.
>> In addition use event->mmap.tid == BPF_FS_MAGIC to indicate bpf related
>> RECORD_MMAP event.
>>
>> Alternatively it's possible to introduce new 'enum perf_event_type' command
>> specificially for bpf prog load/unload, but existing RECORD_MMAP
>> is very close, so the choice made by this patch is to extend it.
>>
>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>
> Acked-by: Song Liu <songliubraving@fb.com>
>
> I guess we should also use this for kernel modules load/unload?
yes. that's possible.
There is similar issue with modules today that get unloaded
before 'perf report'.
Synthetic record_mmap for modules that perf emits into perf.data
has filename==module_name. It solves typical use case though.
We can extend record_mmap further and let kernel emit them
for module load/unload.
Some new magic for 'tid' would be necessary.
^ permalink raw reply
* Re: [PATCH] net: macb: Clean 64b dma addresses if they are not detected
From: Michal Simek @ 2018-09-20 6:23 UTC (permalink / raw)
To: Edgar E. Iglesias, Michal Simek
Cc: linux-kernel, monstr, Edgar E. Iglesias, David S. Miller, netdev,
u-boot, Joe Hershberger, Nicolas Ferre
In-Reply-To: <20180919180806.3tps6yukhm3ry43i@toto>
On 19.9.2018 20:08, Edgar E. Iglesias wrote:
> On Wed, Sep 19, 2018 at 06:08:18PM +0200, Michal Simek wrote:
>> Clear ADDR64 dma bit in DMACFG register in case that HW_DMA_CAP_64B
>> is not detected on 64bit system.
>> The issue was observed when bootloader(u-boot) does not check macb
>> feature at DCFG6 register (DAW64_OFFSET) and enabling 64bit dma support
>> by default. Then macb driver is reading DMACFG register back and only
>> adding 64bit dma configuration but not cleaning it out.
>>
>> This is also align with other features which are also cleared if they are not
>> present.
>
> Hi Michal,
>
>>
>> Signed-off-by: Michal Simek <michal.simek@xilinx.com>
>> ---
>>
>> drivers/net/ethernet/cadence/macb_main.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
>> index 16e4ef7d7185..79707dff3f13 100644
>> --- a/drivers/net/ethernet/cadence/macb_main.c
>> +++ b/drivers/net/ethernet/cadence/macb_main.c
>> @@ -2163,6 +2163,8 @@ static void macb_configure_dma(struct macb *bp)
>> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
>> if (bp->hw_dma_cap & HW_DMA_CAP_64B)
>> dmacfg |= GEM_BIT(ADDR64);
>> + else
>> + dmacfg &= ~GEM_BIT(ADDR64);
>> #endif
>
> I think you might want to do this clearing outside of the #ifdef.
> If CONFIG_ARCH_DMA_ADDR_T_64BIT is not defined, we'd want to make
> sure the ADDR64 is cleared. E.g something like:
>
> dmacfg &= ~GEM_BIT(ADDR64);
> #ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
> if (bp->hw_dma_cap & HW_DMA_CAP_64B)
> dmacfg |= GEM_BIT(ADDR64);
> #endif
>
>
> Same thing for the USE_HWSTAMP/PTP flags below.
Origin patch, which introduce this read with mask,
macfg = gem_readl(bp, DMACFG) & ~GEM_BF(RXBS, -1L);
was done in 2011 and from that time this function was extended a little
bit. I am even not quite sure if make sense to read this reg and apply
setting on the top of it.
Nicolas: Isn't it better simply compose that reg from scratch?
Thanks,
Michal
^ permalink raw reply
* Re: [PATCH] isdn/hisax: Remove unnecessary parenthesis
From: David Miller @ 2018-09-20 6:11 UTC (permalink / raw)
To: natechancellor; +Cc: isdn, netdev, linux-kernel, ndesaulniers
In-Reply-To: <20180920040523.21671-1-natechancellor@gmail.com>
From: Nathan Chancellor <natechancellor@gmail.com>
Date: Wed, 19 Sep 2018 21:05:23 -0700
> Clang warns when more than one set of parentheses are used for single
> conditional statements:
>
> drivers/isdn/hisax/w6692.c:627:30: warning: equality comparison with
> extraneous parentheses [-Wparentheses-equality]
> if ((cs->dc.w6692.ph_state == W_L1IND_DRD)) {
> ~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
> drivers/isdn/hisax/w6692.c:627:30: note: remove extraneous parentheses
> around the comparison to silence this warning
> if ((cs->dc.w6692.ph_state == W_L1IND_DRD)) {
> ~ ^ ~
> drivers/isdn/hisax/w6692.c:627:30: note: use '=' to turn this equality
> comparison into an assignment
> if ((cs->dc.w6692.ph_state == W_L1IND_DRD)) {
> ^~
> =
> 1 warning generated.
>
> Remove the parentheses to silence this warning.
>
> Reported-by: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] atl1: remove set but not used variable 'advertising'
From: David Miller @ 2018-09-20 6:10 UTC (permalink / raw)
To: yuehaibing
Cc: jcliburn, chris.snook, keescook, zhongjiang, netdev,
kernel-janitors, linux-kernel
In-Reply-To: <1537411290-5132-1-git-send-email-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Thu, 20 Sep 2018 02:41:30 +0000
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/ethernet/atheros/atlx/atl1.c: In function 'atl1_set_link_ksettings':
> drivers/net/ethernet/atheros/atlx/atl1.c:3280:6: warning:
> variable 'advertising' set but not used [-Wunused-but-set-variable]
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] e1000: remove set but not used variable 'txb2b'
From: David Miller @ 2018-09-20 6:09 UTC (permalink / raw)
To: yuehaibing
Cc: jeffrey.t.kirsher, intel-wired-lan, netdev, kernel-janitors,
linux-kernel
In-Reply-To: <1537409078-2374-1-git-send-email-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Thu, 20 Sep 2018 02:04:38 +0000
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/ethernet/intel/e1000/e1000_main.c: In function 'e1000_watchdog':
> drivers/net/ethernet/intel/e1000/e1000_main.c:2436:9: warning:
> variable 'txb2b' set but not used [-Wunused-but-set-variable]
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] net: toshiba: remove a redundant local variable 'index_specified'
From: David Miller @ 2018-09-20 6:09 UTC (permalink / raw)
To: zhongjiang; +Cc: benh, paulus, mpe, netdev, linux-kernel
In-Reply-To: <1537408565-22315-1-git-send-email-zhongjiang@huawei.com>
From: zhong jiang <zhongjiang@huawei.com>
Date: Thu, 20 Sep 2018 09:56:05 +0800
> The local variable 'index_specified' is never used after being assigned.
> hence it should be redundant adn can be removed.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
Applied.
^ permalink raw reply
* [PATCH net] af_key: free SKBs under RCU protection
From: Sean Tranchetti @ 2018-09-20 0:18 UTC (permalink / raw)
To: netdev, steffen.klassert; +Cc: Sean Tranchetti
pfkey_broadcast() can make calls to pfkey_broadcast_one() which
will clone or copy the passed in SKB. This new SKB will be assigned
the sock_rfree() function as its destructor, which requires that
the socket reference the SKB contains is valid when the SKB is freed.
If this SKB is ever passed to pfkey_broadcast() again by some other
function (such as pkfey_dump() or pfkey_promisc) it will then be
freed there. However, since this free occurs outside of RCU protection,
it is possible that userspace could close the socket and trigger
pfkey_release() to free the socket before sock_rfree() can run, creating
the following race condition:
1: An SKB belonging to the pfkey socket is passed to pfkey_broadcast().
It performs the broadcast to any other sockets, and calls
rcu_read_unlock(), but does not yet reach kfree_skb().
2: Userspace closes the socket, triggering pfkey_realse(). Since no one
holds the RCU lock, synchronize_rcu() returns and it is allowed to
continue. It calls sock_put() to free the socket.
3: pfkey_broadcast() now calls kfree_skb() on the original SKB it was
passed, triggering a call to sock_rfree(). This function now accesses
the freed struct sock * via skb->sk, and attempts to update invalid
memory.
By ensuring that the pfkey_broadcast() also frees the SKBs while it holds
the RCU lock, we can ensure that the socket will remain valid when the SKB
is freed, avoiding crashes like the following:
Unable to handle kernel paging request at virtual address 6b6b6b6b6b6c4b
[006b6b6b6b6b6c4b] address between user and kernel address ranges
Internal error: Oops: 96000004 [#1] PREEMPT SMP
task: fffffff78f65b380 task.stack: ffffff8049a88000
pc : sock_rfree+0x38/0x6c
lr : skb_release_head_state+0x6c/0xcc
Process repro (pid: 7117, stack limit = 0xffffff8049a88000)
Call trace:
sock_rfree+0x38/0x6c
skb_release_head_state+0x6c/0xcc
skb_release_all+0x1c/0x38
__kfree_skb+0x1c/0x30
kfree_skb+0xd0/0xf4
pfkey_broadcast+0x14c/0x18c
pfkey_sendmsg+0x1d8/0x408
sock_sendmsg+0x44/0x60
___sys_sendmsg+0x1d0/0x2a8
__sys_sendmsg+0x64/0xb4
SyS_sendmsg+0x34/0x4c
el0_svc_naked+0x34/0x38
Kernel panic - not syncing: Fatal exception
Signed-off-by: Sean Tranchetti <stranche@codeaurora.org>
---
net/key/af_key.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/key/af_key.c b/net/key/af_key.c
index 9d61266..dd257c7 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -275,13 +275,13 @@ static int pfkey_broadcast(struct sk_buff *skb, gfp_t allocation,
if ((broadcast_flags & BROADCAST_REGISTERED) && err)
err = err2;
}
- rcu_read_unlock();
if (one_sk != NULL)
err = pfkey_broadcast_one(skb, &skb2, allocation, one_sk);
kfree_skb(skb2);
kfree_skb(skb);
+ rcu_read_unlock();
return err;
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] net: phy: don't reschedule state machine when PHY is halted
From: Florian Fainelli @ 2018-09-20 0:04 UTC (permalink / raw)
To: Heiner Kallweit, Andrew Lunn, David Miller; +Cc: netdev@vger.kernel.org
In-Reply-To: <df6d5770-7508-46f8-106f-6d00f8e1aac8@gmail.com>
On 09/18/2018 01:17 PM, Heiner Kallweit wrote:
> On 18.09.2018 22:02, Florian Fainelli wrote:
>> On 09/18/2018 12:12 PM, Heiner Kallweit wrote:
>>> I think I've seen a similar or same patch before, not sure why it
>>> didn't make it yet. When being in state PHY_HALTED we don't have to
>>> reschedule the state machine, phy_start() will start it again.
>>
>> Yes, this is conceptually the same patch as as this one:
>>
>> https://patchwork.ozlabs.org/patch/830415/
>>
> Thanks for the link, this is what I was referring to.
>
>> I prefer your version, though the comment in the original patch explains
>> why.
>>
> To be sure I understand you correctly:
> You're fine with the patch as is or would you prefer to add a comment
> like in the original patch ?
Correct, unless you think this does not warrant a comment and it is
clear enough as-is?
>
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>> ---
>>> drivers/net/phy/phy.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
>>> index 1ee25877c..c78203b25 100644
>>> --- a/drivers/net/phy/phy.c
>>> +++ b/drivers/net/phy/phy.c
>>> @@ -1123,7 +1123,7 @@ void phy_state_machine(struct work_struct *work)
>>> * PHY, if PHY_IGNORE_INTERRUPT is set, then we will be moving
>>> * between states from phy_mac_interrupt()
>>> */
>>> - if (phy_polling_mode(phydev))
>>> + if (phy_polling_mode(phydev) && old_state != PHY_HALTED)
>>> queue_delayed_work(system_power_efficient_wq, &phydev->state_queue,
>>> PHY_STATE_TIME * HZ);
>>> }
>>>
>>
>>
>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 2/2] net: phy: call state machine synchronously in phy_stop
From: Florian Fainelli @ 2018-09-20 0:03 UTC (permalink / raw)
To: Heiner Kallweit, Andrew Lunn, David Miller
Cc: netdev@vger.kernel.org, Geert Uytterhoeven
In-Reply-To: <a58e1448-0a10-b7a1-ccca-ca99f146c8ea@gmail.com>
On 09/18/2018 12:56 PM, Heiner Kallweit wrote:
> phy_stop() may be called e.g. when suspending, therefore all needed
> actions should be performed synchronously. Therefore add a synchronous
> call to the state machine.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Yes!
--
Florian
^ permalink raw reply
* Re: [PATCH net-next 1/2] net: linkwatch: add check for netdevice being present to linkwatch_do_dev
From: Florian Fainelli @ 2018-09-20 0:03 UTC (permalink / raw)
To: Heiner Kallweit, Andrew Lunn, David Miller
Cc: netdev@vger.kernel.org, Geert Uytterhoeven
In-Reply-To: <11beeaa9-57d5-e641-9486-f2ba202d0998@gmail.com>
On 09/18/2018 12:55 PM, Heiner Kallweit wrote:
> When bringing down the netdevice (incl. detaching it) and calling
> netif_carrier_off directly or indirectly the latter triggers an
> asynchronous linkwatch event.
> This linkwatch event eventually may fail to access chip registers in
> the ndo_get_stats/ndo_get_stats64 callback because the device isn't
> accessible any longer, see call trace in [0].
>
> To prevent this scenario don't check for IFF_UP only, but also make
> sure that the netdevice is present.
>
> [0] https://lists.openwall.net/netdev/2018/03/15/62
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Thanks Heiner!
--
Florian
^ permalink raw reply
* Re: [PATCH bpf-next 2/3] bpf: emit RECORD_MMAP events for bpf prog load/unload
From: Song Liu @ 2018-09-19 23:44 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S . Miller, daniel@iogearbox.net, peterz@infradead.org,
acme@kernel.org, netdev@vger.kernel.org, Kernel Team
In-Reply-To: <20180919223935.999270-3-ast@kernel.org>
> On Sep 19, 2018, at 3:39 PM, Alexei Starovoitov <ast@kernel.org> wrote:
>
> use perf_event_mmap_bpf_prog() helper to notify user space
> about JITed bpf programs.
> Use RECORD_MMAP perf event to tell user space where JITed bpf program was loaded.
> Use empty program name as unload indication.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
> kernel/bpf/core.c | 22 ++++++++++++++++++++--
> 1 file changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c
> index 3f5bf1af0826..ddf11fdafd36 100644
> --- a/kernel/bpf/core.c
> +++ b/kernel/bpf/core.c
> @@ -384,7 +384,7 @@ bpf_get_prog_addr_region(const struct bpf_prog *prog,
> *symbol_end = addr + hdr->pages * PAGE_SIZE;
> }
>
> -static void bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
> +static char *bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
> {
> const char *end = sym + KSYM_NAME_LEN;
>
> @@ -402,9 +402,10 @@ static void bpf_get_prog_name(const struct bpf_prog *prog, char *sym)
> sym += snprintf(sym, KSYM_NAME_LEN, "bpf_prog_");
> sym = bin2hex(sym, prog->tag, sizeof(prog->tag));
> if (prog->aux->name[0])
> - snprintf(sym, (size_t)(end - sym), "_%s", prog->aux->name);
> + sym += snprintf(sym, (size_t)(end - sym), "_%s", prog->aux->name);
> else
> *sym = 0;
> + return sym;
> }
>
> static __always_inline unsigned long
> @@ -480,23 +481,40 @@ static bool bpf_prog_kallsyms_verify_off(const struct bpf_prog *fp)
>
> void bpf_prog_kallsyms_add(struct bpf_prog *fp)
> {
> + unsigned long symbol_start, symbol_end;
> + char buf[KSYM_NAME_LEN], *sym;
> +
> if (!bpf_prog_kallsyms_candidate(fp) ||
> !capable(CAP_SYS_ADMIN))
> return;
>
> + bpf_get_prog_addr_region(fp, &symbol_start, &symbol_end);
> + sym = bpf_get_prog_name(fp, buf);
> + sym++; /* sym - buf is the length of the name including trailing 0 */
> + while (!IS_ALIGNED(sym - buf, sizeof(u64)))
> + *sym++ = 0;
nit: This logic feels a little weird to me. How about we wrap the extra logic
in a separate function:
size_t bpf_get_prog_name_u64_aligned(const struct bpf_prog fp, char *buf)
where the return value is the u64 aligned size.
Other than this
Acked-by: Song Liu <songliubraving@fb.com>
> spin_lock_bh(&bpf_lock);
> bpf_prog_ksym_node_add(fp->aux);
> spin_unlock_bh(&bpf_lock);
> + perf_event_mmap_bpf_prog(symbol_start, symbol_end - symbol_start,
> + buf, sym - buf);
> }
>
> void bpf_prog_kallsyms_del(struct bpf_prog *fp)
> {
> + unsigned long symbol_start, symbol_end;
> + /* mmap_record.filename cannot be NULL and has to be u64 aligned */
> + char buf[sizeof(u64)] = {};
> +
> if (!bpf_prog_kallsyms_candidate(fp))
> return;
>
> spin_lock_bh(&bpf_lock);
> bpf_prog_ksym_node_del(fp->aux);
> spin_unlock_bh(&bpf_lock);
> + bpf_get_prog_addr_region(fp, &symbol_start, &symbol_end);
> + perf_event_mmap_bpf_prog(symbol_start, symbol_end - symbol_start,
> + buf, sizeof(buf));
> }
>
> static struct bpf_prog *bpf_prog_kallsyms_find(unsigned long addr)
> --
> 2.17.1
>
^ permalink raw reply
* [Patch net-next v3] net_sched: change tcf_del_walker() to take idrinfo->lock
From: Cong Wang @ 2018-09-19 23:37 UTC (permalink / raw)
To: netdev; +Cc: jiri, jhs, Vlad Buslov, Cong Wang
From: Vlad Buslov <vladbu@mellanox.com>
From: Vlad Buslov <vladbu@mellanox.com>
Action API was changed to work with actions and action_idr in concurrency
safe manner, however tcf_del_walker() still uses actions without taking a
reference or idrinfo->lock first, and deletes them directly, disregarding
possible concurrent delete.
Change tcf_del_walker() to take idrinfo->lock while iterating over actions
and use new tcf_idr_release_unsafe() to release them while holding the
lock.
And the blocking function fl_hw_destroy_tmplt() could be called when we
put a filter chain, so defer it to a work queue.
Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
[xiyou.wangcong@gmail.com: heavily modify the code and changelog]
Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
---
net/sched/act_api.c | 20 +++++++++++++++++++-
net/sched/cls_flower.c | 13 +++++++++++--
2 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 6f118d62c731..fac8c769454f 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -246,6 +246,20 @@ static int tcf_dump_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
goto done;
}
+static int tcf_idr_release_unsafe(struct tc_action *p)
+{
+ if (atomic_read(&p->tcfa_bindcnt) > 0)
+ return -EPERM;
+
+ if (refcount_dec_and_test(&p->tcfa_refcnt)) {
+ idr_remove(&p->idrinfo->action_idr, p->tcfa_index);
+ tcf_action_cleanup(p);
+ return ACT_P_DELETED;
+ }
+
+ return 0;
+}
+
static int tcf_del_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
const struct tc_action_ops *ops)
{
@@ -262,15 +276,19 @@ static int tcf_del_walker(struct tcf_idrinfo *idrinfo, struct sk_buff *skb,
if (nla_put_string(skb, TCA_KIND, ops->kind))
goto nla_put_failure;
+ spin_lock(&idrinfo->lock);
idr_for_each_entry_ul(idr, p, id) {
- ret = __tcf_idr_release(p, false, true);
+ ret = tcf_idr_release_unsafe(p);
if (ret == ACT_P_DELETED) {
module_put(ops->owner);
n_i++;
} else if (ret < 0) {
+ spin_unlock(&idrinfo->lock);
goto nla_put_failure;
}
}
+ spin_unlock(&idrinfo->lock);
+
if (nla_put_u32(skb, TCA_FCNT, n_i))
goto nla_put_failure;
nla_nest_end(skb, nest);
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 4b8dd37dd4f8..0ed6630a5049 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -79,6 +79,7 @@ struct fl_flow_tmplt {
struct fl_flow_key mask;
struct flow_dissector dissector;
struct tcf_chain *chain;
+ struct rcu_work rwork;
};
struct cls_fl_head {
@@ -1437,14 +1438,22 @@ static void *fl_tmplt_create(struct net *net, struct tcf_chain *chain,
return ERR_PTR(err);
}
-static void fl_tmplt_destroy(void *tmplt_priv)
+static void fl_tmplt_destroy_work(struct work_struct *work)
{
- struct fl_flow_tmplt *tmplt = tmplt_priv;
+ struct fl_flow_tmplt *tmplt = container_of(to_rcu_work(work),
+ struct fl_flow_tmplt, rwork);
fl_hw_destroy_tmplt(tmplt->chain, tmplt);
kfree(tmplt);
}
+static void fl_tmplt_destroy(void *tmplt_priv)
+{
+ struct fl_flow_tmplt *tmplt = tmplt_priv;
+
+ tcf_queue_work(&tmplt->rwork, fl_tmplt_destroy_work);
+}
+
static int fl_dump_key_val(struct sk_buff *skb,
void *val, int val_type,
void *mask, int mask_type, int len)
--
2.14.4
^ permalink raw reply related
* [PATCH iproute2 v2 3/3] testsuite: Warn about empty $(IPVERS)
From: Petr Vorel @ 2018-09-19 23:36 UTC (permalink / raw)
To: netdev; +Cc: Petr Vorel, Stephen Hemminger, Phil Sutter, Luca Boccassi
In-Reply-To: <20180919233624.18494-1-petr.vorel@gmail.com>
alltests target requires having symlink created by configure target
(default target). Without that there is no test being run.
Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
---
testsuite/Makefile | 3 +++
1 file changed, 3 insertions(+)
diff --git a/testsuite/Makefile b/testsuite/Makefile
index 1c2467f5..b3aebec1 100644
--- a/testsuite/Makefile
+++ b/testsuite/Makefile
@@ -54,6 +54,9 @@ distclean: clean
echo "Entering iproute2" && cd iproute2 && $(MAKE) distclean && cd ..;
$(TESTS): testclean
+ifeq (,$(IPVERS))
+ $(error Please run make first)
+endif
ifeq (,$(HAVE_UNSHARED_UTIL))
$(error Please install util-linux tools to run tests in separated network namespace)
endif
--
2.19.0.rc2
^ permalink raw reply related
* [PATCH iproute2 v2 2/3] testsuite: Generate generate_nlmsg when needed
From: Petr Vorel @ 2018-09-19 23:36 UTC (permalink / raw)
To: netdev; +Cc: Petr Vorel, Stephen Hemminger, Phil Sutter, Luca Boccassi
In-Reply-To: <20180919233624.18494-1-petr.vorel@gmail.com>
Commit 886f2c43 added generate_nlmsg.c. Running alltests
target, which uses the binary required to run 'make -C tools' before.
Fixes: 886f2c43 testsuite: Generate nlmsg blob at runtime
Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
---
testsuite/Makefile | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/testsuite/Makefile b/testsuite/Makefile
index a31d4531..1c2467f5 100644
--- a/testsuite/Makefile
+++ b/testsuite/Makefile
@@ -28,7 +28,7 @@ endif
configure:
echo "Entering iproute2" && cd iproute2 && $(MAKE) configure && cd ..;
-compile: configure
+compile: configure generate_nlmsg
echo "Entering iproute2" && cd iproute2 && $(MAKE) && cd ..;
listtests:
@@ -36,7 +36,10 @@ listtests:
echo "$$t"; \
done
-alltests: $(TESTS)
+generate_nlmsg:
+ $(MAKE) -C tools
+
+alltests: generate_nlmsg $(TESTS)
testclean:
@echo "Removing $(RESULTS_DIR) dir ..."
--
2.19.0.rc2
^ permalink raw reply related
* [PATCH iproute2 v2 1/3] testsuite: Fix missing generate_nlmsg
From: Petr Vorel @ 2018-09-19 23:36 UTC (permalink / raw)
To: netdev; +Cc: Petr Vorel, Stephen Hemminger, Phil Sutter, Luca Boccassi
In-Reply-To: <20180919233624.18494-1-petr.vorel@gmail.com>
Commit ad23e152 caused generate_nlmsg to be always missing:
$ make alltests
make: ./tools/generate_nlmsg: Command not found
Create testclean: to remove only results directory.
Fixes: ad23e152 testsuite: remove all temp files and implement make clean
Signed-off-by: Petr Vorel <petr.vorel@gmail.com>
---
testsuite/Makefile | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/testsuite/Makefile b/testsuite/Makefile
index d1ac997d..a31d4531 100644
--- a/testsuite/Makefile
+++ b/testsuite/Makefile
@@ -30,7 +30,6 @@ configure:
compile: configure
echo "Entering iproute2" && cd iproute2 && $(MAKE) && cd ..;
- $(MAKE) -C tools
listtests:
@for t in $(TESTS); do \
@@ -39,9 +38,11 @@ listtests:
alltests: $(TESTS)
-clean:
+testclean:
@echo "Removing $(RESULTS_DIR) dir ..."
@rm -rf $(RESULTS_DIR)
+
+clean: testclean
@rm -f iproute2/iproute2-this
@rm -f tests/ip/link/dev_wo_vf_rate.nl
$(MAKE) -C tools clean
@@ -49,18 +50,18 @@ clean:
distclean: clean
echo "Entering iproute2" && cd iproute2 && $(MAKE) distclean && cd ..;
-$(TESTS): clean
+$(TESTS): testclean
ifeq (,$(HAVE_UNSHARED_UTIL))
$(error Please install util-linux tools to run tests in separated network namespace)
endif
@./tools/generate_nlmsg
@mkdir -p $(RESULTS_DIR)
-
+
@for d in $(TESTS_DIR); do \
mkdir -p $(RESULTS_DIR)/$$d; \
done
-
+
@if [ "$(KCPATH)" = "/proc/config.gz" ]; then \
gunzip -c $(KCPATH) >$(KENVFN); \
elif [ "$(KCPATH)" != "" ]; then \
--
2.19.0.rc2
^ permalink raw reply related
* [PATCH iproute2 v2 0/3] testsuite: make alltests fixes
From: Petr Vorel @ 2018-09-19 23:36 UTC (permalink / raw)
To: netdev; +Cc: Petr Vorel, Stephen Hemminger, Phil Sutter, Luca Boccassi
Hi,
here are simply fixes to restore 'make alltests'.
Currently it does not run.
Kind regards,
Petr
Petr Vorel (3):
testsuite: Fix missing generate_nlmsg
testsuite: Generate generate_nlmsg when needed
testsuite: Warn about empty $(IPVERS)
testsuite/Makefile | 21 ++++++++++++++-------
1 file changed, 14 insertions(+), 7 deletions(-)
--
2.19.0.rc2
^ permalink raw reply
* Re: [PATCH bpf-next 1/3] perf/core: introduce perf_event_mmap_bpf_prog
From: Song Liu @ 2018-09-19 23:30 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S . Miller, daniel@iogearbox.net, peterz@infradead.org,
acme@kernel.org, netdev@vger.kernel.org, Kernel Team
In-Reply-To: <20180919223935.999270-2-ast@kernel.org>
> On Sep 19, 2018, at 3:39 PM, Alexei Starovoitov <ast@kernel.org> wrote:
>
> introduce perf_event_mmap_bpf_prog() helper to emit RECORD_MMAP events
> into perf ring buffer.
> It's used by bpf load/unload logic to notify user space of addresses
> and names of JITed bpf programs.
>
> Note that event->mmap.pid == -1 is an existing indicator of kernel event.
> In addition use event->mmap.tid == BPF_FS_MAGIC to indicate bpf related
> RECORD_MMAP event.
>
> Alternatively it's possible to introduce new 'enum perf_event_type' command
> specificially for bpf prog load/unload, but existing RECORD_MMAP
> is very close, so the choice made by this patch is to extend it.
>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Song Liu <songliubraving@fb.com>
I guess we should also use this for kernel modules load/unload?
> ---
> include/linux/perf_event.h | 1 +
> kernel/events/core.c | 44 +++++++++++++++++++++++++++++++++-----
> 2 files changed, 40 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
> index 53c500f0ca79..0e79af83138f 100644
> --- a/include/linux/perf_event.h
> +++ b/include/linux/perf_event.h
> @@ -1113,6 +1113,7 @@ static inline void perf_event_task_sched_out(struct task_struct *prev,
> }
>
> extern void perf_event_mmap(struct vm_area_struct *vma);
> +void perf_event_mmap_bpf_prog(u64 start, u64 len, char *name, int size);
> extern struct perf_guest_info_callbacks *perf_guest_cbs;
> extern int perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
> extern int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 2a62b96600ad..c48244ddf993 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7152,7 +7152,7 @@ static int perf_event_mmap_match(struct perf_event *event,
> {
> struct perf_mmap_event *mmap_event = data;
> struct vm_area_struct *vma = mmap_event->vma;
> - int executable = vma->vm_flags & VM_EXEC;
> + int executable = !vma || vma->vm_flags & VM_EXEC;
>
> return (!executable && event->attr.mmap_data) ||
> (executable && (event->attr.mmap || event->attr.mmap2));
> @@ -7165,12 +7165,13 @@ static void perf_event_mmap_output(struct perf_event *event,
> struct perf_output_handle handle;
> struct perf_sample_data sample;
> int size = mmap_event->event_id.header.size;
> + bool bpf_event = !mmap_event->vma;
> int ret;
>
> if (!perf_event_mmap_match(event, data))
> return;
>
> - if (event->attr.mmap2) {
> + if (event->attr.mmap2 && !bpf_event) {
> mmap_event->event_id.header.type = PERF_RECORD_MMAP2;
> mmap_event->event_id.header.size += sizeof(mmap_event->maj);
> mmap_event->event_id.header.size += sizeof(mmap_event->min);
> @@ -7186,12 +7187,14 @@ static void perf_event_mmap_output(struct perf_event *event,
> if (ret)
> goto out;
>
> - mmap_event->event_id.pid = perf_event_pid(event, current);
> - mmap_event->event_id.tid = perf_event_tid(event, current);
> + if (!bpf_event) {
> + mmap_event->event_id.pid = perf_event_pid(event, current);
> + mmap_event->event_id.tid = perf_event_tid(event, current);
> + }
>
> perf_output_put(&handle, mmap_event->event_id);
>
> - if (event->attr.mmap2) {
> + if (event->attr.mmap2 && !bpf_event) {
> perf_output_put(&handle, mmap_event->maj);
> perf_output_put(&handle, mmap_event->min);
> perf_output_put(&handle, mmap_event->ino);
> @@ -7448,6 +7451,37 @@ void perf_event_mmap(struct vm_area_struct *vma)
> perf_event_mmap_event(&mmap_event);
> }
>
> +void perf_event_mmap_bpf_prog(u64 start, u64 len, char *name, int size)
> +{
> + struct perf_mmap_event mmap_event;
> +
> + if (!atomic_read(&nr_mmap_events))
> + return;
> +
> + if (!IS_ALIGNED(size, sizeof(u64))) {
> + WARN_ONCE(1, "size is not aligned\n");
> + return;
> + }
> +
> + mmap_event = (struct perf_mmap_event){
> + .file_name = name,
> + .file_size = size,
> + .event_id = {
> + .header = {
> + .type = PERF_RECORD_MMAP,
> + .misc = PERF_RECORD_MISC_KERNEL,
> + .size = sizeof(mmap_event.event_id) + size,
> + },
> + .pid = -1, /* indicates kernel */
> + .tid = BPF_FS_MAGIC, /* bpf mmap event */
> + .start = start,
> + .len = len,
> + .pgoff = start,
> + },
> + };
> + perf_iterate_sb(perf_event_mmap_output, &mmap_event, NULL);
> +}
> +
> void perf_event_aux_event(struct perf_event *event, unsigned long head,
> unsigned long size, u64 flags)
> {
> --
> 2.17.1
>
^ permalink raw reply
* [PATCH] netfilter: nf_tables: add SECMARK support
From: Christian Göttsche @ 2018-09-19 23:14 UTC (permalink / raw)
To: pablo, kadlec, fw, davem, netfilter-devel, coreteam, netdev,
linux-kernel, paul, sds, eparis, jmorris, serge, selinux,
linux-security-module
Add the ability to set the security context of packets within the nf_tables framework.
Add a nft_object for holding security contexts in the kernel and manipulating packets on the wire.
The contexts are kept as strings and are evaluated to security identifiers at runtime (packet arrival),
so that the nft_objects do not need to be refreshed after security changes.
The maximum security context length is set to 256.
Based on v4.18.6
Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
include/net/netfilter/nf_tables_core.h | 4 +
include/uapi/linux/netfilter/nf_tables.h | 18 ++++-
net/netfilter/nf_tables_core.c | 28 ++++++-
net/netfilter/nft_meta.c | 95 ++++++++++++++++++++++++
4 files changed, 140 insertions(+), 5 deletions(-)
diff --git a/include/net/netfilter/nf_tables_core.h b/include/net/netfilter/nf_tables_core.h
index a0513450..0d1f3b96 100644
--- a/include/net/netfilter/nf_tables_core.h
+++ b/include/net/netfilter/nf_tables_core.h
@@ -16,6 +16,10 @@ extern struct nft_expr_type nft_meta_type;
extern struct nft_expr_type nft_rt_type;
extern struct nft_expr_type nft_exthdr_type;
+#ifdef CONFIG_NETWORK_SECMARK
+extern struct nft_object_type nft_secmark_obj_type;
+#endif
+
int nf_tables_core_module_init(void);
void nf_tables_core_module_exit(void);
diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
index 89438e68..f1527962 100644
--- a/include/uapi/linux/netfilter/nf_tables.h
+++ b/include/uapi/linux/netfilter/nf_tables.h
@@ -1169,6 +1169,21 @@ enum nft_quota_attributes {
};
#define NFTA_QUOTA_MAX (__NFTA_QUOTA_MAX - 1)
+/**
+ * enum nft_secmark_attributes - nf_tables secmark object netlink attributes
+ *
+ * @NFTA_SECMARK_CTX: security context (NLA_STRING)
+ */
+enum nft_secmark_attributes {
+ NFTA_SECMARK_UNSPEC,
+ NFTA_SECMARK_CTX,
+ __NFTA_SECMARK_MAX,
+};
+#define NFTA_SECMARK_MAX (__NFTA_SECMARK_MAX - 1)
+
+/* Max security context length */
+#define NFT_SECMARK_CTX_MAXLEN 256
+
/**
* enum nft_reject_types - nf_tables reject expression reject types
*
@@ -1398,7 +1413,8 @@ enum nft_ct_helper_attributes {
#define NFT_OBJECT_CT_HELPER 3
#define NFT_OBJECT_LIMIT 4
#define NFT_OBJECT_CONNLIMIT 5
-#define __NFT_OBJECT_MAX 6
+#define NFT_OBJECT_SECMARK 6
+#define __NFT_OBJECT_MAX 7
#define NFT_OBJECT_MAX (__NFT_OBJECT_MAX - 1)
/**
diff --git a/net/netfilter/nf_tables_core.c b/net/netfilter/nf_tables_core.c
index 8de912ca..d59ebba0 100644
--- a/net/netfilter/nf_tables_core.c
+++ b/net/netfilter/nf_tables_core.c
@@ -235,12 +235,24 @@ static struct nft_expr_type *nft_basic_types[] = {
&nft_exthdr_type,
};
+static struct nft_object_type *nft_basic_objects[] = {
+#ifdef CONFIG_NETWORK_SECMARK
+ &nft_secmark_obj_type,
+#endif
+};
+
int __init nf_tables_core_module_init(void)
{
- int err, i;
+ int err, i, j = 0;
+
+ for (i = 0; i < ARRAY_SIZE(nft_basic_objects); i++) {
+ err = nft_register_obj(nft_basic_objects[i]);
+ if (err)
+ goto err;
+ }
- for (i = 0; i < ARRAY_SIZE(nft_basic_types); i++) {
- err = nft_register_expr(nft_basic_types[i]);
+ for (j = 0; j < ARRAY_SIZE(nft_basic_types); j++) {
+ err = nft_register_expr(nft_basic_types[j]);
if (err)
goto err;
}
@@ -248,8 +260,12 @@ int __init nf_tables_core_module_init(void)
return 0;
err:
+ while (j-- > 0)
+ nft_unregister_expr(nft_basic_types[j]);
+
while (i-- > 0)
- nft_unregister_expr(nft_basic_types[i]);
+ nft_unregister_obj(nft_basic_objects[i]);
+
return err;
}
@@ -260,4 +276,8 @@ void nf_tables_core_module_exit(void)
i = ARRAY_SIZE(nft_basic_types);
while (i-- > 0)
nft_unregister_expr(nft_basic_types[i]);
+
+ i = ARRAY_SIZE(nft_basic_objects);
+ while (i-- > 0)
+ nft_unregister_obj(nft_basic_objects[i]);
}
diff --git a/net/netfilter/nft_meta.c b/net/netfilter/nft_meta.c
index 1105a23b..26b79a3c 100644
--- a/net/netfilter/nft_meta.c
+++ b/net/netfilter/nft_meta.c
@@ -540,3 +540,98 @@ struct nft_expr_type nft_meta_type __read_mostly = {
.maxattr = NFTA_META_MAX,
.owner = THIS_MODULE,
};
+
+#ifdef CONFIG_NETWORK_SECMARK
+
+struct nft_secmark {
+ char ctx[NFT_SECMARK_CTX_MAXLEN];
+ int len;
+};
+
+static const struct nla_policy nft_secmark_policy[NFTA_SECMARK_MAX + 1] = {
+ [NFTA_SECMARK_CTX] = { .type = NLA_STRING, .len = NFT_SECMARK_CTX_MAXLEN },
+};
+
+
+static void nft_secmark_obj_eval(struct nft_object *obj, struct nft_regs *regs, const struct nft_pktinfo *pkt)
+{
+ const struct nft_secmark *priv = nft_obj_data(obj);
+ struct sk_buff *skb = pkt->skb;
+ int err;
+ u32 secid = 0;
+
+ /* skip if packet has already a secmark */
+ if (skb->secmark)
+ return;
+
+ err = security_secctx_to_secid(priv->ctx, priv->len, &secid);
+ if (err) {
+ if (err == -EINVAL)
+ pr_notice_ratelimited("invalid security context \'%s\'\n", priv->ctx);
+ else
+ pr_notice_ratelimited("unable to convert security context \'%s\': %d\n", priv->ctx, -err);
+ return;
+ }
+
+ if (!secid) {
+ pr_notice_ratelimited("unable to map security context \'%s\'\n", priv->ctx);
+ return;
+ }
+
+ err = security_secmark_relabel_packet(secid);
+ if (err) {
+ pr_notice_ratelimited("unable to obtain relabeling permission: %d\n", -err);
+ return;
+ }
+
+ skb->secmark = secid;
+}
+
+
+static int nft_secmark_obj_init(const struct nft_ctx *ctx, const struct nlattr * const tb[], struct nft_object *obj)
+{
+ struct nft_secmark *priv = nft_obj_data(obj);
+
+ if (tb[NFTA_SECMARK_CTX] == NULL)
+ return -EINVAL;
+
+ nla_strlcpy(priv->ctx, tb[NFTA_SECMARK_CTX], NFT_SECMARK_CTX_MAXLEN);
+ priv->len = strlen(priv->ctx);
+
+ security_secmark_refcount_inc();
+
+ return 0;
+}
+
+static int nft_secmark_obj_dump(struct sk_buff *skb, struct nft_object *obj, bool reset)
+{
+ const struct nft_secmark *priv = nft_obj_data(obj);
+
+ if (nla_put_string(skb, NFTA_SECMARK_CTX, priv->ctx))
+ return -1;
+
+ return 0;
+}
+
+static void nft_secmark_obj_destroy(const struct nft_ctx *ctx, struct nft_object *obj)
+{
+ security_secmark_refcount_dec();
+}
+
+static const struct nft_object_ops nft_secmark_obj_ops = {
+ .type = &nft_secmark_obj_type,
+ .size = sizeof(struct nft_secmark),
+ .init = nft_secmark_obj_init,
+ .eval = nft_secmark_obj_eval,
+ .dump = nft_secmark_obj_dump,
+ .destroy = nft_secmark_obj_destroy,
+};
+struct nft_object_type nft_secmark_obj_type __read_mostly = {
+ .type = NFT_OBJECT_SECMARK,
+ .ops = &nft_secmark_obj_ops,
+ .maxattr = NFTA_SECMARK_MAX,
+ .policy = nft_secmark_policy,
+ .owner = THIS_MODULE,
+};
+
+#endif /* CONFIG_NETWORK_SECMARK */
--
2.19.0
^ permalink raw reply related
* Re: [PATCH] net: ibm: remove redundant local variables 'act_nr_of_entries' and 'act_pages'
From: David Miller @ 2018-09-20 4:26 UTC (permalink / raw)
To: zhongjiang; +Cc: dougmill, netdev, linux-kernel
In-Reply-To: <1537367449-59565-1-git-send-email-zhongjiang@huawei.com>
From: zhong jiang <zhongjiang@huawei.com>
Date: Wed, 19 Sep 2018 22:30:49 +0800
> That local variable are never used after being assigned.
> hence it should be redundant and can be removed.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: ibm: remove a redundant local variable 'k'
From: David Miller @ 2018-09-20 4:26 UTC (permalink / raw)
To: zhongjiang; +Cc: dougmill, netdev, linux-kernel
In-Reply-To: <1537366995-59168-1-git-send-email-zhongjiang@huawei.com>
From: zhong jiang <zhongjiang@huawei.com>
Date: Wed, 19 Sep 2018 22:23:15 +0800
> The local variable 'k' is never used after being assigned.
> hence it should be redundant adn can be removed.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 00/12] Fixes, cleanups & minor additions to HNS3 driver
From: David Miller @ 2018-09-20 4:21 UTC (permalink / raw)
To: salil.mehta
Cc: yisen.zhuang, lipeng321, mehta.salil, netdev, linux-kernel,
linuxarm
In-Reply-To: <20180919172958.12992-1-salil.mehta@huawei.com>
From: Salil Mehta <salil.mehta@huawei.com>
Date: Wed, 19 Sep 2018 18:29:46 +0100
> This patch-set present some fixes, cleanups to the HNS£ PF and VF driver.
Series applied, thank you.
^ permalink raw reply
* [PATCH bpf-next 0/3] perf, bpf: reveal invisible bpf programs
From: Alexei Starovoitov @ 2018-09-19 22:39 UTC (permalink / raw)
To: David S . Miller; +Cc: daniel, peterz, acme, netdev, kernel-team
Hi All,
this patch set adds kernel and user space support to reveal
short lived bpf programs.
The kernel stores addr+bpf_prog_name information into perf ring buffer.
Later 'perf report' can properly attribute the cpu time to the program.
The following command was run before and after: 'perf record ./test_progs; perf report'
Before patch set:
# Overhead Command Shared Object Symbol
# ........ .......... ................. .......................................
#
10.73% test_progs [kernel.kallsyms] [k] __htab_map_lookup_elem
8.16% test_progs [kernel.kallsyms] [k] bpf_skb_set_tunnel_key
7.90% test_progs [kernel.kallsyms] [k] memcmp
3.10% test_progs [kernel.kallsyms] [k] lookup_nulls_elem_raw
2.57% test_progs [kernel.kallsyms] [k] dst_release
2.45% test_progs [kernel.kallsyms] [k] do_check
1.89% test_progs [kernel.kallsyms] [k] bpf_xdp_adjust_head
1.52% test_progs [kernel.kallsyms] [k] 0x00007fffa002cc5a
1.22% test_progs [kernel.kallsyms] [k] check_helper_mem_access
0.99% test_progs [kernel.kallsyms] [k] 0x00007fffa001a6a8
0.99% test_progs [kernel.kallsyms] [k] kmem_cache_alloc_trace
0.97% test_progs [kernel.kallsyms] [k] 0x00007fffa001a652
0.95% test_progs [kernel.kallsyms] [k] 0x00007fffa0042d52
0.95% test_progs [kernel.kallsyms] [k] read_tsc
0.81% test_progs [kernel.kallsyms] [k] 0x00007fffa001a660
0.77% test_progs [kernel.kallsyms] [k] 0x00007fffa0042d69
0.74% test_progs [kernel.kallsyms] [k] percpu_array_map_lookup_elem
0.69% test_progs [kernel.kallsyms] [k] 0x00007fffa001a64e
After:
# Overhead Command Shared Object Symbol
# ........ .......... ................. .............................................
#
18.13% test_progs [kernel.kallsyms] [k] bpf_prog_1accc788e7f04c38_balancer_ingres
10.73% test_progs [kernel.kallsyms] [k] __htab_map_lookup_elem
7.94% test_progs [kernel.kallsyms] [k] bpf_prog_20a05d8a586cf0e8_F
7.80% test_progs [kernel.kallsyms] [k] bpf_skb_set_tunnel_key
4.64% test_progs [kernel.kallsyms] [k] __sysfs_match_string
3.61% test_progs [kernel.kallsyms] [k] bpf_prog_9d89afa51f1dc0d7_F
3.51% test_progs [kernel.kallsyms] [k] bpf_prog_73b45270911a0294_F
3.41% test_progs [kernel.kallsyms] [k] bpf_prog_79be7b7bad8026bf_F
3.26% test_progs [kernel.kallsyms] [k] memcmp
3.10% test_progs [kernel.kallsyms] [k] lookup_nulls_elem_raw
2.89% test_progs [kernel.kallsyms] [k] bpf_prog_57bf3d413b9b7455_F
2.57% test_progs [kernel.kallsyms] [k] dst_discard
2.45% test_progs [kernel.kallsyms] [k] do_check
2.39% test_progs [kernel.kallsyms] [k] bpf_prog_576cbdaac1a4d2f6_F
1.82% test_progs [kernel.kallsyms] [k] bpf_prog_53ade2ecbddaa85b_F
1.70% test_progs [kernel.kallsyms] [k] bpf_xdp_adjust_head
1.32% test_progs [kernel.kallsyms] [k] bpf_prog_0edc54822404a598_F
Important considerations:
- Before and after the number of cpu samples are the same. No samples are lost.
But perf cannot find the symbol by IP, so a lot of small 0x00007fffa001a64e-like
symbols appear towards the end of 'perf report' and none of them look hot.
In reallity these IP addresses belong to few bpf programs that
were active at that time.
- newly loaded bpf program can be JITed into address space of unloaded prog.
7fffa001aXXX address at time X can belong to a program A, but similar
7fffa001aYYY address at time Y can belong to a different program B.
- event->mmap.pid == -1 is an existing indicator of kernel event.
event->mmap.tid == BPF_FS_MAGIC is an extension to indicate bpf related event.
Alternatively it's possible to introduce new 'enum perf_event_type' command
specificially for bpf prog load/unload, but existing RECORD_MMAP
is very close, so the choice made by this patchset is to extend it.
- steps to land the set:
Patches 1 and 2 need to go via bpf-next tree,
since we're adding support for better program names exactly in the same lines.
Patch 3 can go in parallel into perf tree. It has no effect without kernel
support and kernel support has not effect on old perf.
Peter, Arnaldo, Daniel, please review.
Alexei Starovoitov (3):
perf/core: introduce perf_event_mmap_bpf_prog
bpf: emit RECORD_MMAP events for bpf prog load/unload
tools/perf: recognize and process RECORD_MMAP events for bpf progs
include/linux/perf_event.h | 1 +
kernel/bpf/core.c | 22 +++++++++++++++++--
kernel/events/core.c | 44 +++++++++++++++++++++++++++++++++-----
tools/perf/util/machine.c | 27 +++++++++++++++++++++++
tools/perf/util/symbol.c | 13 +++++++++++
tools/perf/util/symbol.h | 1 +
6 files changed, 101 insertions(+), 7 deletions(-)
--
2.17.1
^ permalink raw reply
* [PATCH bpf-next 1/3] perf/core: introduce perf_event_mmap_bpf_prog
From: Alexei Starovoitov @ 2018-09-19 22:39 UTC (permalink / raw)
To: David S . Miller; +Cc: daniel, peterz, acme, netdev, kernel-team
In-Reply-To: <20180919223935.999270-1-ast@kernel.org>
introduce perf_event_mmap_bpf_prog() helper to emit RECORD_MMAP events
into perf ring buffer.
It's used by bpf load/unload logic to notify user space of addresses
and names of JITed bpf programs.
Note that event->mmap.pid == -1 is an existing indicator of kernel event.
In addition use event->mmap.tid == BPF_FS_MAGIC to indicate bpf related
RECORD_MMAP event.
Alternatively it's possible to introduce new 'enum perf_event_type' command
specificially for bpf prog load/unload, but existing RECORD_MMAP
is very close, so the choice made by this patch is to extend it.
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
include/linux/perf_event.h | 1 +
kernel/events/core.c | 44 +++++++++++++++++++++++++++++++++-----
2 files changed, 40 insertions(+), 5 deletions(-)
diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index 53c500f0ca79..0e79af83138f 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1113,6 +1113,7 @@ static inline void perf_event_task_sched_out(struct task_struct *prev,
}
extern void perf_event_mmap(struct vm_area_struct *vma);
+void perf_event_mmap_bpf_prog(u64 start, u64 len, char *name, int size);
extern struct perf_guest_info_callbacks *perf_guest_cbs;
extern int perf_register_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
extern int perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *callbacks);
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 2a62b96600ad..c48244ddf993 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7152,7 +7152,7 @@ static int perf_event_mmap_match(struct perf_event *event,
{
struct perf_mmap_event *mmap_event = data;
struct vm_area_struct *vma = mmap_event->vma;
- int executable = vma->vm_flags & VM_EXEC;
+ int executable = !vma || vma->vm_flags & VM_EXEC;
return (!executable && event->attr.mmap_data) ||
(executable && (event->attr.mmap || event->attr.mmap2));
@@ -7165,12 +7165,13 @@ static void perf_event_mmap_output(struct perf_event *event,
struct perf_output_handle handle;
struct perf_sample_data sample;
int size = mmap_event->event_id.header.size;
+ bool bpf_event = !mmap_event->vma;
int ret;
if (!perf_event_mmap_match(event, data))
return;
- if (event->attr.mmap2) {
+ if (event->attr.mmap2 && !bpf_event) {
mmap_event->event_id.header.type = PERF_RECORD_MMAP2;
mmap_event->event_id.header.size += sizeof(mmap_event->maj);
mmap_event->event_id.header.size += sizeof(mmap_event->min);
@@ -7186,12 +7187,14 @@ static void perf_event_mmap_output(struct perf_event *event,
if (ret)
goto out;
- mmap_event->event_id.pid = perf_event_pid(event, current);
- mmap_event->event_id.tid = perf_event_tid(event, current);
+ if (!bpf_event) {
+ mmap_event->event_id.pid = perf_event_pid(event, current);
+ mmap_event->event_id.tid = perf_event_tid(event, current);
+ }
perf_output_put(&handle, mmap_event->event_id);
- if (event->attr.mmap2) {
+ if (event->attr.mmap2 && !bpf_event) {
perf_output_put(&handle, mmap_event->maj);
perf_output_put(&handle, mmap_event->min);
perf_output_put(&handle, mmap_event->ino);
@@ -7448,6 +7451,37 @@ void perf_event_mmap(struct vm_area_struct *vma)
perf_event_mmap_event(&mmap_event);
}
+void perf_event_mmap_bpf_prog(u64 start, u64 len, char *name, int size)
+{
+ struct perf_mmap_event mmap_event;
+
+ if (!atomic_read(&nr_mmap_events))
+ return;
+
+ if (!IS_ALIGNED(size, sizeof(u64))) {
+ WARN_ONCE(1, "size is not aligned\n");
+ return;
+ }
+
+ mmap_event = (struct perf_mmap_event){
+ .file_name = name,
+ .file_size = size,
+ .event_id = {
+ .header = {
+ .type = PERF_RECORD_MMAP,
+ .misc = PERF_RECORD_MISC_KERNEL,
+ .size = sizeof(mmap_event.event_id) + size,
+ },
+ .pid = -1, /* indicates kernel */
+ .tid = BPF_FS_MAGIC, /* bpf mmap event */
+ .start = start,
+ .len = len,
+ .pgoff = start,
+ },
+ };
+ perf_iterate_sb(perf_event_mmap_output, &mmap_event, NULL);
+}
+
void perf_event_aux_event(struct perf_event *event, unsigned long head,
unsigned long size, u64 flags)
{
--
2.17.1
^ permalink raw reply related
* [PATCH perf 3/3] tools/perf: recognize and process RECORD_MMAP events for bpf progs
From: Alexei Starovoitov @ 2018-09-19 22:39 UTC (permalink / raw)
To: David S . Miller; +Cc: daniel, peterz, acme, netdev, kernel-team
In-Reply-To: <20180919223935.999270-1-ast@kernel.org>
Recognize JITed bpf prog load/unload events.
Add/remove kernel symbols accordingly.
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
tools/perf/util/machine.c | 27 +++++++++++++++++++++++++++
tools/perf/util/symbol.c | 13 +++++++++++++
tools/perf/util/symbol.h | 1 +
3 files changed, 41 insertions(+)
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index c4acd2001db0..ae4f8a0fdc7e 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -25,6 +25,7 @@
#include "sane_ctype.h"
#include <symbol/kallsyms.h>
#include <linux/mman.h>
+#include <linux/magic.h>
static void __machine__remove_thread(struct machine *machine, struct thread *th, bool lock);
@@ -1460,6 +1461,32 @@ static int machine__process_kernel_mmap_event(struct machine *machine,
enum dso_kernel_type kernel_type;
bool is_kernel_mmap;
+ /* process JITed bpf programs load/unload events */
+ if (event->mmap.pid == ~0u && event->mmap.tid == BPF_FS_MAGIC) {
+ struct symbol *sym;
+ u64 ip;
+
+ map = map_groups__find(&machine->kmaps, event->mmap.start);
+ if (!map) {
+ pr_err("No kernel map for IP %lx\n", event->mmap.start);
+ goto out_problem;
+ }
+ ip = event->mmap.start - map->start + map->pgoff;
+ if (event->mmap.filename[0]) {
+ sym = symbol__new(ip, event->mmap.len, 0, 0,
+ event->mmap.filename);
+ dso__insert_symbol(map->dso, sym);
+ } else {
+ if (symbols__erase(&map->dso->symbols, ip)) {
+ pr_err("No bpf prog at IP %lx/%lx\n",
+ event->mmap.start, ip);
+ goto out_problem;
+ }
+ dso__reset_find_symbol_cache(map->dso);
+ }
+ return 0;
+ }
+
/* If we have maps from kcore then we do not need or want any others */
if (machine__uses_kcore(machine))
return 0;
diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index d188b7588152..0653f313661d 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -353,6 +353,19 @@ static struct symbol *symbols__find(struct rb_root *symbols, u64 ip)
return NULL;
}
+int symbols__erase(struct rb_root *symbols, u64 ip)
+{
+ struct symbol *s;
+
+ s = symbols__find(symbols, ip);
+ if (!s)
+ return -ENOENT;
+
+ rb_erase(&s->rb_node, symbols);
+ symbol__delete(s);
+ return 0;
+}
+
static struct symbol *symbols__first(struct rb_root *symbols)
{
struct rb_node *n = rb_first(symbols);
diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
index f25fae4b5743..92ef31953d9a 100644
--- a/tools/perf/util/symbol.h
+++ b/tools/perf/util/symbol.h
@@ -310,6 +310,7 @@ char *dso__demangle_sym(struct dso *dso, int kmodule, const char *elf_name);
void __symbols__insert(struct rb_root *symbols, struct symbol *sym, bool kernel);
void symbols__insert(struct rb_root *symbols, struct symbol *sym);
+int symbols__erase(struct rb_root *symbols, u64 ip);
void symbols__fixup_duplicate(struct rb_root *symbols);
void symbols__fixup_end(struct rb_root *symbols);
void map_groups__fixup_end(struct map_groups *mg);
--
2.17.1
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox