* Re: [PATCH] wimax/i2400m: fix spelling mistake "unitialized" -> "uninitialized"
From: Colin Ian King @ 2018-09-24 17:35 UTC (permalink / raw)
To: Sergei Shtylyov, Inaky Perez-Gonzalez, linux-wimax,
David S . Miller, netdev
Cc: kernel-janitors, linux-kernel
In-Reply-To: <db8575a7-a4ab-3234-3e59-06cdeffe0f59@cogentembedded.com>
On 24/09/18 18:27, Sergei Shtylyov wrote:
> On 09/24/2018 08:18 PM, Colin King wrote:
>
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> Trivial fix to spelling mistake in ms_to_errno array of error messages
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>> drivers/net/wimax/i2400m/control.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wimax/i2400m/control.c b/drivers/net/wimax/i2400m/control.c
>> index 094cea775d0c..0475ad553707 100644
>> --- a/drivers/net/wimax/i2400m/control.c
>> +++ b/drivers/net/wimax/i2400m/control.c
>> @@ -257,7 +257,7 @@ static const struct
>> [I2400M_MS_ACCESSIBILITY_ERROR] = { "accesibility error", -EIO },
>> [I2400M_MS_BUSY] = { "busy", -EBUSY },
>> [I2400M_MS_CORRUPTED_TLV] = { "corrupted TLV", -EILSEQ },
>> - [I2400M_MS_UNINITIALIZED] = { "not unitialized", -EILSEQ },
>> + [I2400M_MS_UNINITIALIZED] = { "not uninitialized", -EILSEQ },
>
> I suspect they rather meant "not initialized".
Of course, that makes far more sense. I send a V2.
>
> [...]
>
> MBR, Sergei
>
^ permalink raw reply
* [PATCH][V2] wimax/i2400m: fix spelling mistake "not unitialized" -> "uninitialized"
From: Colin King @ 2018-09-24 17:39 UTC (permalink / raw)
To: Inaky Perez-Gonzalez, linux-wimax, David S . Miller, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Trivial fix to spelling mistake in ms_to_errno array of error messages
and remove confusing "not" from the error text since the error code
refers to an uninitialized error code.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
V2: remove "not" from text too. Thanks to Sergei Shtylyov for spotting that.
---
drivers/net/wimax/i2400m/control.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wimax/i2400m/control.c b/drivers/net/wimax/i2400m/control.c
index 094cea775d0c..0475ad553707 100644
--- a/drivers/net/wimax/i2400m/control.c
+++ b/drivers/net/wimax/i2400m/control.c
@@ -257,7 +257,7 @@ static const struct
[I2400M_MS_ACCESSIBILITY_ERROR] = { "accesibility error", -EIO },
[I2400M_MS_BUSY] = { "busy", -EBUSY },
[I2400M_MS_CORRUPTED_TLV] = { "corrupted TLV", -EILSEQ },
- [I2400M_MS_UNINITIALIZED] = { "not unitialized", -EILSEQ },
+ [I2400M_MS_UNINITIALIZED] = { "uninitialized", -EILSEQ },
[I2400M_MS_UNKNOWN_ERROR] = { "unknown error", -EIO },
[I2400M_MS_PRODUCTION_ERROR] = { "production error", -EIO },
[I2400M_MS_NO_RF] = { "no RF", -EIO },
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 0/2] gpiolib: Fix issues introduced by fast bitmap processing path
From: Marek Szyprowski @ 2018-09-24 11:38 UTC (permalink / raw)
To: Janusz Krzysztofik
Cc: Linus Walleij, Jonathan Corbet, Miguel Ojeda Sandonis,
Peter Korsgaard, Peter Rosin, Ulf Hansson, Andrew Lunn,
Florian Fainelli, David S. Miller, Dominik Brodowski,
Greg Kroah-Hartman, Kishon Vijay Abraham I, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
Peter Meerwald-Stadler, Jiri Slaby, Willy Tarreau
In-Reply-To: <CAGfqbt6H8u_dUokB-pg51kCj09G8Mh7wB0879nKMw5Dw83tcSg@mail.gmail.com>
Hi Janusz,
On 2018-09-24 13:08, Janusz Krzysztofik wrote:
> 2018-09-24 11:43 GMT+02:00, Marek Szyprowski <m.szyprowski@samsung.com>:
>> On 2018-09-24 01:53, Janusz Krzysztofik wrote:
>>> While investigating possible reasons of GPIO fast bitmap processing
>>> related boot hang on Samsung Snow Chromebook, reported by Marek
>>> Szyprowski (thanks!), I've discovered one coding bug, addressed by
>>> PATCH 1/2 of this series, and one potential regression introduced at
>>> design level of the solution, hopefully fixed by PATCH 2/2. See
>>> commit messages for details.
>>>
>>> Janusz Krzysztofik (2):
>>> gpiolib: Fix missing updates of bitmap index
>>> gpiolib: Fix array members of same chip processed separately
>>>
>>> The fixes should resolve the boot hang observed by Marek, however the
>>> second change excludes that particular case from fast bitmap processing
>>> and restores the old behaviour.
>> I confirm, that the above 2 patches fixes boot issue on Samsung Snow
>> Chromebook with next-20180920.
>>
>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>
>>> Hence, it is possible still another
>>> issue which have had an influence on that boot hang exists in the code.
>>> In order to fully verify the fix, it would have to be tested on a
>>> platform where an array of GPIO descriptors is used which starts from
>>> at least two consecutive pins of one GPIO chip in hardware order,
>>> starting ftom 0, followed by one or more pins belonging to other
>>> chip(s).
>>>
>>> In order to verify if separate calls to .set() chip callback for each
>>> pin instead of one call to .set_multiple() is actually the reason of
>>> boot hang on Samsung Snow Chromebook, the affected driver -
>>> drivers/mmc/core/pwrseq_simple.c - would have to be temporarily
>>> modified for testing purposes so it calls gpiod_set_value() for each
>>> pin instead of gpiod_set_array_value() for all of them. If that would
>>> also result in boot hang, we could be sure the issue was really the
>>> one addressed by the second fix. Marek, could you please try to
>>> perform such test?
>> Yes, I've just tested next-20180920 only with the first patch from this
>> patchset and the mentioned change to drivers/mmc/core/pwrseq_simple.c.
>> It boots fine, so indeed the issue is in handling of arrays of gpios.
>>
>> Just to be sure I did it right, this is my change to the mentioned file:
> Yeah, that's what I had on mind. However, I'd be more lucky if it didn't work
> for you. Setting the pins sequentially, not simultaneously as before, was
> exactly what I hoped was the reason of the hang.
>
>> diff --git a/drivers/mmc/core/pwrseq_simple.c
>> b/drivers/mmc/core/pwrseq_simple.c
>> index 7f882a2bb872..9397dc1f2e38 100644
>> --- a/drivers/mmc/core/pwrseq_simple.c
>> +++ b/drivers/mmc/core/pwrseq_simple.c
>> @@ -38,16 +38,11 @@ static void mmc_pwrseq_simple_set_gpios_value(struct
>> mmc_pwrseq_simple *pwrseq,
>> int value)
>> {
>> struct gpio_descs *reset_gpios = pwrseq->reset_gpios;
>> + int i;
>>
>> - if (!IS_ERR(reset_gpios)) {
>> - DECLARE_BITMAP(values, BITS_PER_TYPE(value));
>> - int nvalues = reset_gpios->ndescs;
>> -
>> - values[0] = value;
>> -
>> - gpiod_set_array_value_cansleep(nvalues, reset_gpios->desc,
>> - reset_gpios->info, values);
>> - }
>> + if (!IS_ERR(reset_gpios))
>> + for (i = 0; i < reset_gpios->ndescs; i++)
> The only difference from the behaviour when the hang was occurring is now
> the order the pins are manipulated. Maybe that matters?
> Could you please retry the same with the order of pins reversed, either in
> the .dts file or here inside this for loop?
I've switched the order of pins in dts and next-20180920 + first patch +
above
change also boots fine.
Best regards
--
Marek Szyprowski, PhD
Samsung R&D Institute Poland
^ permalink raw reply
* r8169 hang on 4.18
From: Ortwin Glück @ 2018-09-24 12:00 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, netdev
Hi,
Stable kernel has stability problems on r8169 that were not present in 4.17.3:
[ 0.000000] Linux version 4.18.8 (kbuild@lofw) (gcc version 7.3.0 (Gentoo 7.3.0-r3 p1.4)) #70 SMP
PREEMPT Mon Sep 17 17:56:57 CEST 2018
[ 0.000000] Command line: BOOT_IMAGE=/boot/linux-4.18.8 root=LABEL=ROOT ro rootfstype=ext4
net.ifnames=0 pci=nomsi
[ 1.772849] r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
[ 1.772852] r8169 0000:07:00.0: can't disable ASPM; OS doesn't have ASPM control
[ 1.784948] r8169 0000:07:00.0 eth4: RTL8168h/8111h, 50:9a:4c:2e:92:be, XID 54100800, IRQ 16
[ 1.784949] r8169 0000:07:00.0 eth4: jumbo features [frames: 9200 bytes, tx checksumming: ko]
We saw the interface unresponsive twice during the last 3 days with:
[Mon Sep 24 11:35:56 2018] ------------[ cut here ]------------
[Mon Sep 24 11:35:56 2018] NETDEV WATCHDOG: wan (r8169): transmit queue 0 timed out
[Mon Sep 24 11:35:56 2018] WARNING: CPU: 2 PID: 0 at net/sched/sch_generic.c:461
dev_watchdog+0x215/0x220
[Mon Sep 24 11:35:56 2018] CPU: 2 PID: 0 Comm: swapper/2 Not tainted 4.18.8 #70
[Mon Sep 24 11:35:56 2018] Hardware name: Dell Inc. OptiPlex 3050/0W0CHX, BIOS 1.6.5 09/09/2017
[Mon Sep 24 11:35:56 2018] RIP: 0010:dev_watchdog+0x215/0x220
[Mon Sep 24 11:35:56 2018] Code: 49 63 4c 24 e8 eb 8c 4c 89 ef c6 05 1a 19 ca 00 01 e8 5f 52 fd ff
89 d9 4c 89 ee 48 c7 c7 78 ab 67 89 48 89 c2 e8 1b 2b 49 ff <0f> 0b eb be 0f 1f 80 00 00 00 00 41 57
45 89 cf 41 56 49 89 d6 41
[Mon Sep 24 11:35:56 2018] RSP: 0018:ffff96f05dd03e98 EFLAGS: 00010282
[Mon Sep 24 11:35:56 2018] RAX: 0000000000000000 RBX: 0000000000000000 RCX: 0000000000000006
[Mon Sep 24 11:35:56 2018] RDX: 0000000000000007 RSI: 0000000000000096 RDI: ffff96f05dd15350
[Mon Sep 24 11:35:56 2018] RBP: ffff96f0462ee41c R08: 0000000000000001 R09: 000000000000133d
[Mon Sep 24 11:35:56 2018] R10: 0000000000000202 R11: 0000000000000000 R12: ffff96f0462ee438
[Mon Sep 24 11:35:56 2018] R13: ffff96f0462ee000 R14: 0000000000000001 R15: ffff96f0455eaa80
[Mon Sep 24 11:35:56 2018] FS: 0000000000000000(0000) GS:ffff96f05dd00000(0000) knlGS:0000000000000000
[Mon Sep 24 11:35:56 2018] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[Mon Sep 24 11:35:56 2018] CR2: 000055c9498766e0 CR3: 00000000bb80a006 CR4: 00000000003606e0
[Mon Sep 24 11:35:56 2018] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[Mon Sep 24 11:35:56 2018] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[Mon Sep 24 11:35:56 2018] Call Trace:
[Mon Sep 24 11:35:56 2018] <IRQ>
[Mon Sep 24 11:35:56 2018] ? pfifo_fast_reset+0x130/0x130
[Mon Sep 24 11:35:56 2018] ? pfifo_fast_reset+0x130/0x130
[Mon Sep 24 11:35:56 2018] call_timer_fn+0x11/0x70
[Mon Sep 24 11:35:56 2018] expire_timers+0x8e/0xa0
[Mon Sep 24 11:35:56 2018] run_timer_softirq+0xb9/0x160
[Mon Sep 24 11:35:56 2018] ? __hrtimer_run_queues+0x135/0x1a0
[Mon Sep 24 11:35:56 2018] ? hw_breakpoint_pmu_read+0x10/0x10
[Mon Sep 24 11:35:56 2018] ? ktime_get+0x39/0x90
[Mon Sep 24 11:35:56 2018] ? lapic_next_event+0x20/0x20
[Mon Sep 24 11:35:56 2018] __do_softirq+0xcb/0x1f8
[Mon Sep 24 11:35:56 2018] irq_exit+0xa9/0xb0
[Mon Sep 24 11:35:56 2018] smp_apic_timer_interrupt+0x59/0x90
[Mon Sep 24 11:35:56 2018] apic_timer_interrupt+0xf/0x20
[Mon Sep 24 11:35:56 2018] </IRQ>
[Mon Sep 24 11:35:56 2018] RIP: 0010:cpuidle_enter_state+0x129/0x200
[Mon Sep 24 11:35:56 2018] Code: 45 00 89 c3 e8 d8 3b 55 ff 65 8b 3d b1 eb 45 77 e8 8c 3a 55 ff 31
ff 49 89 c4 e8 72 43 55 ff fb 48 ba cf f7 53 e3 a5 9b c4 20 <4c> 89 e1 4c 29 e9 48 89 c8 48 c1 f9 3f
48 f7 ea b8 ff ff ff 7f 48
[Mon Sep 24 11:35:56 2018] RSP: 0018:ffff9a93c06e7ea8 EFLAGS: 00000286 ORIG_RAX: ffffffffffffff13
[Mon Sep 24 11:35:56 2018] RAX: ffff96f05dd1f800 RBX: 0000000000000003 RCX: 000000000000001f
[Mon Sep 24 11:35:56 2018] RDX: 20c49ba5e353f7cf RSI: 00000000258f0602 RDI: 0000000000000000
[Mon Sep 24 11:35:56 2018] RBP: ffff96f05dd25ee0 R08: 00000000000002b4 R09: 00000000ffffffff
[Mon Sep 24 11:35:56 2018] R10: ffff9a93c06e7e90 R11: 0000000000000142 R12: 00012ec849a182b9
[Mon Sep 24 11:35:56 2018] R13: 00012ec8499ddf88 R14: 0000000000000003 R15: 0000000000000000
[Mon Sep 24 11:35:56 2018] ? cpuidle_enter_state+0x11e/0x200
[Mon Sep 24 11:35:56 2018] do_idle+0x1c0/0x200
[Mon Sep 24 11:35:56 2018] cpu_startup_entry+0x6a/0x70
[Mon Sep 24 11:35:56 2018] start_secondary+0x18a/0x1c0
[Mon Sep 24 11:35:56 2018] secondary_startup_64+0xa5/0xb0
[Mon Sep 24 11:35:56 2018] ---[ end trace 327bd9c035abe307 ]---
This is the built-in ethernet port on a Dell main board:
07:00.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd. RTL8111/8168/8411 PCI Express
Gigabit Ethernet Controller [10ec:8168] (rev 15)
Subsystem: Dell RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller [1028:07a3]
Flags: bus master, fast devsel, latency 0, IRQ 16
I/O ports at e000 [size=256]
Memory at f7404000 (64-bit, non-prefetchable) [size=4K]
Memory at f7400000 (64-bit, non-prefetchable) [size=16K]
Capabilities: [40] Power Management version 3
Capabilities: [50] MSI: Enable- Count=1/1 Maskable- 64bit+
Capabilities: [70] Express Endpoint, MSI 01
Capabilities: [b0] MSI-X: Enable- Count=4 Masked-
Capabilities: [100] Advanced Error Reporting
Capabilities: [140] Virtual Channel
Capabilities: [160] Device Serial Number 01-00-00-00-68-4c-e0-00
Capabilities: [170] Latency Tolerance Reporting
Capabilities: [178] L1 PM Substates
Kernel driver in use: r8169
The box has an extra 4-way ethernet card that uses the same driver. We had to set pci=nomsi because
the card frequently behaved erratic with msi on.
Thanks,
Ortwin
^ permalink raw reply
* vlan missing with AF_PACKET and auxdata
From: Michael Walle @ 2018-09-24 12:01 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Eric Dumazet, Steinar H. Gunderson, heiko.thiery
Hi,
I'm using the AF_PACKET socket with setsockopt(PACKET_AUXDATA) to get
the incoming VLAN tag. Correct me if I'm wrong, but as far as I see the
first VLAN tag is always stripped - either in hardware or in
net/core/dev.c in __netif_receive_skb_core() - and stored in
skb->vlan_tci. Therefore, it won't be in the packet data anymore.
If I use the socket with ETH_P_ALL as protocol, everything works as
expected. But if I'm using an actual protocol number, instead of the
catch all, the tp_vlan_tci field inside the auxdata will be zero. I've
traced this to commit d4b812dea4a236f729526facf97df1a9d18e191c (vlan:
mask vlan prio bits):
+ if (unlikely(vlan_tx_tag_present(skb))) {
+ if (vlan_tx_tag_get_id(skb))
+ skb->pkt_type = PACKET_OTHERHOST;
+ /* Note: we might in the future use prio bits
+ * and set skb->priority like in vlan_do_receive()
+ * For the time being, just ignore Priority Code Point
+ */
+ skb->vlan_tci = 0;
+ }
The ptype_all callbacks (which are working as expected) are before this
code and the ptype_specific callbacks (which are not working) are after
this piece of code. I don't know it this is a bug or not, I guess it is
one, at least from the AF_PACKET socket point of view. If not, how I'm
supposed to get the original VLAN tag with socket(AF_PACKET,
my_protocol)?
I also don't understand the commit (message) as the subject suggesets
only the prio bits should be masked, but with "skb->vlan_tci = 0"
everything is masked. Therefore, I've put the original (hopefully the
mail addresses are still valid) authors in CC.
Thanks,
-michael
^ permalink raw reply
* Re: [PATCHv2 bpf-next 07/11] bpf: Add helper to retrieve socket in BPF
From: Daniel Borkmann @ 2018-09-24 12:12 UTC (permalink / raw)
To: Joe Stringer, ast
Cc: netdev, john.fastabend, tgraf, kafai, nitin.hande,
mauricio.vasquez
In-Reply-To: <20180921171043.20823-8-joe@wand.net.nz>
Hi Joe,
couple of comments inline:
On 09/21/2018 07:10 PM, Joe Stringer wrote:
> This patch adds new BPF helper functions, bpf_sk_lookup_tcp() and
> bpf_sk_lookup_udp() which allows BPF programs to find out if there is a
> socket listening on this host, and returns a socket pointer which the
> BPF program can then access to determine, for instance, whether to
> forward or drop traffic. bpf_sk_lookup_xxx() may take a reference on the
> socket, so when a BPF program makes use of this function, it must
> subsequently pass the returned pointer into the newly added sk_release()
> to return the reference.
>
> By way of example, the following pseudocode would filter inbound
> connections at XDP if there is no corresponding service listening for
> the traffic:
>
> struct bpf_sock_tuple tuple;
> struct bpf_sock_ops *sk;
>
> populate_tuple(ctx, &tuple); // Extract the 5tuple from the packet
> sk = bpf_sk_lookup_tcp(ctx, &tuple, sizeof tuple, netns, 0);
> if (!sk) {
> // Couldn't find a socket listening for this traffic. Drop.
> return TC_ACT_SHOT;
> }
> bpf_sk_release(sk, 0);
> return TC_ACT_OK;
>
> Signed-off-by: Joe Stringer <joe@wand.net.nz>
>
> ---
>
> v2: Rework 'struct bpf_sock_tuple' to allow passing a packet pointer
> Limit netns_id field to 32 bits
> Fix compile error with CONFIG_IPV6 enabled
> Allow direct packet access from helper
> ---
> include/uapi/linux/bpf.h | 57 ++++++++-
> kernel/bpf/verifier.c | 8 +-
> net/core/filter.c | 149 ++++++++++++++++++++++
> tools/include/uapi/linux/bpf.h | 57 ++++++++-
> tools/testing/selftests/bpf/bpf_helpers.h | 12 ++
> 5 files changed, 280 insertions(+), 3 deletions(-)
>
> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> index aa5ccd2385ed..620adbb09a94 100644
> --- a/include/uapi/linux/bpf.h
> +++ b/include/uapi/linux/bpf.h
> @@ -2143,6 +2143,41 @@ union bpf_attr {
> * request in the skb.
> * Return
> * 0 on success, or a negative error in case of failure.
> + *
> + * struct bpf_sock_ops *bpf_sk_lookup_tcp(ctx, tuple, tuple_size, netns, flags)
Nit: could you add proper signature like in other cases that are documented?
> + * Decription
> + * Look for TCP socket matching 'tuple'. The return value must
> + * be checked, and if non-NULL, released via bpf_sk_release().
> + * @ctx: pointer to ctx
> + * @tuple: pointer to struct bpf_sock_tuple
> + * @tuple_size: size of the tuple
> + * @netns: network namespace id
> + * @flags: flags value
Should probably say in all three cases that it's unused right now and reserved
for future.
> + * Return
> + * pointer to socket ops on success, or
> + * NULL in case of failure
> + *
> + * struct bpf_sock_ops *bpf_sk_lookup_udp(ctx, tuple, tuple_size, netns, flags)
> + * Decription
> + * Look for UDP socket matching 'tuple'. The return value must
> + * be checked, and if non-NULL, released via bpf_sk_release().
> + * @ctx: pointer to ctx
> + * @tuple: pointer to struct bpf_sock_tuple
> + * @tuple_size: size of the tuple
> + * @netns: network namespace id
> + * @flags: flags value
> + * Return
> + * pointer to socket ops on success, or
> + * NULL in case of failure
> + *
> + * int bpf_sk_release(sock, flags)
> + * Description
> + * Release the reference held by 'sock'.
> + * @sock: Pointer reference to release. Must be found via
> + * bpf_sk_lookup_xxx().
> + * @flags: flags value
> + * Return
> + * 0 on success, or a negative error in case of failure.
> */
> #define __BPF_FUNC_MAPPER(FN) \
> FN(unspec), \
> @@ -2228,7 +2263,10 @@ union bpf_attr {
> FN(get_current_cgroup_id), \
> FN(get_local_storage), \
> FN(sk_select_reuseport), \
> - FN(skb_ancestor_cgroup_id),
> + FN(skb_ancestor_cgroup_id), \
> + FN(sk_lookup_tcp), \
> + FN(sk_lookup_udp), \
> + FN(sk_release),
>
> /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> * function eBPF program intends to call
> @@ -2398,6 +2436,23 @@ struct bpf_sock {
> */
> };
>
> +struct bpf_sock_tuple {
> + union {
> + struct {
> + __be32 saddr;
> + __be32 daddr;
> + __be16 sport;
> + __be16 dport;
> + } ipv4;
> + struct {
> + __be32 saddr[4];
> + __be32 daddr[4];
> + __be16 sport;
> + __be16 dport;
> + } ipv6;
> + };
> +};
> +
> #define XDP_PACKET_HEADROOM 256
>
> /* User return codes for XDP prog type.
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 16818508b225..7b7fa94aba58 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -153,6 +153,12 @@ static const struct bpf_verifier_ops * const bpf_verifier_ops[] = {
> * PTR_TO_MAP_VALUE, PTR_TO_SOCKET_OR_NULL becomes PTR_TO_SOCKET when the type
> * passes through a NULL-check conditional. For the branch wherein the state is
> * changed to CONST_IMM, the verifier releases the reference.
> + *
> + * For each helper function that allocates a reference, such as
> + * bpf_sk_lookup_tcp(), there is a corresponding release function, such as
> + * bpf_sk_release(). When a reference type passes into the release function,
> + * the verifier also releases the reference. If any unchecked or unreleased
> + * reference remains at the end of the program, the verifier rejects it.
> */
>
> /* verifier_state + insn_idx are pushed to stack when branch is encountered */
> @@ -300,7 +306,7 @@ static bool arg_type_is_refcounted(enum bpf_arg_type type)
> */
> static bool is_release_function(enum bpf_func_id func_id)
> {
> - return false;
> + return func_id == BPF_FUNC_sk_release;
> }
>
> /* string representation of 'enum bpf_reg_type' */
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 43f81883f31d..f0715e2e7b07 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -58,13 +58,17 @@
> #include <net/busy_poll.h>
> #include <net/tcp.h>
> #include <net/xfrm.h>
> +#include <net/udp.h>
> #include <linux/bpf_trace.h>
> #include <net/xdp_sock.h>
> #include <linux/inetdevice.h>
> +#include <net/inet_hashtables.h>
> +#include <net/inet6_hashtables.h>
> #include <net/ip_fib.h>
> #include <net/flow.h>
> #include <net/arp.h>
> #include <net/ipv6.h>
> +#include <net/net_namespace.h>
> #include <linux/seg6_local.h>
> #include <net/seg6.h>
> #include <net/seg6_local.h>
> @@ -4812,6 +4816,139 @@ static const struct bpf_func_proto bpf_lwt_seg6_adjust_srh_proto = {
> };
> #endif /* CONFIG_IPV6_SEG6_BPF */
>
> +struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
> + struct sk_buff *skb, u8 family, u8 proto)
> +{
> + int dif = skb->dev->ifindex;
> + bool refcounted = false;
> + struct sock *sk = NULL;
> +
> + if (family == AF_INET) {
> + __be32 src4 = tuple->ipv4.saddr;
> + __be32 dst4 = tuple->ipv4.daddr;
> + int sdif = inet_sdif(skb);
> +
> + if (proto == IPPROTO_TCP)
> + sk = __inet_lookup(net, &tcp_hashinfo, skb, 0,
> + src4, tuple->ipv4.sport,
> + dst4, tuple->ipv4.dport,
> + dif, sdif, &refcounted);
> + else
> + sk = __udp4_lib_lookup(net, src4, tuple->ipv4.sport,
> + dst4, tuple->ipv4.dport,
> + dif, sdif, &udp_table, skb);
> +#if IS_ENABLED(CONFIG_IPV6)
> + } else {
> + struct in6_addr *src6 = (struct in6_addr *)&tuple->ipv6.saddr;
> + struct in6_addr *dst6 = (struct in6_addr *)&tuple->ipv6.daddr;
> + int sdif = inet6_sdif(skb);
> +
> + if (proto == IPPROTO_TCP)
> + sk = __inet6_lookup(net, &tcp_hashinfo, skb, 0,
> + src6, tuple->ipv6.sport,
> + dst6, tuple->ipv6.dport,
> + dif, sdif, &refcounted);
> + else
> + sk = __udp6_lib_lookup(net, src6, tuple->ipv6.sport,
> + dst6, tuple->ipv6.dport,
> + dif, sdif, &udp_table, skb);
> +#endif
> + }
> +
> + if (unlikely(sk && !refcounted && !sock_flag(sk, SOCK_RCU_FREE))) {
> + WARN_ONCE(1, "Found non-RCU, unreferenced socket!");
> + sk = NULL;
> + }
> + return sk;
> +}
> +
> +/* bpf_sk_lookup performs the core lookup for different types of sockets,
> + * taking a reference on the socket if it doesn't have the flag SOCK_RCU_FREE.
> + * Returns the socket as an 'unsigned long' to simplify the casting in the
> + * callers to satisfy BPF_CALL declarations.
> + */
> +static unsigned long
> +bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
> + u8 proto, u64 netns_id, u64 flags)
> +{
> + struct net *caller_net = dev_net(skb->dev);
For sk_skb programs, are we *always* guaranteed to have a skb->dev assigned?
This definitely holds true for tc programs, but afaik not for sk_skb ones where
you enable the helpers below, so this would result in a NULL ptr dereference.
> + struct sock *sk = NULL;
> + u8 family = AF_UNSPEC;
> + struct net *net;
> +
> + family = len == sizeof(tuple->ipv4) ? AF_INET : AF_INET6;
> + if (unlikely(family == AF_UNSPEC || netns_id > U32_MAX || flags))
> + goto out;
> +
> + if (netns_id)
> + net = get_net_ns_by_id(caller_net, netns_id);
> + else
> + net = caller_net;
> + if (unlikely(!net))
> + goto out;
> + sk = sk_lookup(net, tuple, skb, family, proto);
> + put_net(net);
> +
> + if (sk)
> + sk = sk_to_full_sk(sk);
> +out:
> + return (unsigned long) sk;
> +}
> +
> +BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
> + struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
> +{
> + return bpf_sk_lookup(skb, tuple, len, IPPROTO_TCP, netns_id, flags);
> +}
> +
> +static const struct bpf_func_proto bpf_sk_lookup_tcp_proto = {
> + .func = bpf_sk_lookup_tcp,
> + .gpl_only = false,
> + .pkt_access = true,
> + .ret_type = RET_PTR_TO_SOCKET_OR_NULL,
> + .arg1_type = ARG_PTR_TO_CTX,
> + .arg2_type = ARG_PTR_TO_MEM,
> + .arg3_type = ARG_CONST_SIZE,
> + .arg4_type = ARG_ANYTHING,
> + .arg5_type = ARG_ANYTHING,
> +};
> +
> +BPF_CALL_5(bpf_sk_lookup_udp, struct sk_buff *, skb,
> + struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
> +{
> + return bpf_sk_lookup(skb, tuple, len, IPPROTO_UDP, netns_id, flags);
> +}
> +
> +static const struct bpf_func_proto bpf_sk_lookup_udp_proto = {
> + .func = bpf_sk_lookup_udp,
> + .gpl_only = false,
> + .pkt_access = true,
> + .ret_type = RET_PTR_TO_SOCKET_OR_NULL,
> + .arg1_type = ARG_PTR_TO_CTX,
> + .arg2_type = ARG_PTR_TO_MEM,
> + .arg3_type = ARG_CONST_SIZE,
> + .arg4_type = ARG_ANYTHING,
> + .arg5_type = ARG_ANYTHING,
> +};
> +
> +BPF_CALL_2(bpf_sk_release, struct sock *, sk, u64, flags)
> +{
> + if (!sock_flag(sk, SOCK_RCU_FREE))
> + sock_gen_put(sk);
> +
> + if (unlikely(flags))
> + return -EINVAL;
I guess it's probably okay to leave here, though I'm wondering whether it's
worth to have a flags part in general in this helper. We need to release the
reference in any case beforehands anyway as we otherwise leak it. Personally,
I'd probably that arg here.
> + return 0;
> +}
> +
> +static const struct bpf_func_proto bpf_sk_release_proto = {
> + .func = bpf_sk_release,
> + .gpl_only = false,
> + .ret_type = RET_INTEGER,
> + .arg1_type = ARG_PTR_TO_SOCKET,
> + .arg2_type = ARG_ANYTHING,
> +};
> +
> bool bpf_helper_changes_pkt_data(void *func)
> {
> if (func == bpf_skb_vlan_push ||
> @@ -5018,6 +5155,12 @@ tc_cls_act_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> case BPF_FUNC_skb_ancestor_cgroup_id:
> return &bpf_skb_ancestor_cgroup_id_proto;
> #endif
> + case BPF_FUNC_sk_lookup_tcp:
> + return &bpf_sk_lookup_tcp_proto;
> + case BPF_FUNC_sk_lookup_udp:
> + return &bpf_sk_lookup_udp_proto;
> + case BPF_FUNC_sk_release:
> + return &bpf_sk_release_proto;
> default:
> return bpf_base_func_proto(func_id);
> }
> @@ -5118,6 +5261,12 @@ sk_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> return &bpf_sk_redirect_hash_proto;
> case BPF_FUNC_get_local_storage:
> return &bpf_get_local_storage_proto;
> + case BPF_FUNC_sk_lookup_tcp:
> + return &bpf_sk_lookup_tcp_proto;
> + case BPF_FUNC_sk_lookup_udp:
> + return &bpf_sk_lookup_udp_proto;
> + case BPF_FUNC_sk_release:
> + return &bpf_sk_release_proto;
(See comment above.)
> default:
> return bpf_base_func_proto(func_id);
> }
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index aa5ccd2385ed..620adbb09a94 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -2143,6 +2143,41 @@ union bpf_attr {
> * request in the skb.
> * Return
> * 0 on success, or a negative error in case of failure.
> + *
> + * struct bpf_sock_ops *bpf_sk_lookup_tcp(ctx, tuple, tuple_size, netns, flags)
> + * Decription
> + * Look for TCP socket matching 'tuple'. The return value must
> + * be checked, and if non-NULL, released via bpf_sk_release().
> + * @ctx: pointer to ctx
> + * @tuple: pointer to struct bpf_sock_tuple
> + * @tuple_size: size of the tuple
> + * @netns: network namespace id
> + * @flags: flags value
> + * Return
> + * pointer to socket ops on success, or
> + * NULL in case of failure
> + *
> + * struct bpf_sock_ops *bpf_sk_lookup_udp(ctx, tuple, tuple_size, netns, flags)
> + * Decription
> + * Look for UDP socket matching 'tuple'. The return value must
> + * be checked, and if non-NULL, released via bpf_sk_release().
> + * @ctx: pointer to ctx
> + * @tuple: pointer to struct bpf_sock_tuple
> + * @tuple_size: size of the tuple
> + * @netns: network namespace id
> + * @flags: flags value
> + * Return
> + * pointer to socket ops on success, or
> + * NULL in case of failure
> + *
> + * int bpf_sk_release(sock, flags)
> + * Description
> + * Release the reference held by 'sock'.
> + * @sock: Pointer reference to release. Must be found via
> + * bpf_sk_lookup_xxx().
> + * @flags: flags value
> + * Return
> + * 0 on success, or a negative error in case of failure.
> */
> #define __BPF_FUNC_MAPPER(FN) \
> FN(unspec), \
> @@ -2228,7 +2263,10 @@ union bpf_attr {
> FN(get_current_cgroup_id), \
> FN(get_local_storage), \
> FN(sk_select_reuseport), \
> - FN(skb_ancestor_cgroup_id),
> + FN(skb_ancestor_cgroup_id), \
> + FN(sk_lookup_tcp), \
> + FN(sk_lookup_udp), \
> + FN(sk_release),
>
> /* integer value in 'imm' field of BPF_CALL instruction selects which helper
> * function eBPF program intends to call
> @@ -2398,6 +2436,23 @@ struct bpf_sock {
> */
> };
>
> +struct bpf_sock_tuple {
> + union {
> + struct {
> + __be32 saddr;
> + __be32 daddr;
> + __be16 sport;
> + __be16 dport;
> + } ipv4;
> + struct {
> + __be32 saddr[4];
> + __be32 daddr[4];
> + __be16 sport;
> + __be16 dport;
> + } ipv6;
> + };
> +};
> +
> #define XDP_PACKET_HEADROOM 256
>
> /* User return codes for XDP prog type.
> diff --git a/tools/testing/selftests/bpf/bpf_helpers.h b/tools/testing/selftests/bpf/bpf_helpers.h
> index e4be7730222d..88ce00c3aa0f 100644
> --- a/tools/testing/selftests/bpf/bpf_helpers.h
> +++ b/tools/testing/selftests/bpf/bpf_helpers.h
> @@ -143,6 +143,18 @@ static unsigned long long (*bpf_skb_cgroup_id)(void *ctx) =
> (void *) BPF_FUNC_skb_cgroup_id;
> static unsigned long long (*bpf_skb_ancestor_cgroup_id)(void *ctx, int level) =
> (void *) BPF_FUNC_skb_ancestor_cgroup_id;
> +static struct bpf_sock *(*bpf_sk_lookup_tcp)(void *ctx,
> + struct bpf_sock_tuple *tuple,
> + int size, unsigned int netns_id,
> + unsigned long long flags) =
> + (void *) BPF_FUNC_sk_lookup_tcp;
> +static struct bpf_sock *(*bpf_sk_lookup_udp)(void *ctx,
> + struct bpf_sock_tuple *tuple,
> + int size, unsigned int netns_id,
> + unsigned long long flags) =
> + (void *) BPF_FUNC_sk_lookup_udp;
> +static int (*bpf_sk_release)(struct bpf_sock *sk, unsigned long long flags) =
> + (void *) BPF_FUNC_sk_release;
>
> /* llvm builtin functions that eBPF C program may use to
> * emit BPF_LD_ABS and BPF_LD_IND instructions
>
^ permalink raw reply
* Re: [PATCH] wireless: ipw2x00: Remove unnecessary parentheses
From: Stanislav Yakovlev @ 2018-09-24 18:21 UTC (permalink / raw)
To: Nathan Chancellor; +Cc: Kalle Valo, linux-wireless, netdev, linux-kernel
In-Reply-To: <20180920204548.2405-1-natechancellor@gmail.com>
On 21/09/2018, Nathan Chancellor <natechancellor@gmail.com> wrote:
> Clang warns when multiple pairs of parentheses are used for a single
> conditional statement.
>
> drivers/net/wireless/intel/ipw2x00/ipw2200.c:5655:28: warning: equality
> comparison with extraneous parentheses [-Wparentheses-equality]
> if ((priv->ieee->iw_mode == IW_MODE_ADHOC)) {
> ~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~
> drivers/net/wireless/intel/ipw2x00/ipw2200.c:5655:28: note: remove
> extraneous parentheses around the comparison to silence this warning
> if ((priv->ieee->iw_mode == IW_MODE_ADHOC)) {
> ~ ^ ~
> drivers/net/wireless/intel/ipw2x00/ipw2200.c:5655:28: note: use '=' to
> turn this equality comparison into an assignment
> if ((priv->ieee->iw_mode == IW_MODE_ADHOC)) {
> ^~
> =
> 1 warning generated.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/134
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> ---
> drivers/net/wireless/intel/ipw2x00/ipw2200.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Looks fine, thanks!
Stanislav.
^ permalink raw reply
* Re: [PATCHv2 bpf-next 08/11] selftests/bpf: Add tests for reference tracking
From: Daniel Borkmann @ 2018-09-24 12:21 UTC (permalink / raw)
To: Joe Stringer, ast
Cc: netdev, john.fastabend, tgraf, kafai, nitin.hande,
mauricio.vasquez
In-Reply-To: <20180921171043.20823-9-joe@wand.net.nz>
On 09/21/2018 07:10 PM, Joe Stringer wrote:
> reference tracking: leak potential reference
> reference tracking: leak potential reference on stack
> reference tracking: leak potential reference on stack 2
> reference tracking: zero potential reference
> reference tracking: copy and zero potential references
> reference tracking: release reference without check
> reference tracking: release reference
> reference tracking: release reference twice
> reference tracking: release reference twice inside branch
> reference tracking: alloc, check, free in one subbranch
> reference tracking: alloc, check, free in both subbranches
> reference tracking in call: free reference in subprog
> reference tracking in call: free reference in subprog and outside
> reference tracking in call: alloc & leak reference in subprog
> reference tracking in call: alloc in subprog, release outside
> reference tracking in call: sk_ptr leak into caller stack
> reference tracking in call: sk_ptr spill into caller stack
>
> Signed-off-by: Joe Stringer <joe@wand.net.nz>
> Acked-by: Alexei Starovoitov <ast@kernel.org>
> ---
> tools/testing/selftests/bpf/test_verifier.c | 359 ++++++++++++++++++++
> 1 file changed, 359 insertions(+)
I think this here needs to have some more test cases that we current do not track but
should in order to have better coverage. At minimum what comes to mind additionally:
- verifier interaction with LD_ABS, LD_IND
- verifier interaction with tail calls (e.g. try to leak socket, socket_or_null, etc,
but should also have a positive test where we drop ref before tail call to show it
works in combination)
- Try to mangle a socket and socket_or_null pointer with ALU ops and pass it to helper
- Try to access the socket data fields after we released its reference
- Access socket member fields in general (I think not present right now)
- Use direct packet access in combination with lookup helper (it's enabled
via pkt_access = true in the helper, so we should also test for it here to make
sure future changes don't break it)
- ...
^ permalink raw reply
* Re: vlan missing with AF_PACKET and auxdata
From: Jan Grashöfer @ 2018-09-24 12:22 UTC (permalink / raw)
To: Michael Walle, netdev
Cc: David S. Miller, Eric Dumazet, Steinar H. Gunderson, heiko.thiery
In-Reply-To: <9a284a4d29bb3301f50d7993c6bebb04@walle.cc>
Hi Michael,
On 24/09/2018 14:01, Michael Walle wrote:
> I'm using the AF_PACKET socket with setsockopt(PACKET_AUXDATA) to get
> the incoming VLAN tag. Correct me if I'm wrong, but as far as I see the
> first VLAN tag is always stripped - either in hardware or in
> net/core/dev.c in __netif_receive_skb_core() - and stored in
> skb->vlan_tci. Therefore, it won't be in the packet data anymore.
although the documentation says "SOCK_RAW packets are passed to and from
the device driver without any changes in the packet data." [1] that's
correct and was discussed here:
https://www.spinics.net/lists/netdev/msg440313.html
Result of the discussion: Won't fix, too complicated.
Jan
[1] http://man7.org/linux/man-pages/man7/packet.7.html
^ permalink raw reply
* Re: [PATCHv2 bpf-next 04/11] bpf: Add PTR_TO_SOCKET verifier type
From: Daniel Borkmann @ 2018-09-24 12:24 UTC (permalink / raw)
To: Joe Stringer, ast
Cc: netdev, john.fastabend, tgraf, kafai, nitin.hande,
mauricio.vasquez
In-Reply-To: <20180921171043.20823-5-joe@wand.net.nz>
On 09/21/2018 07:10 PM, Joe Stringer wrote:
> Teach the verifier a little bit about a new type of pointer, a
> PTR_TO_SOCKET. This pointer type is accessed from BPF through the
> 'struct bpf_sock' structure.
>
> Signed-off-by: Joe Stringer <joe@wand.net.nz>
>
> ---
>
> v2: Reuse reg_type_mismatch() in more places
> Reduce the number of passes at convert_ctx_access()
> ---
> include/linux/bpf.h | 17 +++++
> include/linux/bpf_verifier.h | 2 +
> kernel/bpf/verifier.c | 120 +++++++++++++++++++++++++++++++----
> net/core/filter.c | 30 +++++----
> 4 files changed, 143 insertions(+), 26 deletions(-)
>
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 988a00797bcd..daeb0d343d9c 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -154,6 +154,7 @@ enum bpf_arg_type {
>
> ARG_PTR_TO_CTX, /* pointer to context */
> ARG_ANYTHING, /* any (initialized) argument is ok */
> + ARG_PTR_TO_SOCKET, /* pointer to bpf_sock */
> };
>
> /* type of values returned from helper functions */
> @@ -162,6 +163,7 @@ enum bpf_return_type {
> RET_VOID, /* function doesn't return anything */
> RET_PTR_TO_MAP_VALUE, /* returns a pointer to map elem value */
> RET_PTR_TO_MAP_VALUE_OR_NULL, /* returns a pointer to map elem value or NULL */
> + RET_PTR_TO_SOCKET_OR_NULL, /* returns a pointer to a socket or NULL */
> };
>
> /* eBPF function prototype used by verifier to allow BPF_CALLs from eBPF programs
> @@ -213,6 +215,8 @@ enum bpf_reg_type {
> PTR_TO_PACKET, /* reg points to skb->data */
> PTR_TO_PACKET_END, /* skb->data + headlen */
> PTR_TO_FLOW_KEYS, /* reg points to bpf_flow_keys */
> + PTR_TO_SOCKET, /* reg points to struct bpf_sock */
> + PTR_TO_SOCKET_OR_NULL, /* reg points to struct bpf_sock or NULL */
> };
>
> /* The information passed from prog-specific *_is_valid_access
> @@ -335,6 +339,11 @@ const struct bpf_func_proto *bpf_get_trace_printk_proto(void);
>
> typedef unsigned long (*bpf_ctx_copy_t)(void *dst, const void *src,
> unsigned long off, unsigned long len);
> +typedef u32 (*bpf_convert_ctx_access_t)(enum bpf_access_type type,
> + const struct bpf_insn *src,
> + struct bpf_insn *dst,
> + struct bpf_prog *prog,
> + u32 *target_size);
>
> u64 bpf_event_output(struct bpf_map *map, u64 flags, void *meta, u64 meta_size,
> void *ctx, u64 ctx_size, bpf_ctx_copy_t ctx_copy);
> @@ -828,4 +837,12 @@ extern const struct bpf_func_proto bpf_get_local_storage_proto;
> void bpf_user_rnd_init_once(void);
> u64 bpf_user_rnd_u32(u64 r1, u64 r2, u64 r3, u64 r4, u64 r5);
>
> +bool bpf_sock_is_valid_access(int off, int size, enum bpf_access_type type,
> + struct bpf_insn_access_aux *info);
> +u32 bpf_sock_convert_ctx_access(enum bpf_access_type type,
> + const struct bpf_insn *si,
> + struct bpf_insn *insn_buf,
> + struct bpf_prog *prog,
> + u32 *target_size);
> +
> #endif /* _LINUX_BPF_H */
> diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
> index af262b97f586..23a2b17bfd75 100644
> --- a/include/linux/bpf_verifier.h
> +++ b/include/linux/bpf_verifier.h
> @@ -58,6 +58,8 @@ struct bpf_reg_state {
> * offset, so they can share range knowledge.
> * For PTR_TO_MAP_VALUE_OR_NULL this is used to share which map value we
> * came from, when one is tested for != NULL.
> + * For PTR_TO_SOCKET this is used to share which pointers retain the
> + * same reference to the socket, to determine proper reference freeing.
> */
> u32 id;
> /* For scalar types (SCALAR_VALUE), this represents our knowledge of
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 7dccb18ede03..1fee63d82290 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -80,8 +80,8 @@ static const struct bpf_verifier_ops * const bpf_verifier_ops[] = {
> * (like pointer plus pointer becomes SCALAR_VALUE type)
> *
> * When verifier sees load or store instructions the type of base register
> - * can be: PTR_TO_MAP_VALUE, PTR_TO_CTX, PTR_TO_STACK. These are three pointer
> - * types recognized by check_mem_access() function.
> + * can be: PTR_TO_MAP_VALUE, PTR_TO_CTX, PTR_TO_STACK, PTR_TO_SOCKET. These are
> + * four pointer types recognized by check_mem_access() function.
> *
> * PTR_TO_MAP_VALUE means that this register is pointing to 'map element value'
> * and the range of [ptr, ptr + map's value_size) is accessible.
> @@ -267,6 +267,8 @@ static const char * const reg_type_str[] = {
> [PTR_TO_PACKET_META] = "pkt_meta",
> [PTR_TO_PACKET_END] = "pkt_end",
> [PTR_TO_FLOW_KEYS] = "flow_keys",
> + [PTR_TO_SOCKET] = "sock",
> + [PTR_TO_SOCKET_OR_NULL] = "sock_or_null",
> };
>
> static char slot_type_char[] = {
> @@ -973,6 +975,8 @@ static bool is_spillable_regtype(enum bpf_reg_type type)
> case PTR_TO_PACKET_END:
> case PTR_TO_FLOW_KEYS:
> case CONST_PTR_TO_MAP:
> + case PTR_TO_SOCKET:
> + case PTR_TO_SOCKET_OR_NULL:
> return true;
> default:
> return false;
> @@ -1341,6 +1345,28 @@ static int check_flow_keys_access(struct bpf_verifier_env *env, int off,
> return 0;
> }
>
> +static int check_sock_access(struct bpf_verifier_env *env, u32 regno, int off,
> + int size, enum bpf_access_type t)
> +{
> + struct bpf_reg_state *regs = cur_regs(env);
> + struct bpf_reg_state *reg = ®s[regno];
> + struct bpf_insn_access_aux info;
> +
> + if (reg->smin_value < 0) {
> + verbose(env, "R%d min value is negative, either use unsigned index or do a if (index >=0) check.\n",
> + regno);
> + return -EACCES;
> + }
> +
> + if (!bpf_sock_is_valid_access(off, size, t, &info)) {
> + verbose(env, "invalid bpf_sock_ops access off=%d size=%d\n",
> + off, size);
> + return -EACCES;
> + }
> +
> + return 0;
> +}
> +
> static bool __is_pointer_value(bool allow_ptr_leaks,
> const struct bpf_reg_state *reg)
> {
> @@ -1459,6 +1485,9 @@ static int check_ptr_alignment(struct bpf_verifier_env *env,
> */
> strict = true;
> break;
> + case PTR_TO_SOCKET:
> + pointer_desc = "sock ";
> + break;
> default:
> break;
> }
> @@ -1726,6 +1755,14 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn
> err = check_flow_keys_access(env, off, size);
> if (!err && t == BPF_READ && value_regno >= 0)
> mark_reg_unknown(env, regs, value_regno);
> + } else if (reg->type == PTR_TO_SOCKET) {
> + if (t == BPF_WRITE) {
> + verbose(env, "cannot write into socket\n");
> + return -EACCES;
> + }
> + err = check_sock_access(env, regno, off, size, t);
> + if (!err && value_regno >= 0)
> + mark_reg_unknown(env, regs, value_regno);
> } else {
> verbose(env, "R%d invalid mem access '%s'\n", regno,
> reg_type_str[reg->type]);
> @@ -1949,6 +1986,10 @@ static int check_func_arg(struct bpf_verifier_env *env, u32 regno,
> err = check_ctx_reg(env, reg, regno);
> if (err < 0)
> return err;
> + } else if (arg_type == ARG_PTR_TO_SOCKET) {
> + expected_type = PTR_TO_SOCKET;
> + if (type != expected_type)
> + goto err_type;
> } else if (arg_type_is_mem_ptr(arg_type)) {
> expected_type = PTR_TO_STACK;
> /* One exception here. In case function allows for NULL to be
> @@ -2542,6 +2583,10 @@ static int check_helper_call(struct bpf_verifier_env *env, int func_id, int insn
> }
> regs[BPF_REG_0].map_ptr = meta.map_ptr;
> regs[BPF_REG_0].id = ++env->id_gen;
> + } else if (fn->ret_type == RET_PTR_TO_SOCKET_OR_NULL) {
> + mark_reg_known_zero(env, regs, BPF_REG_0);
> + regs[BPF_REG_0].type = PTR_TO_SOCKET_OR_NULL;
> + regs[BPF_REG_0].id = ++env->id_gen;
> } else {
> verbose(env, "unknown return type %d of func %s#%d\n",
> fn->ret_type, func_id_name(func_id), func_id);
> @@ -2679,6 +2724,8 @@ static int adjust_ptr_min_max_vals(struct bpf_verifier_env *env,
> return -EACCES;
> case CONST_PTR_TO_MAP:
> case PTR_TO_PACKET_END:
> + case PTR_TO_SOCKET:
> + case PTR_TO_SOCKET_OR_NULL:
> verbose(env, "R%d pointer arithmetic on %s prohibited\n",
> dst, reg_type_str[ptr_reg->type]);
> return -EACCES;
> @@ -3626,6 +3673,8 @@ static void mark_ptr_or_null_reg(struct bpf_reg_state *reg, u32 id,
> } else {
> reg->type = PTR_TO_MAP_VALUE;
> }
> + } else if (reg->type == PTR_TO_SOCKET_OR_NULL) {
> + reg->type = PTR_TO_SOCKET;
> }
> /* We don't need id from this point onwards anymore, thus we
> * should better reset it, so that state pruning has chances
> @@ -4401,6 +4450,8 @@ static bool regsafe(struct bpf_reg_state *rold, struct bpf_reg_state *rcur,
> case CONST_PTR_TO_MAP:
> case PTR_TO_PACKET_END:
> case PTR_TO_FLOW_KEYS:
> + case PTR_TO_SOCKET:
> + case PTR_TO_SOCKET_OR_NULL:
> /* Only valid matches are exact, which memcmp() above
> * would have accepted
> */
> @@ -4678,6 +4729,37 @@ static int is_state_visited(struct bpf_verifier_env *env, int insn_idx)
> return 0;
> }
>
> +/* Return true if it's OK to have the same insn return a different type. */
> +static bool reg_type_mismatch_ok(enum bpf_reg_type type)
> +{
> + switch (type) {
> + case PTR_TO_CTX:
> + case PTR_TO_SOCKET:
> + case PTR_TO_SOCKET_OR_NULL:
> + return false;
> + default:
> + return true;
> + }
> +}
> +
> +/* If an instruction was previously used with particular pointer types, then we
> + * need to be careful to avoid cases such as the below, where it may be ok
> + * for one branch accessing the pointer, but not ok for the other branch:
> + *
> + * R1 = sock_ptr
> + * goto X;
> + * ...
> + * R1 = some_other_valid_ptr;
> + * goto X;
> + * ...
> + * R2 = *(u32 *)(R1 + 0);
> + */
> +static bool reg_type_mismatch(enum bpf_reg_type src, enum bpf_reg_type prev)
> +{
> + return src != prev && (!reg_type_mismatch_ok(src) ||
> + !reg_type_mismatch_ok(prev));
> +}
> +
> static int do_check(struct bpf_verifier_env *env)
> {
> struct bpf_verifier_state *state;
> @@ -4810,9 +4892,7 @@ static int do_check(struct bpf_verifier_env *env)
> */
> *prev_src_type = src_reg_type;
>
> - } else if (src_reg_type != *prev_src_type &&
> - (src_reg_type == PTR_TO_CTX ||
> - *prev_src_type == PTR_TO_CTX)) {
> + } else if (reg_type_mismatch(src_reg_type, *prev_src_type)) {
> /* ABuser program is trying to use the same insn
> * dst_reg = *(u32*) (src_reg + off)
> * with different pointer types:
> @@ -4857,9 +4937,7 @@ static int do_check(struct bpf_verifier_env *env)
>
> if (*prev_dst_type == NOT_INIT) {
> *prev_dst_type = dst_reg_type;
> - } else if (dst_reg_type != *prev_dst_type &&
> - (dst_reg_type == PTR_TO_CTX ||
> - *prev_dst_type == PTR_TO_CTX)) {
> + } else if (reg_type_mismatch(dst_reg_type, *prev_dst_type)) {
> verbose(env, "same insn cannot be used with different pointers\n");
> return -EINVAL;
> }
> @@ -5276,8 +5354,10 @@ static void sanitize_dead_code(struct bpf_verifier_env *env)
> }
> }
>
> -/* convert load instructions that access fields of 'struct __sk_buff'
> - * into sequence of instructions that access fields of 'struct sk_buff'
> +/* convert load instructions that access fields of a context type into a
> + * sequence of instructions that access fields of the underlying structure:
> + * struct __sk_buff -> struct sk_buff
> + * struct bpf_sock_ops -> struct sock
> */
> static int convert_ctx_accesses(struct bpf_verifier_env *env)
> {
> @@ -5306,12 +5386,14 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
> }
> }
>
> - if (!ops->convert_ctx_access || bpf_prog_is_dev_bound(env->prog->aux))
> + if (bpf_prog_is_dev_bound(env->prog->aux))
> return 0;
>
> insn = env->prog->insnsi + delta;
>
> for (i = 0; i < insn_cnt; i++, insn++) {
> + bpf_convert_ctx_access_t convert_ctx_access;
> +
> if (insn->code == (BPF_LDX | BPF_MEM | BPF_B) ||
> insn->code == (BPF_LDX | BPF_MEM | BPF_H) ||
> insn->code == (BPF_LDX | BPF_MEM | BPF_W) ||
> @@ -5353,8 +5435,18 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
> continue;
> }
>
> - if (env->insn_aux_data[i + delta].ptr_type != PTR_TO_CTX)
> + switch (env->insn_aux_data[i + delta].ptr_type) {
> + case PTR_TO_CTX:
> + if (!ops->convert_ctx_access)
> + continue;
> + convert_ctx_access = ops->convert_ctx_access;
> + break;
> + case PTR_TO_SOCKET:
> + convert_ctx_access = bpf_sock_convert_ctx_access;
> + break;
> + default:
> continue;
> + }
>
> ctx_field_size = env->insn_aux_data[i + delta].ctx_field_size;
> size = BPF_LDST_BYTES(insn);
When you compile without NET, this would need to have a stub implementation as
otherwise it might break build. We recently tripped over it in bpf flow dissector
as well.
^ permalink raw reply
* Re: [PATCHv2 bpf-next 07/11] bpf: Add helper to retrieve socket in BPF
From: Daniel Borkmann @ 2018-09-24 12:38 UTC (permalink / raw)
To: Joe Stringer, ast
Cc: netdev, john.fastabend, tgraf, kafai, nitin.hande,
mauricio.vasquez
In-Reply-To: <d2b803a4-509f-6d7f-1177-f3ccc1f875eb@iogearbox.net>
On 09/24/2018 02:12 PM, Daniel Borkmann wrote:
> Hi Joe,
>
> couple of comments inline:
>
> On 09/21/2018 07:10 PM, Joe Stringer wrote:
>> This patch adds new BPF helper functions, bpf_sk_lookup_tcp() and
>> bpf_sk_lookup_udp() which allows BPF programs to find out if there is a
>> socket listening on this host, and returns a socket pointer which the
>> BPF program can then access to determine, for instance, whether to
>> forward or drop traffic. bpf_sk_lookup_xxx() may take a reference on the
>> socket, so when a BPF program makes use of this function, it must
>> subsequently pass the returned pointer into the newly added sk_release()
>> to return the reference.
>>
>> By way of example, the following pseudocode would filter inbound
>> connections at XDP if there is no corresponding service listening for
>> the traffic:
>>
>> struct bpf_sock_tuple tuple;
>> struct bpf_sock_ops *sk;
>>
>> populate_tuple(ctx, &tuple); // Extract the 5tuple from the packet
>> sk = bpf_sk_lookup_tcp(ctx, &tuple, sizeof tuple, netns, 0);
>> if (!sk) {
>> // Couldn't find a socket listening for this traffic. Drop.
>> return TC_ACT_SHOT;
>> }
>> bpf_sk_release(sk, 0);
>> return TC_ACT_OK;
>>
>> Signed-off-by: Joe Stringer <joe@wand.net.nz>
>>
>> ---
>>
>> v2: Rework 'struct bpf_sock_tuple' to allow passing a packet pointer
>> Limit netns_id field to 32 bits
>> Fix compile error with CONFIG_IPV6 enabled
>> Allow direct packet access from helper
>> ---
>> include/uapi/linux/bpf.h | 57 ++++++++-
>> kernel/bpf/verifier.c | 8 +-
>> net/core/filter.c | 149 ++++++++++++++++++++++
>> tools/include/uapi/linux/bpf.h | 57 ++++++++-
>> tools/testing/selftests/bpf/bpf_helpers.h | 12 ++
>> 5 files changed, 280 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
>> index aa5ccd2385ed..620adbb09a94 100644
>> --- a/include/uapi/linux/bpf.h
>> +++ b/include/uapi/linux/bpf.h
>> @@ -2143,6 +2143,41 @@ union bpf_attr {
>> * request in the skb.
>> * Return
>> * 0 on success, or a negative error in case of failure.
>> + *
>> + * struct bpf_sock_ops *bpf_sk_lookup_tcp(ctx, tuple, tuple_size, netns, flags)
>
> Nit: could you add proper signature like in other cases that are documented?
>
>> + * Decription
>> + * Look for TCP socket matching 'tuple'. The return value must
>> + * be checked, and if non-NULL, released via bpf_sk_release().
>> + * @ctx: pointer to ctx
>> + * @tuple: pointer to struct bpf_sock_tuple
>> + * @tuple_size: size of the tuple
>> + * @netns: network namespace id
>> + * @flags: flags value
>
> Should probably say in all three cases that it's unused right now and reserved
> for future.
I think the two lookup helpers here also need to have a better documentation in
terms of semantics wrt netns, so it definitely needs to be documented when it's
derived from the skb's dev and when netns is used.
>> + * Return
>> + * pointer to socket ops on success, or
>> + * NULL in case of failure
>> + *
>> + * struct bpf_sock_ops *bpf_sk_lookup_udp(ctx, tuple, tuple_size, netns, flags)
>> + * Decription
>> + * Look for UDP socket matching 'tuple'. The return value must
>> + * be checked, and if non-NULL, released via bpf_sk_release().
>> + * @ctx: pointer to ctx
>> + * @tuple: pointer to struct bpf_sock_tuple
>> + * @tuple_size: size of the tuple
>> + * @netns: network namespace id
>> + * @flags: flags value
>> + * Return
>> + * pointer to socket ops on success, or
>> + * NULL in case of failure
>> + *
>> + * int bpf_sk_release(sock, flags)
>> + * Description
>> + * Release the reference held by 'sock'.
>> + * @sock: Pointer reference to release. Must be found via
>> + * bpf_sk_lookup_xxx().
>> + * @flags: flags value
>> + * Return
>> + * 0 on success, or a negative error in case of failure.
>> */
^ permalink raw reply
* OFFICE
From: OFFICE @ 2018-09-24 12:40 UTC (permalink / raw)
[-- Attachment #1: Type: text/plain, Size: 1 bytes --]
[-- Attachment #2: OFFICE.png --]
[-- Type: image/png, Size: 39871 bytes --]
^ permalink raw reply
* Re: [PATCHv2 bpf-next 07/11] bpf: Add helper to retrieve socket in BPF
From: Daniel Borkmann @ 2018-09-24 12:51 UTC (permalink / raw)
To: Joe Stringer, ast
Cc: netdev, john.fastabend, tgraf, kafai, nitin.hande,
mauricio.vasquez
In-Reply-To: <d2b803a4-509f-6d7f-1177-f3ccc1f875eb@iogearbox.net>
On 09/24/2018 02:12 PM, Daniel Borkmann wrote:
> On 09/21/2018 07:10 PM, Joe Stringer wrote:
[...]
>> +/* bpf_sk_lookup performs the core lookup for different types of sockets,
>> + * taking a reference on the socket if it doesn't have the flag SOCK_RCU_FREE.
>> + * Returns the socket as an 'unsigned long' to simplify the casting in the
>> + * callers to satisfy BPF_CALL declarations.
>> + */
>> +static unsigned long
>> +bpf_sk_lookup(struct sk_buff *skb, struct bpf_sock_tuple *tuple, u32 len,
>> + u8 proto, u64 netns_id, u64 flags)
>> +{
>> + struct net *caller_net = dev_net(skb->dev);
>
> For sk_skb programs, are we *always* guaranteed to have a skb->dev assigned?
>
> This definitely holds true for tc programs, but afaik not for sk_skb ones where
> you enable the helpers below, so this would result in a NULL ptr dereference.
>
>> + struct sock *sk = NULL;
>> + u8 family = AF_UNSPEC;
>> + struct net *net;
>> +
>> + family = len == sizeof(tuple->ipv4) ? AF_INET : AF_INET6;
>> + if (unlikely(family == AF_UNSPEC || netns_id > U32_MAX || flags))
>> + goto out;
>> +
>> + if (netns_id)
>> + net = get_net_ns_by_id(caller_net, netns_id);
>> + else
>> + net = caller_net;
>> + if (unlikely(!net))
>> + goto out;
>> + sk = sk_lookup(net, tuple, skb, family, proto);
>> + put_net(net);
Hmm, isn't this also resulting in a use-after-free on net ?
Presume we use dev_net(skb->dev) as net. It does read_pnet(&dev->nd_net), which
either gets you pnet->net when netns is configured or &init_net otherwise.
I don't see where an additional reference was taken in this path, hence you'll
get an imbalance which triggers freeing the underlying netns ..
>> + if (sk)
>> + sk = sk_to_full_sk(sk);
>> +out:
>> + return (unsigned long) sk;
>> +}
>> +
>> +BPF_CALL_5(bpf_sk_lookup_tcp, struct sk_buff *, skb,
>> + struct bpf_sock_tuple *, tuple, u32, len, u64, netns_id, u64, flags)
>> +{
>> + return bpf_sk_lookup(skb, tuple, len, IPPROTO_TCP, netns_id, flags);
>> +}
^ permalink raw reply
* [PATCH v2 net-next] tcp: expose sk_state in tcp_retransmit_skb tracepoint
From: Yafang Shao @ 2018-09-24 12:57 UTC (permalink / raw)
To: edumazet, davem; +Cc: netdev, linux-kernel, Yafang Shao
After sk_state exposed, we can get in which state this retransmission
occurs. That could give us more detail for dignostic.
For example, if this retransmission occurs in SYN_SENT state, it may
also indicates that the syn packet may be dropped on the remote peer due
to syn backlog queue full and then we could check the remote peer.
BTW,SYNACK retransmission is traced in tcp_retransmit_synack tracepoint.
Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
---
include/trace/events/tcp.h | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/include/trace/events/tcp.h b/include/trace/events/tcp.h
index ac55b32..2bc9960 100644
--- a/include/trace/events/tcp.h
+++ b/include/trace/events/tcp.h
@@ -56,6 +56,7 @@
TP_STRUCT__entry(
__field(const void *, skbaddr)
__field(const void *, skaddr)
+ __field(int, state)
__field(__u16, sport)
__field(__u16, dport)
__array(__u8, saddr, 4)
@@ -70,6 +71,7 @@
__entry->skbaddr = skb;
__entry->skaddr = sk;
+ __entry->state = sk->sk_state;
__entry->sport = ntohs(inet->inet_sport);
__entry->dport = ntohs(inet->inet_dport);
@@ -84,9 +86,10 @@
sk->sk_v6_rcv_saddr, sk->sk_v6_daddr);
),
- TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c",
+ TP_printk("sport=%hu dport=%hu saddr=%pI4 daddr=%pI4 saddrv6=%pI6c daddrv6=%pI6c state=%s\n",
__entry->sport, __entry->dport, __entry->saddr, __entry->daddr,
- __entry->saddr_v6, __entry->daddr_v6)
+ __entry->saddr_v6, __entry->daddr_v6,
+ show_tcp_state_name(__entry->state))
);
DEFINE_EVENT(tcp_event_sk_skb, tcp_retransmit_skb,
--
1.8.3.1
^ permalink raw reply related
* [PATCH v2 0/7] include: add setbits32/clrbits32/clrsetbits32/setbits64/clrbits64/clrsetbits64
From: Corentin Labbe @ 2018-09-24 19:04 UTC (permalink / raw)
To: Gilles.Muller, Julia.Lawall, agust, airlied, alexandre.torgue,
alistair, benh, carlo, davem, galak, joabreu, khilman,
maxime.ripard, michal.lkml, mpe, mporter, narmstrong,
nicolas.palix, oss, paulus, peppe.cavallaro, tj, vitb, wens
Cc: cocci, dri-devel, linux-amlogic, linux-arm-kernel, linux-ide,
linux-kernel, linuxppc-dev, netdev, Corentin Labbe
Hello
This patchset adds a new set of functions which are open-coded in lot of
place.
Basicly the pattern is always the same, "read, modify a bit, write"
some driver and the powerpc arch already have thoses pattern them as functions. (like ahci_sunxi.c or dwmac-meson8b)
The first patch rename some powerpc funtions for being consistent with
the new name convention.
The second patch adds the header with all setbits functions.
The third patch is a try to implement a coccinelle semantic patch to
find all place where xxxbits function could be used.
It should not be merged since it is un-finalized.
For the moment, the "add setbits.h header" is not working and need a future
coccinelle version.
The four last patch are example of some drivers convertion.
Thoses patchs give an example of the reduction of code won by using xxxbits32.
I would like to thanks Julia Lawall for her help on the coccinelle
patch.
Regards
Changes since v1:
- renamed LE functions to _leXX
- updated coccinnelle patch with JLawall's comments
Corentin Labbe (7):
powerpc: rename setbits32/clrbits32 to setbits32_be/clrbits32_be
include: add
setbits32/clrbits32/clrsetbits32/setbits64/clrbits64/clrsetbits64 in
linux/setbits.h
coccinelle: add xxxsetbitsXX converting spatch
ata: ahci_sunxi: use xxxsetbits32 functions
net: ethernet: stmmac: dwmac-sun8i: use xxxsetbits32
drm: meson: use xxxsetbits32
net: stmmac: dwmac-meson8b: use xxxsetbits32
arch/powerpc/include/asm/fsl_lbc.h | 2 +-
arch/powerpc/include/asm/io.h | 5 +-
arch/powerpc/platforms/44x/canyonlands.c | 4 +-
arch/powerpc/platforms/4xx/gpio.c | 28 +-
arch/powerpc/platforms/512x/pdm360ng.c | 6 +-
arch/powerpc/platforms/52xx/mpc52xx_common.c | 6 +-
arch/powerpc/platforms/52xx/mpc52xx_gpt.c | 10 +-
arch/powerpc/platforms/82xx/ep8248e.c | 2 +-
arch/powerpc/platforms/82xx/km82xx.c | 6 +-
arch/powerpc/platforms/82xx/mpc8272_ads.c | 10 +-
arch/powerpc/platforms/82xx/pq2.c | 2 +-
arch/powerpc/platforms/82xx/pq2ads-pci-pic.c | 4 +-
arch/powerpc/platforms/82xx/pq2fads.c | 10 +-
arch/powerpc/platforms/83xx/km83xx.c | 6 +-
arch/powerpc/platforms/83xx/mpc836x_mds.c | 2 +-
arch/powerpc/platforms/85xx/mpc85xx_mds.c | 2 +-
arch/powerpc/platforms/85xx/mpc85xx_pm_ops.c | 4 +-
arch/powerpc/platforms/85xx/mpc85xx_rdb.c | 2 +-
arch/powerpc/platforms/85xx/p1022_ds.c | 4 +-
arch/powerpc/platforms/85xx/p1022_rdk.c | 4 +-
arch/powerpc/platforms/85xx/t1042rdb_diu.c | 4 +-
arch/powerpc/platforms/85xx/twr_p102x.c | 2 +-
arch/powerpc/platforms/86xx/mpc8610_hpcd.c | 4 +-
arch/powerpc/platforms/8xx/adder875.c | 2 +-
arch/powerpc/platforms/8xx/m8xx_setup.c | 4 +-
arch/powerpc/platforms/8xx/mpc86xads_setup.c | 4 +-
arch/powerpc/platforms/8xx/mpc885ads_setup.c | 28 +-
arch/powerpc/platforms/embedded6xx/flipper-pic.c | 6 +-
arch/powerpc/platforms/embedded6xx/hlwd-pic.c | 8 +-
arch/powerpc/platforms/embedded6xx/wii.c | 10 +-
arch/powerpc/sysdev/cpm1.c | 26 +-
arch/powerpc/sysdev/cpm2.c | 16 +-
arch/powerpc/sysdev/cpm_common.c | 4 +-
arch/powerpc/sysdev/fsl_85xx_l2ctlr.c | 8 +-
arch/powerpc/sysdev/fsl_lbc.c | 2 +-
arch/powerpc/sysdev/fsl_pci.c | 8 +-
arch/powerpc/sysdev/fsl_pmc.c | 2 +-
arch/powerpc/sysdev/fsl_rcpm.c | 74 ++--
arch/powerpc/sysdev/fsl_rio.c | 4 +-
arch/powerpc/sysdev/fsl_rmu.c | 8 +-
arch/powerpc/sysdev/mpic_timer.c | 12 +-
drivers/ata/ahci_sunxi.c | 51 +--
drivers/gpu/drm/meson/meson_crtc.c | 14 +-
drivers/gpu/drm/meson/meson_dw_hdmi.c | 33 +-
drivers/gpu/drm/meson/meson_plane.c | 13 +-
drivers/gpu/drm/meson/meson_registers.h | 3 -
drivers/gpu/drm/meson/meson_venc.c | 13 +-
drivers/gpu/drm/meson/meson_venc_cvbs.c | 4 +-
drivers/gpu/drm/meson/meson_viu.c | 65 +--
drivers/gpu/drm/meson/meson_vpp.c | 22 +-
.../net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 56 +--
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 62 +--
include/linux/setbits.h | 88 ++++
scripts/add_new_include_in_source.py | 61 +++
scripts/coccinelle/misc/setbits32.cocci | 487 +++++++++++++++++++++
scripts/coccinelle/misc/setbits32_relaxed.cocci | 487 +++++++++++++++++++++
scripts/coccinelle/misc/setbits64.cocci | 487 +++++++++++++++++++++
scripts/coccinelle/misc/setbits_dev.cocci | 235 ++++++++++
58 files changed, 2159 insertions(+), 377 deletions(-)
create mode 100644 include/linux/setbits.h
create mode 100755 scripts/add_new_include_in_source.py
create mode 100644 scripts/coccinelle/misc/setbits32.cocci
create mode 100644 scripts/coccinelle/misc/setbits32_relaxed.cocci
create mode 100644 scripts/coccinelle/misc/setbits64.cocci
create mode 100644 scripts/coccinelle/misc/setbits_dev.cocci
--
2.16.4
^ permalink raw reply
* [PATCH v2 2/7] include: add setbits32/clrbits32/clrsetbits32/setbits64/clrbits64/clrsetbits64 in linux/setbits.h
From: Corentin Labbe @ 2018-09-24 19:04 UTC (permalink / raw)
To: Gilles.Muller, Julia.Lawall, agust, airlied, alexandre.torgue,
alistair, benh, carlo, davem, galak, joabreu, khilman,
maxime.ripard, michal.lkml, mpe, mporter, narmstrong,
nicolas.palix, oss, paulus, peppe.cavallaro, tj, vitb, wens
Cc: cocci, dri-devel, linux-amlogic, linux-arm-kernel, linux-ide,
linux-kernel, linuxppc-dev, netdev, Corentin Labbe
In-Reply-To: <1537815856-31728-1-git-send-email-clabbe@baylibre.com>
This patch adds setbits32/clrbits32/clrsetbits32 and
setbits64/clrbits64/clrsetbits64 in linux/setbits.h header.
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
include/linux/setbits.h | 88 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 88 insertions(+)
create mode 100644 include/linux/setbits.h
diff --git a/include/linux/setbits.h b/include/linux/setbits.h
new file mode 100644
index 000000000000..6e7e257134ae
--- /dev/null
+++ b/include/linux/setbits.h
@@ -0,0 +1,88 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __LINUX_SETBITS_H
+#define __LINUX_SETBITS_H
+
+#include <linux/io.h>
+
+#define __setbits(readfunction, writefunction, addr, set) \
+ writefunction((readfunction(addr) | (set)), addr)
+#define __clrbits(readfunction, writefunction, addr, mask) \
+ writefunction((readfunction(addr) & ~(mask)), addr)
+#define __clrsetbits(readfunction, writefunction, addr, mask, set) \
+ writefunction(((readfunction(addr) & ~(mask)) | (set)), addr)
+#define __setclrbits(readfunction, writefunction, addr, mask, set) \
+ writefunction(((readfunction(addr) | (set)) & ~(mask)), addr)
+
+#ifndef setbits_le32
+#define setbits_le32(addr, set) __setbits(readl, writel, addr, set)
+#endif
+#ifndef setbits_le32_relaxed
+#define setbits_le32_relaxed(addr, set) __setbits(readl_relaxed, writel_relaxed, \
+ addr, set)
+#endif
+
+#ifndef clrbits_le32
+#define clrbits_le32(addr, mask) __clrbits(readl, writel, addr, mask)
+#endif
+#ifndef clrbits_le32_relaxed
+#define clrbits_le32_relaxed(addr, mask) __clrbits(readl_relaxed, writel_relaxed, \
+ addr, mask)
+#endif
+
+#ifndef clrsetbits_le32
+#define clrsetbits_le32(addr, mask, set) __clrsetbits(readl, writel, addr, mask, set)
+#endif
+#ifndef clrsetbits_le32_relaxed
+#define clrsetbits_le32_relaxed(addr, mask, set) __clrsetbits(readl_relaxed, \
+ writel_relaxed, \
+ addr, mask, set)
+#endif
+
+#ifndef setclrbits_le32
+#define setclrbits_le32(addr, mask, set) __setclrbits(readl, writel, addr, mask, set)
+#endif
+#ifndef setclrbits_le32_relaxed
+#define setclrbits_le32_relaxed(addr, mask, set) __setclrbits(readl_relaxed, \
+ writel_relaxed, \
+ addr, mask, set)
+#endif
+
+/* We cannot use CONFIG_64BIT as some x86 drivers use non-atomicwriteq() */
+#if defined(writeq) && defined(readq)
+#ifndef setbits_le64
+#define setbits_le64(addr, set) __setbits(readq, writeq, addr, set)
+#endif
+#ifndef setbits_le64_relaxed
+#define setbits_le64_relaxed(addr, set) __setbits(readq_relaxed, writeq_relaxed, \
+ addr, set)
+#endif
+
+#ifndef clrbits_le64
+#define clrbits_le64(addr, mask) __clrbits(readq, writeq, addr, mask)
+#endif
+#ifndef clrbits_le64_relaxed
+#define clrbits_le64_relaxed(addr, mask) __clrbits(readq_relaxed, writeq_relaxed, \
+ addr, mask)
+#endif
+
+#ifndef clrsetbits_le64
+#define clrsetbits_le64(addr, mask, set) __clrsetbits(readq, writeq, addr, mask, set)
+#endif
+#ifndef clrsetbits_le64_relaxed
+#define clrsetbits_le64_relaxed(addr, mask, set) __clrsetbits(readq_relaxed, \
+ writeq_relaxed, \
+ addr, mask, set)
+#endif
+
+#ifndef setclrbits_le64
+#define setclrbits_le64(addr, mask, set) __setclrbits(readq, writeq, addr, mask, set)
+#endif
+#ifndef setclrbits_le64_relaxed
+#define setclrbits_le64_relaxed(addr, mask, set) __setclrbits(readq_relaxed, \
+ writeq_relaxed, \
+ addr, mask, set)
+#endif
+
+#endif /* writeq/readq */
+
+#endif /* __LINUX_SETBITS_H */
--
2.16.4
^ permalink raw reply related
* [PATCH v2 5/7] net: ethernet: stmmac: dwmac-sun8i: use xxxsetbits32
From: Corentin Labbe @ 2018-09-24 19:04 UTC (permalink / raw)
To: Gilles.Muller, Julia.Lawall, agust, airlied, alexandre.torgue,
alistair, benh, carlo, davem, galak, joabreu, khilman,
maxime.ripard, michal.lkml, mpe, mporter, narmstrong,
nicolas.palix, oss, paulus, peppe.cavallaro, tj, vitb, wens
Cc: cocci, dri-devel, linux-amlogic, linux-arm-kernel, linux-ide,
linux-kernel, linuxppc-dev, netdev, Corentin Labbe
In-Reply-To: <1537815856-31728-1-git-send-email-clabbe@baylibre.com>
This patch convert dwmac-sun8i driver to use all xxxsetbits32 functions.
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 62 ++++++-----------------
1 file changed, 16 insertions(+), 46 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index f9a61f90cfbc..74067a59af50 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -28,6 +28,7 @@
#include <linux/regulator/consumer.h>
#include <linux/regmap.h>
#include <linux/stmmac.h>
+#include <linux/setbits.h>
#include "stmmac.h"
#include "stmmac_platform.h"
@@ -342,50 +343,30 @@ static void sun8i_dwmac_disable_dma_irq(void __iomem *ioaddr, u32 chan)
static void sun8i_dwmac_dma_start_tx(void __iomem *ioaddr, u32 chan)
{
- u32 v;
-
- v = readl(ioaddr + EMAC_TX_CTL1);
- v |= EMAC_TX_DMA_START;
- v |= EMAC_TX_DMA_EN;
- writel(v, ioaddr + EMAC_TX_CTL1);
+ setbits_le32(ioaddr + EMAC_TX_CTL1,
+ EMAC_TX_DMA_START | EMAC_TX_DMA_EN);
}
static void sun8i_dwmac_enable_dma_transmission(void __iomem *ioaddr)
{
- u32 v;
-
- v = readl(ioaddr + EMAC_TX_CTL1);
- v |= EMAC_TX_DMA_START;
- v |= EMAC_TX_DMA_EN;
- writel(v, ioaddr + EMAC_TX_CTL1);
+ setbits_le32(ioaddr + EMAC_TX_CTL1,
+ EMAC_TX_DMA_START | EMAC_TX_DMA_EN);
}
static void sun8i_dwmac_dma_stop_tx(void __iomem *ioaddr, u32 chan)
{
- u32 v;
-
- v = readl(ioaddr + EMAC_TX_CTL1);
- v &= ~EMAC_TX_DMA_EN;
- writel(v, ioaddr + EMAC_TX_CTL1);
+ clrbits_le32(ioaddr + EMAC_TX_CTL1, EMAC_TX_DMA_EN);
}
static void sun8i_dwmac_dma_start_rx(void __iomem *ioaddr, u32 chan)
{
- u32 v;
-
- v = readl(ioaddr + EMAC_RX_CTL1);
- v |= EMAC_RX_DMA_START;
- v |= EMAC_RX_DMA_EN;
- writel(v, ioaddr + EMAC_RX_CTL1);
+ setbits_le32(ioaddr + EMAC_RX_CTL1,
+ EMAC_RX_DMA_START | EMAC_RX_DMA_EN);
}
static void sun8i_dwmac_dma_stop_rx(void __iomem *ioaddr, u32 chan)
{
- u32 v;
-
- v = readl(ioaddr + EMAC_RX_CTL1);
- v &= ~EMAC_RX_DMA_EN;
- writel(v, ioaddr + EMAC_RX_CTL1);
+ clrbits_le32(ioaddr + EMAC_RX_CTL1, EMAC_RX_DMA_EN);
}
static int sun8i_dwmac_dma_interrupt(void __iomem *ioaddr,
@@ -578,7 +559,6 @@ static void sun8i_dwmac_set_umac_addr(struct mac_device_info *hw,
unsigned int reg_n)
{
void __iomem *ioaddr = hw->pcsr;
- u32 v;
if (!addr) {
writel(0, ioaddr + EMAC_MACADDR_HI(reg_n));
@@ -588,9 +568,8 @@ static void sun8i_dwmac_set_umac_addr(struct mac_device_info *hw,
stmmac_set_mac_addr(ioaddr, addr, EMAC_MACADDR_HI(reg_n),
EMAC_MACADDR_LO(reg_n));
if (reg_n > 0) {
- v = readl(ioaddr + EMAC_MACADDR_HI(reg_n));
- v |= MAC_ADDR_TYPE_DST;
- writel(v, ioaddr + EMAC_MACADDR_HI(reg_n));
+ setbits_le32(ioaddr + EMAC_MACADDR_HI(reg_n),
+ MAC_ADDR_TYPE_DST);
}
}
@@ -608,11 +587,8 @@ static void sun8i_dwmac_get_umac_addr(struct mac_device_info *hw,
static int sun8i_dwmac_rx_ipc_enable(struct mac_device_info *hw)
{
void __iomem *ioaddr = hw->pcsr;
- u32 v;
- v = readl(ioaddr + EMAC_RX_CTL0);
- v |= EMAC_RX_DO_CRC;
- writel(v, ioaddr + EMAC_RX_CTL0);
+ setbits_le32(ioaddr + EMAC_RX_CTL0, EMAC_RX_DO_CRC);
return 1;
}
@@ -662,21 +638,15 @@ static void sun8i_dwmac_flow_ctrl(struct mac_device_info *hw,
unsigned int pause_time, u32 tx_cnt)
{
void __iomem *ioaddr = hw->pcsr;
- u32 v;
- v = readl(ioaddr + EMAC_RX_CTL0);
if (fc == FLOW_AUTO)
- v |= EMAC_RX_FLOW_CTL_EN;
+ setbits_le32(ioaddr + EMAC_RX_CTL0, EMAC_RX_FLOW_CTL_EN);
else
- v &= ~EMAC_RX_FLOW_CTL_EN;
- writel(v, ioaddr + EMAC_RX_CTL0);
-
- v = readl(ioaddr + EMAC_TX_FLOW_CTL);
+ clrbits_le32(ioaddr + EMAC_RX_CTL0, EMAC_RX_FLOW_CTL_EN);
if (fc == FLOW_AUTO)
- v |= EMAC_TX_FLOW_CTL_EN;
+ setbits_le32(ioaddr + EMAC_TX_FLOW_CTL, EMAC_TX_FLOW_CTL_EN);
else
- v &= ~EMAC_TX_FLOW_CTL_EN;
- writel(v, ioaddr + EMAC_TX_FLOW_CTL);
+ clrbits_le32(ioaddr + EMAC_TX_FLOW_CTL, EMAC_TX_FLOW_CTL_EN);
}
static int sun8i_dwmac_reset(struct stmmac_priv *priv)
--
2.16.4
^ permalink raw reply related
* [PATCH v2 7/7] net: stmmac: dwmac-meson8b: use xxxsetbits32
From: Corentin Labbe @ 2018-09-24 19:04 UTC (permalink / raw)
To: Gilles.Muller, Julia.Lawall, agust, airlied, alexandre.torgue,
alistair, benh, carlo, davem, galak, joabreu, khilman,
maxime.ripard, michal.lkml, mpe, mporter, narmstrong,
nicolas.palix, oss, paulus, peppe.cavallaro, tj, vitb, wens
Cc: cocci, dri-devel, linux-amlogic, linux-arm-kernel, linux-ide,
linux-kernel, linuxppc-dev, netdev, Corentin Labbe
In-Reply-To: <1537815856-31728-1-git-send-email-clabbe@baylibre.com>
This patch convert meson stmmac glue driver to use all xxxsetbits32 functions.
Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
---
.../net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 56 +++++++++-------------
1 file changed, 22 insertions(+), 34 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
index c5979569fd60..abcf65588576 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
@@ -23,6 +23,7 @@
#include <linux/mfd/syscon.h>
#include <linux/platform_device.h>
#include <linux/stmmac.h>
+#include <linux/setbits.h>
#include "stmmac_platform.h"
@@ -75,18 +76,6 @@ struct meson8b_dwmac_clk_configs {
struct clk_gate rgmii_tx_en;
};
-static void meson8b_dwmac_mask_bits(struct meson8b_dwmac *dwmac, u32 reg,
- u32 mask, u32 value)
-{
- u32 data;
-
- data = readl(dwmac->regs + reg);
- data &= ~mask;
- data |= (value & mask);
-
- writel(data, dwmac->regs + reg);
-}
-
static struct clk *meson8b_dwmac_register_clk(struct meson8b_dwmac *dwmac,
const char *name_suffix,
const char **parent_names,
@@ -192,14 +181,13 @@ static int meson8b_set_phy_mode(struct meson8b_dwmac *dwmac)
case PHY_INTERFACE_MODE_RGMII_ID:
case PHY_INTERFACE_MODE_RGMII_TXID:
/* enable RGMII mode */
- meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
- PRG_ETH0_RGMII_MODE,
- PRG_ETH0_RGMII_MODE);
+ clrsetbits_le32(dwmac->regs + PRG_ETH0, PRG_ETH0_RGMII_MODE,
+ PRG_ETH0_RGMII_MODE);
break;
case PHY_INTERFACE_MODE_RMII:
/* disable RGMII mode -> enables RMII mode */
- meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
- PRG_ETH0_RGMII_MODE, 0);
+ clrsetbits_le32(dwmac->regs + PRG_ETH0, PRG_ETH0_RGMII_MODE,
+ 0);
break;
default:
dev_err(dwmac->dev, "fail to set phy-mode %s\n",
@@ -218,15 +206,15 @@ static int meson_axg_set_phy_mode(struct meson8b_dwmac *dwmac)
case PHY_INTERFACE_MODE_RGMII_ID:
case PHY_INTERFACE_MODE_RGMII_TXID:
/* enable RGMII mode */
- meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
- PRG_ETH0_EXT_PHY_MODE_MASK,
- PRG_ETH0_EXT_RGMII_MODE);
+ clrsetbits_le32(dwmac->regs + PRG_ETH0,
+ PRG_ETH0_EXT_PHY_MODE_MASK,
+ PRG_ETH0_EXT_RGMII_MODE);
break;
case PHY_INTERFACE_MODE_RMII:
/* disable RGMII mode -> enables RMII mode */
- meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
- PRG_ETH0_EXT_PHY_MODE_MASK,
- PRG_ETH0_EXT_RMII_MODE);
+ clrsetbits_le32(dwmac->regs + PRG_ETH0,
+ PRG_ETH0_EXT_PHY_MODE_MASK,
+ PRG_ETH0_EXT_RMII_MODE);
break;
default:
dev_err(dwmac->dev, "fail to set phy-mode %s\n",
@@ -255,11 +243,11 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
case PHY_INTERFACE_MODE_RGMII_ID:
case PHY_INTERFACE_MODE_RGMII_TXID:
/* only relevant for RMII mode -> disable in RGMII mode */
- meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
- PRG_ETH0_INVERTED_RMII_CLK, 0);
+ clrsetbits_le32(dwmac->regs + PRG_ETH0,
+ PRG_ETH0_INVERTED_RMII_CLK, 0);
- meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
- tx_dly_val << PRG_ETH0_TXDLY_SHIFT);
+ clrsetbits_le32(dwmac->regs + PRG_ETH0, PRG_ETH0_TXDLY_MASK,
+ tx_dly_val << PRG_ETH0_TXDLY_SHIFT);
/* Configure the 125MHz RGMII TX clock, the IP block changes
* the output automatically (= without us having to configure
@@ -287,13 +275,13 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
case PHY_INTERFACE_MODE_RMII:
/* invert internal clk_rmii_i to generate 25/2.5 tx_rx_clk */
- meson8b_dwmac_mask_bits(dwmac, PRG_ETH0,
- PRG_ETH0_INVERTED_RMII_CLK,
- PRG_ETH0_INVERTED_RMII_CLK);
+ clrsetbits_le32(dwmac->regs + PRG_ETH0,
+ PRG_ETH0_INVERTED_RMII_CLK,
+ PRG_ETH0_INVERTED_RMII_CLK);
/* TX clock delay cannot be configured in RMII mode */
- meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TXDLY_MASK,
- 0);
+ clrsetbits_le32(dwmac->regs + PRG_ETH0, PRG_ETH0_TXDLY_MASK,
+ 0);
break;
@@ -304,8 +292,8 @@ static int meson8b_init_prg_eth(struct meson8b_dwmac *dwmac)
}
/* enable TX_CLK and PHY_REF_CLK generator */
- meson8b_dwmac_mask_bits(dwmac, PRG_ETH0, PRG_ETH0_TX_AND_PHY_REF_CLK,
- PRG_ETH0_TX_AND_PHY_REF_CLK);
+ clrsetbits_le32(dwmac->regs + PRG_ETH0, PRG_ETH0_TX_AND_PHY_REF_CLK,
+ PRG_ETH0_TX_AND_PHY_REF_CLK);
return 0;
}
--
2.16.4
^ permalink raw reply related
* Re: [PATCH v2 7/7] net: stmmac: dwmac-meson8b: use xxxsetbits32
From: Neil Armstrong @ 2018-09-24 19:08 UTC (permalink / raw)
To: Corentin Labbe, Gilles.Muller, Julia.Lawall, agust, airlied,
alexandre.torgue, alistair, benh, carlo, davem, galak, joabreu,
khilman, maxime.ripard, michal.lkml, mpe, mporter, nicolas.palix,
oss, paulus, peppe.cavallaro, tj, vitb, wens
Cc: cocci, dri-devel, linux-amlogic, linux-arm-kernel, linux-ide,
linux-kernel, linuxppc-dev, netdev
In-Reply-To: <1537815856-31728-8-git-send-email-clabbe@baylibre.com>
On 24/09/2018 21:04, Corentin Labbe wrote:
> This patch convert meson stmmac glue driver to use all xxxsetbits32 functions.
>
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
> .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 56 +++++++++-------------
> 1 file changed, 22 insertions(+), 34 deletions(-)
>
[...]
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
^ permalink raw reply
* Re: [PATCH v2 7/7] net: stmmac: dwmac-meson8b: use xxxsetbits32
From: Florian Fainelli @ 2018-09-24 19:17 UTC (permalink / raw)
To: Corentin Labbe, Gilles.Muller, Julia.Lawall, agust, airlied,
alexandre.torgue, alistair, benh, carlo, davem, galak, joabreu,
khilman, maxime.ripard, michal.lkml, mpe, mporter, narmstrong,
nicolas.palix, oss, paulus, peppe.cavallaro, tj, vitb, wens
Cc: netdev, linux-kernel, dri-devel, linux-ide, linux-amlogic,
linuxppc-dev, cocci, linux-arm-kernel
In-Reply-To: <1537815856-31728-8-git-send-email-clabbe@baylibre.com>
On 09/24/2018 12:04 PM, Corentin Labbe wrote:
> This patch convert meson stmmac glue driver to use all xxxsetbits32 functions.
>
> Signed-off-by: Corentin Labbe <clabbe@baylibre.com>
> ---
> .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c | 56 +++++++++-------------
> 1 file changed, 22 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
> index c5979569fd60..abcf65588576 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
> @@ -23,6 +23,7 @@
> #include <linux/mfd/syscon.h>
> #include <linux/platform_device.h>
> #include <linux/stmmac.h>
> +#include <linux/setbits.h>
>
> #include "stmmac_platform.h"
>
> @@ -75,18 +76,6 @@ struct meson8b_dwmac_clk_configs {
> struct clk_gate rgmii_tx_en;
> };
>
> -static void meson8b_dwmac_mask_bits(struct meson8b_dwmac *dwmac, u32 reg,
> - u32 mask, u32 value)
> -{
> - u32 data;
> -
> - data = readl(dwmac->regs + reg);
> - data &= ~mask;
> - data |= (value & mask);
> -
> - writel(data, dwmac->regs + reg);
> -}
Why not make mseon8b_dwmac_mask_bits() a wrapper around
clrsetbits_le32() whose purpose is only to dereference dwmac->regs and
pass it to clrsetbits_le32()? That would be far less changes to review
and audit for correctness, same goes with every other patch in this
series touching the meson drivers.
--
Florian
^ permalink raw reply
* Re: [PATCH] net: qca_spi: Introduce write register verification
From: David Miller @ 2018-09-24 19:26 UTC (permalink / raw)
To: stefan.wahren; +Cc: netdev, linux-kernel, michael.heimpold
In-Reply-To: <1537788010-6766-1-git-send-email-stefan.wahren@i2se.com>
From: Stefan Wahren <stefan.wahren@i2se.com>
Date: Mon, 24 Sep 2018 13:20:10 +0200
> The SPI protocol for the QCA7000 doesn't have any fault detection.
> In order to increase the drivers reliability in noisy environments,
> we could implement a write verification inspired by the enc28j60.
> This should avoid situations were the driver wrongly assumes the
> receive interrupt is enabled and miss all incoming packets.
>
> This function is disabled per default and can be controlled via module
> parameter wr_verify.
>
> Signed-off-by: Michael Heimpold <michael.heimpold@i2se.com>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
Applied.
^ permalink raw reply
* [PATCH net] ip_tunnel: be careful when accessing the inner header
From: Paolo Abeni @ 2018-09-24 13:48 UTC (permalink / raw)
To: netdev; +Cc: David S. Miller, Cong Wang, Pravin B Shelar
Cong noted that we need the same checks introduced by commit 76c0ddd8c3a6
("ip6_tunnel: be careful when accessing the inner header")
even for ipv4 tunnels.
Fixes: c54419321455 ("GRE: Refactor GRE tunneling code.")
Suggested-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
Note: the bug is probably present even before the commit mentioned in
the fixes tag, but porting this change on older kernel is likely worthless
---
net/ipv4/ip_tunnel.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/net/ipv4/ip_tunnel.c b/net/ipv4/ip_tunnel.c
index c4f5602308ed..284a22154b4e 100644
--- a/net/ipv4/ip_tunnel.c
+++ b/net/ipv4/ip_tunnel.c
@@ -627,6 +627,7 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
const struct iphdr *tnl_params, u8 protocol)
{
struct ip_tunnel *tunnel = netdev_priv(dev);
+ unsigned int inner_nhdr_len = 0;
const struct iphdr *inner_iph;
struct flowi4 fl4;
u8 tos, ttl;
@@ -636,6 +637,14 @@ void ip_tunnel_xmit(struct sk_buff *skb, struct net_device *dev,
__be32 dst;
bool connected;
+ /* ensure we can access the inner net header, for several users below */
+ if (skb->protocol == htons(ETH_P_IP))
+ inner_nhdr_len = sizeof(struct iphdr);
+ else if (skb->protocol == htons(ETH_P_IPV6))
+ inner_nhdr_len = sizeof(struct ipv6hdr);
+ if (unlikely(!pskb_may_pull(skb, inner_nhdr_len)))
+ goto tx_error;
+
inner_iph = (const struct iphdr *)skb_inner_network_header(skb);
connected = (tunnel->parms.iph.daddr != 0);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v2 05/17] compat_ioctl: move more drivers to generic_compat_ioctl_ptrarg
From: Arnd Bergmann @ 2018-09-24 20:18 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA, linux-pci,
linux-remoteproc-u79uwXL29TY76Z2rM5mHXA,
linux-nvme-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Platform Driver,
sparclinux, driverdevel, linux-scsi,
linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, linux-rdma,
qat-linux-ral2JQCrhuEAvxtiuMwx3w,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
open list:HID CORE LAYER, Darren Hart, Linux Media Mailing List,
linaro-mm-sig-cunTk1MwBs8s++Sfvej+rw, dri-devel, ceph-devel,
gregkh, USB list, linux-wireless, Linux Kernel Mailing List <l
In-Reply-To: <20180918175952.GJ11367-uk2M96/98Pc@public.gmane.org>
On Tue, Sep 18, 2018 at 7:59 PM Jason Gunthorpe <jgg-uk2M96/98Pc@public.gmane.org> wrote:
>
> On Tue, Sep 18, 2018 at 10:51:08AM -0700, Darren Hart wrote:
> > On Fri, Sep 14, 2018 at 09:57:48PM +0100, Al Viro wrote:
> > > On Fri, Sep 14, 2018 at 01:35:06PM -0700, Darren Hart wrote:
> > >
> > > > Acked-by: Darren Hart (VMware) <dvhart-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org>
> > > >
> > > > As for a longer term solution, would it be possible to init fops in such
> > > > a way that the compat_ioctl call defaults to generic_compat_ioctl_ptrarg
> > > > so we don't have to duplicate this boilerplate for every ioctl fops
> > > > structure?
> > >
> > > Bad idea, that... Because several years down the road somebody will add
> > > an ioctl that takes an unsigned int for argument. Without so much as looking
> > > at your magical mystery macro being used to initialize file_operations.
> >
> > Fair, being explicit in the declaration as it is currently may be
> > preferable then.
>
> It would be much cleaner and safer if you could arrange things to add
> something like this to struct file_operations:
>
> long (*ptr_ioctl) (struct file *, unsigned int, void __user *);
>
> Where the core code automatically converts the unsigned long to the
> void __user * as appropriate.
>
> Then it just works right always and the compiler will help address
> Al's concern down the road.
I think if we wanted to do this with a new file operation, the best
way would be to do the copy_from_user()/copy_to_user() in the caller
as well.
We already do this inside of some subsystems, notably drivers/media/,
and it simplifies the implementation of the ioctl handler function
significantly. We obviously cannot do this in general, both because of
traditional drivers that have 16-bit command codes (drivers/tty and others)
and also because of drivers that by accident defined the commands
incorrectly and use the wrong type or the wrong direction in the
definition.
Arnd
^ permalink raw reply
* Re: [PATCH 0/2] gpiolib: Fix issues introduced by fast bitmap processing path
From: Janusz Krzysztofik @ 2018-09-24 14:18 UTC (permalink / raw)
To: Marek Szyprowski
Cc: Linus Walleij, Jonathan Corbet, Miguel Ojeda Sandonis,
Peter Korsgaard, Peter Rosin, Ulf Hansson, Andrew Lunn,
Florian Fainelli, David S. Miller, Dominik Brodowski,
Greg Kroah-Hartman, Kishon Vijay Abraham I, Lars-Peter Clausen,
Michael Hennerich, Jonathan Cameron, Hartmut Knaack,
Peter Meerwald-Stadler, Jiri Slaby, Willy Tarreau
In-Reply-To: <20180924113859eucas1p160322f5844c771bc9c680d35de4877a3~XUxM0smXV0334103341eucas1p1s@eucas1p1.samsung.com>
Hi Marek,
2018-09-24 13:38 GMT+02:00, Marek Szyprowski <m.szyprowski@samsung.com>:
> Hi Janusz,
>
> On 2018-09-24 13:08, Janusz Krzysztofik wrote:
>> 2018-09-24 11:43 GMT+02:00, Marek Szyprowski <m.szyprowski@samsung.com>:
>>> On 2018-09-24 01:53, Janusz Krzysztofik wrote:
>>>> While investigating possible reasons of GPIO fast bitmap processing
>>>> related boot hang on Samsung Snow Chromebook, reported by Marek
>>>> Szyprowski (thanks!), I've discovered one coding bug, addressed by
>>>> PATCH 1/2 of this series, and one potential regression introduced at
>>>> design level of the solution, hopefully fixed by PATCH 2/2. See
>>>> commit messages for details.
>>>>
>>>> Janusz Krzysztofik (2):
>>>> gpiolib: Fix missing updates of bitmap index
>>>> gpiolib: Fix array members of same chip processed separately
>>>>
>>>> The fixes should resolve the boot hang observed by Marek, however the
>>>> second change excludes that particular case from fast bitmap processing
>>>> and restores the old behaviour.
>>> I confirm, that the above 2 patches fixes boot issue on Samsung Snow
>>> Chromebook with next-20180920.
>>>
>>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>>
>>>> Hence, it is possible still another
>>>> issue which have had an influence on that boot hang exists in the code.
>>>> In order to fully verify the fix, it would have to be tested on a
>>>> platform where an array of GPIO descriptors is used which starts from
>>>> at least two consecutive pins of one GPIO chip in hardware order,
>>>> starting ftom 0, followed by one or more pins belonging to other
>>>> chip(s).
>>>>
>>>> In order to verify if separate calls to .set() chip callback for each
>>>> pin instead of one call to .set_multiple() is actually the reason of
>>>> boot hang on Samsung Snow Chromebook, the affected driver -
>>>> drivers/mmc/core/pwrseq_simple.c - would have to be temporarily
>>>> modified for testing purposes so it calls gpiod_set_value() for each
>>>> pin instead of gpiod_set_array_value() for all of them. If that would
>>>> also result in boot hang, we could be sure the issue was really the
>>>> one addressed by the second fix. Marek, could you please try to
>>>> perform such test?
>>> Yes, I've just tested next-20180920 only with the first patch from this
>>> patchset and the mentioned change to drivers/mmc/core/pwrseq_simple.c.
>>> It boots fine, so indeed the issue is in handling of arrays of gpios.
>>>
>>> Just to be sure I did it right, this is my change to the mentioned file:
>> Yeah, that's what I had on mind. However, I'd be more lucky if it didn't
>> work
>> for you. Setting the pins sequentially, not simultaneously as before,
>> was
>> exactly what I hoped was the reason of the hang.
>>
>>> diff --git a/drivers/mmc/core/pwrseq_simple.c
>>> b/drivers/mmc/core/pwrseq_simple.c
>>> index 7f882a2bb872..9397dc1f2e38 100644
>>> --- a/drivers/mmc/core/pwrseq_simple.c
>>> +++ b/drivers/mmc/core/pwrseq_simple.c
>>> @@ -38,16 +38,11 @@ static void mmc_pwrseq_simple_set_gpios_value(struct
>>> mmc_pwrseq_simple *pwrseq,
>>> int value)
>>> {
>>> struct gpio_descs *reset_gpios = pwrseq->reset_gpios;
>>> + int i;
>>>
>>> - if (!IS_ERR(reset_gpios)) {
>>> - DECLARE_BITMAP(values, BITS_PER_TYPE(value));
>>> - int nvalues = reset_gpios->ndescs;
>>> -
>>> - values[0] = value;
>>> -
>>> - gpiod_set_array_value_cansleep(nvalues,
>>> reset_gpios->desc,
>>> - reset_gpios->info,
>>> values);
>>> - }
>>> + if (!IS_ERR(reset_gpios))
>>> + for (i = 0; i < reset_gpios->ndescs; i++)
>> The only difference from the behaviour when the hang was occurring is now
>> the order the pins are manipulated. Maybe that matters?
>> Could you please retry the same with the order of pins reversed, either
>> in
>> the .dts file or here inside this for loop?
>
> I've switched the order of pins in dts and next-20180920 + first patch +
> above
> change also boots fine.
Thanks for performing those tests.
Since we are not able to reproduce the issue by any means other than
using the original code introduced by fast bitmap processing changes,
regardless of the first fix being applied or not, and we are only able to
resolve the hangup by excluding affected use case from the fast path,
we have to assume one or more bugs which affect mixed arrays, i.e.,
those which apply for fast bitmap processing only in part, may still exist
in the code introduced by the fast bitmap processing series. I hope we
are able to resolve it soon, before the changes reach mainline.
Thanks,
Janusz
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>
>
^ permalink raw reply
* Re: vlan missing with AF_PACKET and auxdata
From: Michael Walle @ 2018-09-24 14:25 UTC (permalink / raw)
To: Jan Grashöfer
Cc: netdev, David S. Miller, Eric Dumazet, Steinar H. Gunderson,
heiko.thiery
In-Reply-To: <29cdf22a-eceb-eab9-979d-049e261a6064@gmail.com>
Am 2018-09-24 14:22, schrieb Jan Grashöfer:
> Hi Michael,
>
> On 24/09/2018 14:01, Michael Walle wrote:
>> I'm using the AF_PACKET socket with setsockopt(PACKET_AUXDATA) to get
>> the incoming VLAN tag. Correct me if I'm wrong, but as far as I see
>> the first VLAN tag is always stripped - either in hardware or in
>> net/core/dev.c in __netif_receive_skb_core() - and stored in
>> skb->vlan_tci. Therefore, it won't be in the packet data anymore.
>
> although the documentation says "SOCK_RAW packets are passed to and
> from the device driver without any changes in the packet data." [1]
> that's correct and was discussed here:
> https://www.spinics.net/lists/netdev/msg440313.html
>
> Result of the discussion: Won't fix, too complicated.
Hi Jan,
thanks for the pointer. I'm fine with this decision, because the VLAN
tag should be available through metadata. And this was my original
question, because the metadata is only available in case of proto ==
ETH_P_ALL, but not for proto != ETH_P_ALL because of the commit in
question.
-michael
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox