* [bpf PATCH 2/3] bpf: sockmap, fix transition through disconnect without close
From: John Fastabend @ 2018-09-17 17:31 UTC (permalink / raw)
To: edumazet, ast, daniel; +Cc: netdev
In-Reply-To: <20180917172946.21218.66049.stgit@john-Precision-Tower-5810>
It is possible (via shutdown()) for TCP socks to go trough TCP_CLOSE
state via tcp_disconnect() without actually calling tcp_close which
would then call our bpf_tcp_close() callback. Because of this a user
could disconnect a socket then put it in a LISTEN state which would
break our assumptions about sockets always being ESTABLISHED state.
To resolve this rely on the unhash hook, which is called in the
disconnect case, to remove the sock from the sockmap.
Reported-by: Eric Dumazet <edumazet@google.com>
Fixes: 1aa12bdf1bfb ("bpf: sockmap, add sock close() hook to remove socks")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/sockmap.c | 71 +++++++++++++++++++++++++++++++++++++-------------
1 file changed, 52 insertions(+), 19 deletions(-)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 998b7bd..f6ab7f3 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -132,6 +132,7 @@ struct smap_psock {
struct work_struct gc_work;
struct proto *sk_proto;
+ void (*save_unhash)(struct sock *sk);
void (*save_close)(struct sock *sk, long timeout);
void (*save_data_ready)(struct sock *sk);
void (*save_write_space)(struct sock *sk);
@@ -143,6 +144,7 @@ static int bpf_tcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
static int bpf_tcp_sendmsg(struct sock *sk, struct msghdr *msg, size_t size);
static int bpf_tcp_sendpage(struct sock *sk, struct page *page,
int offset, size_t size, int flags);
+static void bpf_tcp_unhash(struct sock *sk);
static void bpf_tcp_close(struct sock *sk, long timeout);
static inline struct smap_psock *smap_psock_sk(const struct sock *sk)
@@ -184,6 +186,7 @@ static void build_protos(struct proto prot[SOCKMAP_NUM_CONFIGS],
struct proto *base)
{
prot[SOCKMAP_BASE] = *base;
+ prot[SOCKMAP_BASE].unhash = bpf_tcp_unhash;
prot[SOCKMAP_BASE].close = bpf_tcp_close;
prot[SOCKMAP_BASE].recvmsg = bpf_tcp_recvmsg;
prot[SOCKMAP_BASE].stream_memory_read = bpf_tcp_stream_read;
@@ -217,6 +220,7 @@ static int bpf_tcp_init(struct sock *sk)
return -EBUSY;
}
+ psock->save_unhash = sk->sk_prot->unhash;
psock->save_close = sk->sk_prot->close;
psock->sk_proto = sk->sk_prot;
@@ -305,30 +309,12 @@ static struct smap_psock_map_entry *psock_map_pop(struct sock *sk,
return e;
}
-static void bpf_tcp_close(struct sock *sk, long timeout)
+static void bpf_tcp_remove(struct sock *sk, struct smap_psock *psock)
{
- void (*close_fun)(struct sock *sk, long timeout);
struct smap_psock_map_entry *e;
struct sk_msg_buff *md, *mtmp;
- struct smap_psock *psock;
struct sock *osk;
- lock_sock(sk);
- rcu_read_lock();
- psock = smap_psock_sk(sk);
- if (unlikely(!psock)) {
- rcu_read_unlock();
- release_sock(sk);
- return sk->sk_prot->close(sk, timeout);
- }
-
- /* The psock may be destroyed anytime after exiting the RCU critial
- * section so by the time we use close_fun the psock may no longer
- * be valid. However, bpf_tcp_close is called with the sock lock
- * held so the close hook and sk are still valid.
- */
- close_fun = psock->save_close;
-
if (psock->cork) {
free_start_sg(psock->sock, psock->cork, true);
kfree(psock->cork);
@@ -379,6 +365,53 @@ static void bpf_tcp_close(struct sock *sk, long timeout)
kfree(e);
e = psock_map_pop(sk, psock);
}
+}
+
+static void bpf_tcp_unhash(struct sock *sk)
+{
+ void (*unhash_fun)(struct sock *sk);
+ struct smap_psock *psock;
+
+ rcu_read_lock();
+ psock = smap_psock_sk(sk);
+ if (unlikely(!psock)) {
+ rcu_read_unlock();
+ release_sock(sk);
+ return sk->sk_prot->unhash(sk);
+ }
+
+ /* The psock may be destroyed anytime after exiting the RCU critial
+ * section so by the time we use close_fun the psock may no longer
+ * be valid. However, bpf_tcp_close is called with the sock lock
+ * held so the close hook and sk are still valid.
+ */
+ unhash_fun = psock->save_unhash;
+ bpf_tcp_remove(sk, psock);
+ rcu_read_unlock();
+ unhash_fun(sk);
+}
+
+static void bpf_tcp_close(struct sock *sk, long timeout)
+{
+ void (*close_fun)(struct sock *sk, long timeout);
+ struct smap_psock *psock;
+
+ lock_sock(sk);
+ rcu_read_lock();
+ psock = smap_psock_sk(sk);
+ if (unlikely(!psock)) {
+ rcu_read_unlock();
+ release_sock(sk);
+ return sk->sk_prot->close(sk, timeout);
+ }
+
+ /* The psock may be destroyed anytime after exiting the RCU critial
+ * section so by the time we use close_fun the psock may no longer
+ * be valid. However, bpf_tcp_close is called with the sock lock
+ * held so the close hook and sk are still valid.
+ */
+ close_fun = psock->save_close;
+ bpf_tcp_remove(sk, psock);
rcu_read_unlock();
release_sock(sk);
close_fun(sk, timeout);
^ permalink raw reply related
* [bpf PATCH 1/3] bpf: sockmap only allow ESTABLISHED sock state
From: John Fastabend @ 2018-09-17 17:31 UTC (permalink / raw)
To: edumazet, ast, daniel; +Cc: netdev
In-Reply-To: <20180917172946.21218.66049.stgit@john-Precision-Tower-5810>
After this patch we only allow socks that are in ESTABLISHED state or
are being added via a sock_ops event that is transitioning into an
ESTABLISHED state. By allowing sock_ops events we allow users to
manage sockmaps directly from sock ops programs. The two supported
sock_ops ops are BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB and
BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB.
Similar to TLS ULP this ensures sk_user_data is correct.
Reported-by: Eric Dumazet <edumazet@google.com>
Fixes: 1aa12bdf1bfb ("bpf: sockmap, add sock close() hook to remove socks")
Signed-off-by: John Fastabend <john.fastabend@gmail.com>
---
kernel/bpf/sockmap.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/kernel/bpf/sockmap.c b/kernel/bpf/sockmap.c
index 488ef96..998b7bd 100644
--- a/kernel/bpf/sockmap.c
+++ b/kernel/bpf/sockmap.c
@@ -2097,8 +2097,12 @@ static int sock_map_update_elem(struct bpf_map *map,
return -EINVAL;
}
+ /* ULPs are currently supported only for TCP sockets in ESTABLISHED
+ * state.
+ */
if (skops.sk->sk_type != SOCK_STREAM ||
- skops.sk->sk_protocol != IPPROTO_TCP) {
+ skops.sk->sk_protocol != IPPROTO_TCP ||
+ skops.sk->sk_state != TCP_ESTABLISHED) {
fput(socket->file);
return -EOPNOTSUPP;
}
@@ -2543,10 +2547,22 @@ struct sock *__sock_hash_lookup_elem(struct bpf_map *map, void *key)
.map_check_btf = map_check_no_btf,
};
+static bool bpf_is_valid_sock_op(struct bpf_sock_ops_kern *ops)
+{
+ return ops->op == BPF_SOCK_OPS_PASSIVE_ESTABLISHED_CB ||
+ ops->op == BPF_SOCK_OPS_ACTIVE_ESTABLISHED_CB;
+}
BPF_CALL_4(bpf_sock_map_update, struct bpf_sock_ops_kern *, bpf_sock,
struct bpf_map *, map, void *, key, u64, flags)
{
WARN_ON_ONCE(!rcu_read_lock_held());
+
+ /* ULPs are currently supported only for TCP sockets in ESTABLISHED
+ * state. This checks that the sock ops triggering the update is
+ * one indicating we are (or will be soon) in an ESTABLISHED state.
+ */
+ if (!bpf_is_valid_sock_op(bpf_sock))
+ return -EOPNOTSUPP;
return sock_map_ctx_update_elem(bpf_sock, map, key, flags);
}
@@ -2565,6 +2581,9 @@ struct sock *__sock_hash_lookup_elem(struct bpf_map *map, void *key)
struct bpf_map *, map, void *, key, u64, flags)
{
WARN_ON_ONCE(!rcu_read_lock_held());
+
+ if (!bpf_is_valid_sock_op(bpf_sock))
+ return -EOPNOTSUPP;
return sock_hash_ctx_update_elem(bpf_sock, map, key, flags);
}
^ permalink raw reply related
* [bpf PATCH 0/3] bpf, sockmap ESTABLISHED state only
From: John Fastabend @ 2018-09-17 17:31 UTC (permalink / raw)
To: edumazet, ast, daniel; +Cc: netdev
Eric noted that using the close callback is not sufficient
to catch all transitions from ESTABLISHED state to a LISTEN
state. So this series does two things. First, only allow
adding socks in ESTABLISH state and second use unhash callback
to catch tcp_disconnect() transitions.
Thanks,
John
---
John Fastabend (3):
bpf: sockmap only allow ESTABLISHED sock state
bpf: sockmap, fix transition through disconnect without close
bpf: test_maps, only support ESTABLISHED socks
kernel/bpf/sockmap.c | 92 ++++++++++++++++++++++++-------
tools/testing/selftests/bpf/test_maps.c | 10 ++-
2 files changed, 79 insertions(+), 23 deletions(-)
^ permalink raw reply
* Re: [Patch net v2] net/ipv6: do not copy dst flags on rt init
From: David Ahern @ 2018-09-17 17:27 UTC (permalink / raw)
To: Peter Oskolkov, David Miller, netdev
In-Reply-To: <20180917172053.126170-1-posk@google.com>
On 9/17/18 10:20 AM, Peter Oskolkov wrote:
> DST_NOCOUNT in dst_entry::flags tracks whether the entry counts
> toward route cache size (net->ipv6.sysctl.ip6_rt_max_size).
>
> If the flag is NOT set, dst_ops::pcpuc_entries counter is incremented
> in dist_init() and decremented in dst_destroy().
>
> This flag is tied to allocation/deallocation of dst_entry and
> should not be copied from another dst/route. Otherwise it can happen
> that dst_ops::pcpuc_entries counter grows until no new routes can
> be allocated because the counter reached ip6_rt_max_size due to
> DST_NOCOUNT not set and thus no counter decrements on gc-ed routes.
>
> Fixes: 3b6761d18bc1 ("net/ipv6: Move dst flags to booleans in fib entries")
> Cc: David Ahern <dsahern@gmail.com>
> Acked-by: Wei Wang <weiwan@google.com>
> Signed-off-by: Peter Oskolkov <posk@google.com>
> ---
> net/ipv6/route.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 3eed045c65a5..480a79f47c52 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -946,8 +946,6 @@ static void ip6_rt_init_dst_reject(struct rt6_info *rt, struct fib6_info *ort)
>
> static void ip6_rt_init_dst(struct rt6_info *rt, struct fib6_info *ort)
> {
> - rt->dst.flags |= fib6_info_dst_flags(ort);
> -
> if (ort->fib6_flags & RTF_REJECT) {
> ip6_rt_init_dst_reject(rt, ort);
> return;
>
Reviewed-by: David Ahern <dsahern@gmail.com>
Thanks for the patch.
^ permalink raw reply
* Re: [PATCH net] net/ipv6: do not copy DST_NOCOUNT flag on rt init
From: Peter Oskolkov @ 2018-09-17 17:23 UTC (permalink / raw)
To: dsahern; +Cc: davem, netdev
In-Reply-To: <ecda87c9-4fdb-8967-2948-2fd138d330ec@gmail.com>
On Mon, Sep 17, 2018 at 9:59 AM David Ahern <dsahern@gmail.com> wrote:
>
> On 9/17/18 9:11 AM, Peter Oskolkov wrote:
> > On Thu, Sep 13, 2018 at 9:11 PM David Ahern <dsahern@gmail.com> wrote:
> >>
> >> On 9/13/18 1:38 PM, Peter Oskolkov wrote:
> >>
> >>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> >>> index 3eed045c65a5..a3902f805305 100644
> >>> --- a/net/ipv6/route.c
> >>> +++ b/net/ipv6/route.c
> >>> @@ -946,7 +946,7 @@ static void ip6_rt_init_dst_reject(struct rt6_info *rt, struct fib6_info *ort)
> >>>
> >>> static void ip6_rt_init_dst(struct rt6_info *rt, struct fib6_info *ort)
> >>> {
> >>> - rt->dst.flags |= fib6_info_dst_flags(ort);
> >>> + rt->dst.flags |= fib6_info_dst_flags(ort) & ~DST_NOCOUNT;
> >>
> >> I think my mistake is setting dst.flags in ip6_rt_init_dst. Flags
> >> argument is passed to ip6_dst_alloc which is always invoked before
> >> ip6_rt_copy_init is called which is the only caller of ip6_rt_init_dst.
> >
> > ip6_rt_cache_alloc calls ip6_dst_alloc with zero as flags; and only
> > one flag is copied later (DST_HOST) outside of ip6_rt_init_dst().
> > If the flag assignment is completely removed from ip6_rt_init_dst(),
> > then DST_NOPOLICY flag will be lost.
> >
> > Which may be OK, but is more than what this patch tries to solve (do not
> > copy DST_NOCOUNT flag).
>
> In the 4.17 kernel (prior to the fib6_info change), ip6_rt_cache_alloc
> calls __ip6_dst_alloc with 0 for flags so this is correct. The mistake
> is ip6_rt_copy_init -> ip6_rt_init_dst -> fib6_info_dst_flags.
>
> I believe the right fix is to drop the 'rt->dst.flags |=
> fib6_info_dst_flags(ort);' from ip6_rt_init_dst.
OK, I sent a v2 with the assignment removed. Thanks for the review!
^ permalink raw reply
* [Patch net v2] net/ipv6: do not copy dst flags on rt init
From: Peter Oskolkov @ 2018-09-17 17:20 UTC (permalink / raw)
To: David Miller, netdev; +Cc: Peter Oskolkov, David Ahern
DST_NOCOUNT in dst_entry::flags tracks whether the entry counts
toward route cache size (net->ipv6.sysctl.ip6_rt_max_size).
If the flag is NOT set, dst_ops::pcpuc_entries counter is incremented
in dist_init() and decremented in dst_destroy().
This flag is tied to allocation/deallocation of dst_entry and
should not be copied from another dst/route. Otherwise it can happen
that dst_ops::pcpuc_entries counter grows until no new routes can
be allocated because the counter reached ip6_rt_max_size due to
DST_NOCOUNT not set and thus no counter decrements on gc-ed routes.
Fixes: 3b6761d18bc1 ("net/ipv6: Move dst flags to booleans in fib entries")
Cc: David Ahern <dsahern@gmail.com>
Acked-by: Wei Wang <weiwan@google.com>
Signed-off-by: Peter Oskolkov <posk@google.com>
---
net/ipv6/route.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 3eed045c65a5..480a79f47c52 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -946,8 +946,6 @@ static void ip6_rt_init_dst_reject(struct rt6_info *rt, struct fib6_info *ort)
static void ip6_rt_init_dst(struct rt6_info *rt, struct fib6_info *ort)
{
- rt->dst.flags |= fib6_info_dst_flags(ort);
-
if (ort->fib6_flags & RTF_REJECT) {
ip6_rt_init_dst_reject(rt, ort);
return;
--
2.19.0.397.gdd90340f6a-goog
^ permalink raw reply related
* [PATCH 4.14 121/126] ip: frags: fix crash in ip_do_fragment()
From: Greg Kroah-Hartman @ 2018-09-17 22:42 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable,
netdev@vger.kernel.org, stable@vger.kernel.org, edumazet@google.com, Taehee Yoo,
Eric Dumazet, David S. Miller, Taehee Yoo
In-Reply-To: <20180917211703.481236999@linuxfoundation.org>
4.14-stable review patch. If anyone has any objections, please let me know.
------------------
From: Taehee Yoo <ap420073@gmail.com>
commit 5d407b071dc369c26a38398326ee2be53651cfe4 upstream
A kernel crash occurrs when defragmented packet is fragmented
in ip_do_fragment().
In defragment routine, skb_orphan() is called and
skb->ip_defrag_offset is set. but skb->sk and
skb->ip_defrag_offset are same union member. so that
frag->sk is not NULL.
Hence crash occurrs in skb->sk check routine in ip_do_fragment() when
defragmented packet is fragmented.
test commands:
%iptables -t nat -I POSTROUTING -j MASQUERADE
%hping3 192.168.4.2 -s 1000 -p 2000 -d 60000
splat looks like:
[ 261.069429] kernel BUG at net/ipv4/ip_output.c:636!
[ 261.075753] invalid opcode: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN PTI
[ 261.083854] CPU: 1 PID: 1349 Comm: hping3 Not tainted 4.19.0-rc2+ #3
[ 261.100977] RIP: 0010:ip_do_fragment+0x1613/0x2600
[ 261.106945] Code: e8 e2 38 e3 fe 4c 8b 44 24 18 48 8b 74 24 08 e9 92 f6 ff ff 80 3c 02 00 0f 85 da 07 00 00 48 8b b5 d0 00 00 00 e9 25 f6 ff ff <0f> 0b 0f 0b 44 8b 54 24 58 4c 8b 4c 24 18 4c 8b 5c 24 60 4c 8b 6c
[ 261.127015] RSP: 0018:ffff8801031cf2c0 EFLAGS: 00010202
[ 261.134156] RAX: 1ffff1002297537b RBX: ffffed0020639e6e RCX: 0000000000000004
[ 261.142156] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff880114ba9bd8
[ 261.150157] RBP: ffff880114ba8a40 R08: ffffed0022975395 R09: ffffed0022975395
[ 261.158157] R10: 0000000000000001 R11: ffffed0022975394 R12: ffff880114ba9ca4
[ 261.166159] R13: 0000000000000010 R14: ffff880114ba9bc0 R15: dffffc0000000000
[ 261.174169] FS: 00007fbae2199700(0000) GS:ffff88011b400000(0000) knlGS:0000000000000000
[ 261.183012] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 261.189013] CR2: 00005579244fe000 CR3: 0000000119bf4000 CR4: 00000000001006e0
[ 261.198158] Call Trace:
[ 261.199018] ? dst_output+0x180/0x180
[ 261.205011] ? save_trace+0x300/0x300
[ 261.209018] ? ip_copy_metadata+0xb00/0xb00
[ 261.213034] ? sched_clock_local+0xd4/0x140
[ 261.218158] ? kill_l4proto+0x120/0x120 [nf_conntrack]
[ 261.223014] ? rt_cpu_seq_stop+0x10/0x10
[ 261.227014] ? find_held_lock+0x39/0x1c0
[ 261.233008] ip_finish_output+0x51d/0xb50
[ 261.237006] ? ip_fragment.constprop.56+0x220/0x220
[ 261.243011] ? nf_ct_l4proto_register_one+0x5b0/0x5b0 [nf_conntrack]
[ 261.250152] ? rcu_is_watching+0x77/0x120
[ 261.255010] ? nf_nat_ipv4_out+0x1e/0x2b0 [nf_nat_ipv4]
[ 261.261033] ? nf_hook_slow+0xb1/0x160
[ 261.265007] ip_output+0x1c7/0x710
[ 261.269005] ? ip_mc_output+0x13f0/0x13f0
[ 261.273002] ? __local_bh_enable_ip+0xe9/0x1b0
[ 261.278152] ? ip_fragment.constprop.56+0x220/0x220
[ 261.282996] ? nf_hook_slow+0xb1/0x160
[ 261.287007] raw_sendmsg+0x21f9/0x4420
[ 261.291008] ? dst_output+0x180/0x180
[ 261.297003] ? sched_clock_cpu+0x126/0x170
[ 261.301003] ? find_held_lock+0x39/0x1c0
[ 261.306155] ? stop_critical_timings+0x420/0x420
[ 261.311004] ? check_flags.part.36+0x450/0x450
[ 261.315005] ? _raw_spin_unlock_irq+0x29/0x40
[ 261.320995] ? _raw_spin_unlock_irq+0x29/0x40
[ 261.326142] ? cyc2ns_read_end+0x10/0x10
[ 261.330139] ? raw_bind+0x280/0x280
[ 261.334138] ? sched_clock_cpu+0x126/0x170
[ 261.338995] ? check_flags.part.36+0x450/0x450
[ 261.342991] ? __lock_acquire+0x4500/0x4500
[ 261.348994] ? inet_sendmsg+0x11c/0x500
[ 261.352989] ? dst_output+0x180/0x180
[ 261.357012] inet_sendmsg+0x11c/0x500
[ ... ]
v2:
- clear skb->sk at reassembly routine.(Eric Dumarzet)
Fixes: fa0f527358bd ("ip: use rb trees for IP frag queue.")
Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/ipv4/ip_fragment.c | 1 +
net/ipv6/netfilter/nf_conntrack_reasm.c | 1 +
2 files changed, 2 insertions(+)
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -599,6 +599,7 @@ static int ip_frag_reasm(struct ipq *qp,
nextp = &fp->next;
fp->prev = NULL;
memset(&fp->rbnode, 0, sizeof(fp->rbnode));
+ fp->sk = NULL;
head->data_len += fp->len;
head->len += fp->len;
if (head->ip_summed != fp->ip_summed)
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -453,6 +453,7 @@ nf_ct_frag6_reasm(struct frag_queue *fq,
else if (head->ip_summed == CHECKSUM_COMPLETE)
head->csum = csum_add(head->csum, fp->csum);
head->truesize += fp->truesize;
+ fp->sk = NULL;
}
sub_frag_mem_limit(fq->q.net, head->truesize);
^ permalink raw reply
* [PATCH 4.14 118/126] ipv4: frags: precedence bug in ip_expire()
From: Greg Kroah-Hartman @ 2018-09-17 22:42 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable,
netdev@vger.kernel.org, stable@vger.kernel.org, edumazet@google.com, Dan Carpenter,
David S. Miller, Dan Carpenter
In-Reply-To: <20180917211703.481236999@linuxfoundation.org>
4.14-stable review patch. If anyone has any objections, please let me know.
------------------
From: Dan Carpenter <dan.carpenter@oracle.com>
We accidentally removed the parentheses here, but they are required
because '!' has higher precedence than '&'.
Fixes: fa0f527358bd ("ip: use rb trees for IP frag queue.")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 70837ffe3085c9a91488b52ca13ac84424da1042)
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/ipv4/ip_fragment.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -154,7 +154,7 @@ static void ip_expire(struct timer_list
__IP_INC_STATS(net, IPSTATS_MIB_REASMFAILS);
__IP_INC_STATS(net, IPSTATS_MIB_REASMTIMEOUT);
- if (!qp->q.flags & INET_FRAG_FIRST_IN)
+ if (!(qp->q.flags & INET_FRAG_FIRST_IN))
goto out;
/* sk_buff::dev and sk_buff::rbnode are unionized. So we
^ permalink raw reply
* [PATCH 4.14 098/126] ipv6: export ip6 fragments sysctl to unprivileged users
From: Greg Kroah-Hartman @ 2018-09-17 22:42 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable,
netdev@vger.kernel.org, stable@vger.kernel.org, edumazet@google.com, Nikolay Borisov,
Eric Dumazet, David S. Miller, Nikolay Borisov
In-Reply-To: <20180917211703.481236999@linuxfoundation.org>
4.14-stable review patch. If anyone has any objections, please let me know.
------------------
From: Eric Dumazet <edumazet@google.com>
IPv4 was changed in commit 52a773d645e9 ("net: Export ip fragment
sysctl to unprivileged users")
The only sysctl that is not per-netns is not used :
ip6frag_secret_interval
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Nikolay Borisov <kernel@kyup.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 18dcbe12fe9fca0ab825f7eff993060525ac2503)
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
net/ipv6/reassembly.c | 4 ----
1 file changed, 4 deletions(-)
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -649,10 +649,6 @@ static int __net_init ip6_frags_ns_sysct
table[1].data = &net->ipv6.frags.low_thresh;
table[1].extra2 = &net->ipv6.frags.high_thresh;
table[2].data = &net->ipv6.frags.timeout;
-
- /* Don't export sysctls to unprivileged users */
- if (net->user_ns != &init_user_ns)
- table[0].procname = NULL;
}
hdr = register_net_sysctl(net, "net/ipv6", table);
^ permalink raw reply
* [PATCH 4.14 095/126] inet: frags: Convert timers to use timer_setup()
From: Greg Kroah-Hartman @ 2018-09-17 22:42 UTC (permalink / raw)
To: linux-kernel
Cc: Greg Kroah-Hartman, stable, Alexander Aring, Stefan Schmidt,
David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI,
Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal, linux-wpan,
netdev, netfilter-devel, coreteam, Kees Cook
In-Reply-To: <20180917211703.481236999@linuxfoundation.org>
4.14-stable review patch. If anyone has any objections, please let me know.
------------------
From: Kees Cook <keescook@chromium.org>
In preparation for unconditionally passing the struct timer_list pointer to
all timer callbacks, switch to using the new timer_setup() and from_timer()
to pass the timer pointer explicitly.
Cc: Alexander Aring <alex.aring@gmail.com>
Cc: Stefan Schmidt <stefan@osg.samsung.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru>
Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org>
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Jozsef Kadlecsik <kadlec@blackhole.kfki.hu>
Cc: Florian Westphal <fw@strlen.de>
Cc: linux-wpan@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: netfilter-devel@vger.kernel.org
Cc: coreteam@netfilter.org
Signed-off-by: Kees Cook <keescook@chromium.org>
Acked-by: Stefan Schmidt <stefan@osg.samsung.com> # for ieee802154
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 78802011fbe34331bdef6f2dfb1634011f0e4c32)
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
include/net/inet_frag.h | 2 +-
net/ieee802154/6lowpan/reassembly.c | 5 +++--
net/ipv4/inet_fragment.c | 4 ++--
net/ipv4/ip_fragment.c | 5 +++--
net/ipv6/netfilter/nf_conntrack_reasm.c | 5 +++--
net/ipv6/reassembly.c | 5 +++--
6 files changed, 15 insertions(+), 11 deletions(-)
--- a/include/net/inet_frag.h
+++ b/include/net/inet_frag.h
@@ -97,7 +97,7 @@ struct inet_frags {
void (*constructor)(struct inet_frag_queue *q,
const void *arg);
void (*destructor)(struct inet_frag_queue *);
- void (*frag_expire)(unsigned long data);
+ void (*frag_expire)(struct timer_list *t);
struct kmem_cache *frags_cachep;
const char *frags_cache_name;
};
--- a/net/ieee802154/6lowpan/reassembly.c
+++ b/net/ieee802154/6lowpan/reassembly.c
@@ -80,12 +80,13 @@ static void lowpan_frag_init(struct inet
fq->daddr = *arg->dst;
}
-static void lowpan_frag_expire(unsigned long data)
+static void lowpan_frag_expire(struct timer_list *t)
{
+ struct inet_frag_queue *frag = from_timer(frag, t, timer);
struct frag_queue *fq;
struct net *net;
- fq = container_of((struct inet_frag_queue *)data, struct frag_queue, q);
+ fq = container_of(frag, struct frag_queue, q);
net = container_of(fq->q.net, struct net, ieee802154_lowpan.frags);
spin_lock(&fq->q.lock);
--- a/net/ipv4/inet_fragment.c
+++ b/net/ipv4/inet_fragment.c
@@ -150,7 +150,7 @@ inet_evict_bucket(struct inet_frags *f,
spin_unlock(&hb->chain_lock);
hlist_for_each_entry_safe(fq, n, &expired, list_evictor)
- f->frag_expire((unsigned long) fq);
+ f->frag_expire(&fq->timer);
return evicted;
}
@@ -367,7 +367,7 @@ static struct inet_frag_queue *inet_frag
f->constructor(q, arg);
add_frag_mem_limit(nf, f->qsize);
- setup_timer(&q->timer, f->frag_expire, (unsigned long)q);
+ timer_setup(&q->timer, f->frag_expire, 0);
spin_lock_init(&q->lock);
refcount_set(&q->refcnt, 1);
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -191,12 +191,13 @@ static bool frag_expire_skip_icmp(u32 us
/*
* Oops, a fragment queue timed out. Kill it and send an ICMP reply.
*/
-static void ip_expire(unsigned long arg)
+static void ip_expire(struct timer_list *t)
{
+ struct inet_frag_queue *frag = from_timer(frag, t, timer);
struct ipq *qp;
struct net *net;
- qp = container_of((struct inet_frag_queue *) arg, struct ipq, q);
+ qp = container_of(frag, struct ipq, q);
net = container_of(qp->q.net, struct net, ipv4.frags);
rcu_read_lock();
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -169,12 +169,13 @@ static unsigned int nf_hashfn(const stru
return nf_hash_frag(nq->id, &nq->saddr, &nq->daddr);
}
-static void nf_ct_frag6_expire(unsigned long data)
+static void nf_ct_frag6_expire(struct timer_list *t)
{
+ struct inet_frag_queue *frag = from_timer(frag, t, timer);
struct frag_queue *fq;
struct net *net;
- fq = container_of((struct inet_frag_queue *)data, struct frag_queue, q);
+ fq = container_of(frag, struct frag_queue, q);
net = container_of(fq->q.net, struct net, nf_frag.frags);
ip6_expire_frag_queue(net, fq);
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -169,12 +169,13 @@ out:
}
EXPORT_SYMBOL(ip6_expire_frag_queue);
-static void ip6_frag_expire(unsigned long data)
+static void ip6_frag_expire(struct timer_list *t)
{
+ struct inet_frag_queue *frag = from_timer(frag, t, timer);
struct frag_queue *fq;
struct net *net;
- fq = container_of((struct inet_frag_queue *)data, struct frag_queue, q);
+ fq = container_of(frag, struct frag_queue, q);
net = container_of(fq->q.net, struct net, ipv6.frags);
ip6_expire_frag_queue(net, fq);
^ permalink raw reply
* iproute2: fail to add fdb entries to ipv6 vxlan device
From: Lorenzo Bianconi @ 2018-09-17 17:13 UTC (permalink / raw)
To: netdev; +Cc: stephen, serhe.popovych
Hi all,
while working on IPv6 vlxan driver I figured out that with recent version
of iproute2 it is no longer possible to configure an IPv6 vxlan device without
endpoint info (local ip, remote ip or group ip) and later add entries in
the vxlan fdb. This issue can be triggered with the following reproducer:
$ip -6 link add vxlan0 type vxlan id 42 dev enp0s2 \
proxy nolearning l2miss l3miss
$bridge fdb add 46:47:1f:a7:1c:25 dev vxlan1 dst 2000::2
RTNETLINK answers: Address family not supported by protocol
Starting from commit 1e9b8072de2c ("iplink_vxlan: Get rid of inet_get_addr()")
the preferred_family is no longer taken into account if neither saddr or daddr
are provided and vxlan kernel module will use IPv4 as default remote inet
family neglecting the one provided by userspace.
I guess we can fix that issue in two ways:
1- add a new netlink attribute to vxlan driver in order to specify the
preferred_family to use
2- restore the behaviour introduced in commit 97d564b90ccb ("vxlan: use
preferred address family when neither group or remote is specified") with
the following patch
-- >8 --
diff --git a/ip/iplink_vxlan.c b/ip/iplink_vxlan.c
index 2bc253fc..831f39a2 100644
--- a/ip/iplink_vxlan.c
+++ b/ip/iplink_vxlan.c
@@ -82,6 +82,7 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
__u64 attrs = 0;
bool set_op = (n->nlmsg_type == RTM_NEWLINK &&
!(n->nlmsg_flags & NLM_F_CREATE));
+ bool selected_family = false;
saddr.family = daddr.family = AF_UNSPEC;
@@ -356,12 +357,26 @@ static int vxlan_parse_opt(struct link_util *lu, int argc, char **argv,
int type = (saddr.family == AF_INET) ? IFLA_VXLAN_LOCAL
: IFLA_VXLAN_LOCAL6;
addattr_l(n, 1024, type, saddr.data, saddr.bytelen);
+ selected_family = true;
}
if (is_addrtype_inet(&daddr)) {
int type = (daddr.family == AF_INET) ? IFLA_VXLAN_GROUP
: IFLA_VXLAN_GROUP6;
addattr_l(n, 1024, type, daddr.data, daddr.bytelen);
+ selected_family = true;
+ }
+
+ if (!selected_family) {
+ if (preferred_family == AF_INET) {
+ get_addr(&daddr, "default", AF_INET);
+ addattr_l(n, 1024, IFLA_VXLAN_GROUP,
+ daddr.data, daddr.bytelen);
+ } else if (preferred_family == AF_INET6) {
+ get_addr(&daddr, "default", AF_INET6);
+ addattr_l(n, 1024, IFLA_VXLAN_GROUP6,
+ daddr.data, daddr.bytelen);
+ }
}
if (!set_op || VXLAN_ATTRSET(attrs, IFLA_VXLAN_LEARNING))
-- 8< --
What is the best way to proceed?
Regards,
Lorenzo
^ permalink raw reply related
* Re: [PATCH net] net/ipv6: do not copy DST_NOCOUNT flag on rt init
From: David Ahern @ 2018-09-17 16:59 UTC (permalink / raw)
To: Peter Oskolkov; +Cc: davem, netdev
In-Reply-To: <CAPNVh5f3_GPpvV6nequZ8JNBhMVVOpEsJQ_XRfTN-+eaMPOWpw@mail.gmail.com>
On 9/17/18 9:11 AM, Peter Oskolkov wrote:
> On Thu, Sep 13, 2018 at 9:11 PM David Ahern <dsahern@gmail.com> wrote:
>>
>> On 9/13/18 1:38 PM, Peter Oskolkov wrote:
>>
>>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>>> index 3eed045c65a5..a3902f805305 100644
>>> --- a/net/ipv6/route.c
>>> +++ b/net/ipv6/route.c
>>> @@ -946,7 +946,7 @@ static void ip6_rt_init_dst_reject(struct rt6_info *rt, struct fib6_info *ort)
>>>
>>> static void ip6_rt_init_dst(struct rt6_info *rt, struct fib6_info *ort)
>>> {
>>> - rt->dst.flags |= fib6_info_dst_flags(ort);
>>> + rt->dst.flags |= fib6_info_dst_flags(ort) & ~DST_NOCOUNT;
>>
>> I think my mistake is setting dst.flags in ip6_rt_init_dst. Flags
>> argument is passed to ip6_dst_alloc which is always invoked before
>> ip6_rt_copy_init is called which is the only caller of ip6_rt_init_dst.
>
> ip6_rt_cache_alloc calls ip6_dst_alloc with zero as flags; and only
> one flag is copied later (DST_HOST) outside of ip6_rt_init_dst().
> If the flag assignment is completely removed from ip6_rt_init_dst(),
> then DST_NOPOLICY flag will be lost.
>
> Which may be OK, but is more than what this patch tries to solve (do not
> copy DST_NOCOUNT flag).
In the 4.17 kernel (prior to the fib6_info change), ip6_rt_cache_alloc
calls __ip6_dst_alloc with 0 for flags so this is correct. The mistake
is ip6_rt_copy_init -> ip6_rt_init_dst -> fib6_info_dst_flags.
I believe the right fix is to drop the 'rt->dst.flags |=
fib6_info_dst_flags(ort);' from ip6_rt_init_dst.
^ permalink raw reply
* Re: [PATCH v2 2/4] dt-bindings: net: qcom: Add binding for shared mdio bus
From: Florian Fainelli @ 2018-09-17 16:54 UTC (permalink / raw)
To: Wang, Dongsheng, Andrew Lunn
Cc: timur@kernel.org, davem@davemloft.net, Zheng, Joey,
netdev@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <b01bb33b30164088a9cae59a825acdc9@HXTBJIDCEMVIW02.hxtcorp.net>
On 09/17/2018 09:47 AM, Wang, Dongsheng wrote:
> On 9/17/2018 10:50 PM, Andrew Lunn wrote:
>> On Mon, Sep 17, 2018 at 04:53:29PM +0800, Wang Dongsheng wrote:
>>> This property copy from "ibm,emac.txt" to describe a shared MIDO bus.
>>> Since emac include MDIO, so If the motherboard has more than one PHY
>>> connected to an MDIO bus, this property will point to the MAC device
>>> that has the MDIO bus.
>>>
>>> Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>
>>> ---
>>> V2: s/Since QDF2400 emac/Since emac/
>>> ---
>>> Documentation/devicetree/bindings/net/qcom-emac.txt | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/qcom-emac.txt b/Documentation/devicetree/bindings/net/qcom-emac.txt
>>> index 346e6c7f47b7..50db71771358 100644
>>> --- a/Documentation/devicetree/bindings/net/qcom-emac.txt
>>> +++ b/Documentation/devicetree/bindings/net/qcom-emac.txt
>>> @@ -24,6 +24,9 @@ Internal PHY node:
>>> The external phy child node:
>>> - reg : The phy address
>>>
>>> +Optional properties:
>>> +- mdio-device : Shared MIDO bus.
>> Hi Dongsheng
>>
>> I don't see why you need this property. The ethernet interface has a
>> phy-handle which points to a PHY. That is all you need to find the PHY.
> phy-handle is description PHY address. This property is describing an
> MDIO controller.
> Each QCOM emac include an MDIO controller, normally each emac only
> connect one
> PHY device, but when all of the PHY devices mdio lines connect one MDIO
> controller
> that is included in EMAC, we need to share this MDIO controller for
> others EMAC.
If you want to describe the MDIO controller, then you embed a mdio
subnode into your Ethernet MAC node:
emac0: ethernet@feb20000 {
mdio {
#address-cells = <1>;
#size-cells = <0>;
phy0: ethernet-phy@0 {
reg = <0>;
};
};
};
And then each Ethernet MAC controller refers to their appropriate PHY
device tree node using a phy-handle property to point to either their
own MDIO controller, or another MAC's MDIO controller.
The IBM Emac is a old (not to say bad) example and it does not use the
PHY library and the standard Device Tree node property, please don't use
it as a reference.
--
Florian
^ permalink raw reply
* Re: [PATCH v2 2/4] dt-bindings: net: qcom: Add binding for shared mdio bus
From: Wang, Dongsheng @ 2018-09-17 16:47 UTC (permalink / raw)
To: Andrew Lunn
Cc: timur@kernel.org, davem@davemloft.net, Zheng, Joey,
netdev@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <20180917145028.GC5458@lunn.ch>
On 9/17/2018 10:50 PM, Andrew Lunn wrote:
> On Mon, Sep 17, 2018 at 04:53:29PM +0800, Wang Dongsheng wrote:
>> This property copy from "ibm,emac.txt" to describe a shared MIDO bus.
>> Since emac include MDIO, so If the motherboard has more than one PHY
>> connected to an MDIO bus, this property will point to the MAC device
>> that has the MDIO bus.
>>
>> Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>
>> ---
>> V2: s/Since QDF2400 emac/Since emac/
>> ---
>> Documentation/devicetree/bindings/net/qcom-emac.txt | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/qcom-emac.txt b/Documentation/devicetree/bindings/net/qcom-emac.txt
>> index 346e6c7f47b7..50db71771358 100644
>> --- a/Documentation/devicetree/bindings/net/qcom-emac.txt
>> +++ b/Documentation/devicetree/bindings/net/qcom-emac.txt
>> @@ -24,6 +24,9 @@ Internal PHY node:
>> The external phy child node:
>> - reg : The phy address
>>
>> +Optional properties:
>> +- mdio-device : Shared MIDO bus.
> Hi Dongsheng
>
> I don't see why you need this property. The ethernet interface has a
> phy-handle which points to a PHY. That is all you need to find the PHY.
phy-handle is description PHY address. This property is describing an
MDIO controller.
Each QCOM emac include an MDIO controller, normally each emac only
connect one
PHY device, but when all of the PHY devices mdio lines connect one MDIO
controller
that is included in EMAC, we need to share this MDIO controller for
others EMAC.
Normally:
(MDIO)
MAC0 ---------------------------------------PHY0
| |
| (DATA) |
---------------------------------------------
(MDIO)
MAC1 ---------------------------------------PHY1
| |
| (DATA) |
---------------------------------------------
Shared MDIO bus: "mdio-device" = &MAC0, MAC1 will get MAC0's MDIO bus
and also get the corresponding PHY device.
(DATA)
MAC0 ---------------------------------------PHY0
| |
| (MDIO) |
--------------------------------------------
|
MAC1 PHY0
| |
| (DATA) |
--------------------------------------------
Cheers,
Dongsheng
> emac0: ethernet@feb20000 {
> compatible = "qcom,fsm9900-emac";
> reg = <0xfeb20000 0x10000>,
> <0xfeb36000 0x1000>;
> interrupts = <76>;
>
> clocks = <&gcc 0>, <&gcc 1>, <&gcc 3>, <&gcc 4>, <&gcc 5>,
> <&gcc 6>, <&gcc 7>;
> clock-names = "axi_clk", "cfg_ahb_clk", "high_speed_clk",
> "mdio_clk", "tx_clk", "rx_clk", "sys_clk";
>
> internal-phy = <&emac_sgmii>;
>
> phy-handle = <&phy0>;
>
> #address-cells = <1>;
> #size-cells = <0>;
>
> phy0: ethernet-phy@0 {
> reg = <0>;
> };
>
> phy1: ethernet-phy@1 {
> reg = <1>;
> };
>
> pinctrl-names = "default";
> pinctrl-0 = <&mdio_pins_a>;
> };
>
> emac1: ethernet@38900000 {
> compatible = "qcom,fsm9900-emac";
> ...
> ...
>
> phy-handle = <&phy1>;
> };
>
> Andrew
>
^ permalink raw reply
* Re: [PATCH net] net/ipv4: defensive cipso option parsing
From: Paul Moore @ 2018-09-17 16:35 UTC (permalink / raw)
To: snu; +Cc: netdev, aams, yujuan.qi, stable
In-Reply-To: <20180917151149.22231-1-snu@amazon.com>
On Mon, Sep 17, 2018 at 11:12 AM Stefan Nuernberger <snu@amazon.com> wrote:
> commit 40413955ee26 ("Cipso: cipso_v4_optptr enter infinite loop") fixed
> a possible infinite loop in the IP option parsing of CIPSO. The fix
> assumes that ip_options_compile filtered out all zero length options and
> that no other one-byte options beside IPOPT_END and IPOPT_NOOP exist.
> While this assumption currently holds true, add explicit checks for zero
> length and invalid length options to be safe for the future. Even though
> ip_options_compile should have validated the options, the introduction of
> new one-byte options can still confuse this code without the additional
> checks.
>
> Signed-off-by: Stefan Nuernberger <snu@amazon.com>
> Reviewed-by: David Woodhouse <dwmw@amazon.co.uk>
> Reviewed-by: Simon Veith <sveith@amazon.de>
> Cc: stable@vger.kernel.org
> ---
> net/ipv4/cipso_ipv4.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
> index 82178cc69c96..f291b57b8474 100644
> --- a/net/ipv4/cipso_ipv4.c
> +++ b/net/ipv4/cipso_ipv4.c
> @@ -1512,7 +1512,7 @@ static int cipso_v4_parsetag_loc(const struct cipso_v4_doi *doi_def,
> *
> * Description:
> * Parse the packet's IP header looking for a CIPSO option. Returns a pointer
> - * to the start of the CIPSO option on success, NULL if one if not found.
> + * to the start of the CIPSO option on success, NULL if one is not found.
> *
> */
> unsigned char *cipso_v4_optptr(const struct sk_buff *skb)
> @@ -1522,9 +1522,11 @@ unsigned char *cipso_v4_optptr(const struct sk_buff *skb)
> int optlen;
> int taglen;
>
> - for (optlen = iph->ihl*4 - sizeof(struct iphdr); optlen > 0; ) {
> + for (optlen = iph->ihl*4 - sizeof(struct iphdr); optlen > 1; ) {
> switch (optptr[0]) {
> case IPOPT_CIPSO:
> + if (!optptr[1] || optptr[1] > optlen)
> + return NULL;
> return optptr;
> case IPOPT_END:
> return NULL;
> @@ -1534,6 +1536,10 @@ unsigned char *cipso_v4_optptr(const struct sk_buff *skb)
> default:
> taglen = optptr[1];
> }
> +
> + if (!taglen || taglen > optlen)
> + break;
I tend to think that you reach a point where you simply need to trust
that the stack is doing the right thing and that by the time you hit a
certain point you can safely assume that the packet is well formed,
but I'm not going to fight about that here.
Regardless of the above, I don't like how you're doing the option
length check twice in this code, that looks ugly to me, I think we can
do better. How about something like this:
for (...) {
switch(optptr[0]) {
case IPOPT_END:
return NULL;
case IPOPT_NOOP:
taglen = 1;
default:
taglen = optptr[1];
}
if (taglen == 0 || taglen > optlen)
return NULL;
if (optptr[0] == IPOPT_CIPSO)
return optptr;
....
}
> optlen -= taglen;
> optptr += taglen;
> }
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH net] net/ipv6: do not copy DST_NOCOUNT flag on rt init
From: David Ahern @ 2018-09-17 16:13 UTC (permalink / raw)
To: Peter Oskolkov; +Cc: davem, netdev
In-Reply-To: <CAPNVh5f3_GPpvV6nequZ8JNBhMVVOpEsJQ_XRfTN-+eaMPOWpw@mail.gmail.com>
On 9/17/18 9:11 AM, Peter Oskolkov wrote:
> On Thu, Sep 13, 2018 at 9:11 PM David Ahern <dsahern@gmail.com> wrote:
>>
>> On 9/13/18 1:38 PM, Peter Oskolkov wrote:
>>
>>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>>> index 3eed045c65a5..a3902f805305 100644
>>> --- a/net/ipv6/route.c
>>> +++ b/net/ipv6/route.c
>>> @@ -946,7 +946,7 @@ static void ip6_rt_init_dst_reject(struct rt6_info *rt, struct fib6_info *ort)
>>>
>>> static void ip6_rt_init_dst(struct rt6_info *rt, struct fib6_info *ort)
>>> {
>>> - rt->dst.flags |= fib6_info_dst_flags(ort);
>>> + rt->dst.flags |= fib6_info_dst_flags(ort) & ~DST_NOCOUNT;
>>
>> I think my mistake is setting dst.flags in ip6_rt_init_dst. Flags
>> argument is passed to ip6_dst_alloc which is always invoked before
>> ip6_rt_copy_init is called which is the only caller of ip6_rt_init_dst.
>
> ip6_rt_cache_alloc calls ip6_dst_alloc with zero as flags; and only
> one flag is copied later (DST_HOST) outside of ip6_rt_init_dst().
> If the flag assignment is completely removed from ip6_rt_init_dst(),
> then DST_NOPOLICY flag will be lost.
>
> Which may be OK, but is more than what this patch tries to solve (do not
> copy DST_NOCOUNT flag).
After 5+ days mostly offline I just started looking at this problem.
Give me some time to chase down a thought I had from my last response.
^ permalink raw reply
* Re: [PATCH net] net/ipv6: do not copy DST_NOCOUNT flag on rt init
From: Peter Oskolkov @ 2018-09-17 16:11 UTC (permalink / raw)
To: dsahern; +Cc: davem, netdev
In-Reply-To: <351e6fbc-5356-b5c1-0638-0554843ad833@gmail.com>
On Thu, Sep 13, 2018 at 9:11 PM David Ahern <dsahern@gmail.com> wrote:
>
> On 9/13/18 1:38 PM, Peter Oskolkov wrote:
>
> > diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> > index 3eed045c65a5..a3902f805305 100644
> > --- a/net/ipv6/route.c
> > +++ b/net/ipv6/route.c
> > @@ -946,7 +946,7 @@ static void ip6_rt_init_dst_reject(struct rt6_info *rt, struct fib6_info *ort)
> >
> > static void ip6_rt_init_dst(struct rt6_info *rt, struct fib6_info *ort)
> > {
> > - rt->dst.flags |= fib6_info_dst_flags(ort);
> > + rt->dst.flags |= fib6_info_dst_flags(ort) & ~DST_NOCOUNT;
>
> I think my mistake is setting dst.flags in ip6_rt_init_dst. Flags
> argument is passed to ip6_dst_alloc which is always invoked before
> ip6_rt_copy_init is called which is the only caller of ip6_rt_init_dst.
ip6_rt_cache_alloc calls ip6_dst_alloc with zero as flags; and only
one flag is copied later (DST_HOST) outside of ip6_rt_init_dst().
If the flag assignment is completely removed from ip6_rt_init_dst(),
then DST_NOPOLICY flag will be lost.
Which may be OK, but is more than what this patch tries to solve (do not
copy DST_NOCOUNT flag).
>
> >
> > if (ort->fib6_flags & RTF_REJECT) {
> > ip6_rt_init_dst_reject(rt, ort);
> >
>
^ permalink raw reply
* Re: [PATCH net-next 00/15] s390/qeth: updates 2018-09-17
From: David Miller @ 2018-09-17 16:10 UTC (permalink / raw)
To: jwi; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20180917153609.94628-1-jwi@linux.ibm.com>
From: Julian Wiedmann <jwi@linux.ibm.com>
Date: Mon, 17 Sep 2018 17:35:54 +0200
> please apply the following patchset to net-next. This brings more restructuring
> of qeth's transmit code (eliminating its last usage of skb_realloc_headroom()),
> and the usual mix of minor improvements & cleanups.
Series applied, thank you.
^ permalink raw reply
* Re: [PATCH net-next v3 02/17] zinc: introduce minimal cryptography library
From: Andy Lutomirski @ 2018-09-17 16:07 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: Ard Biesheuvel, Andrew Lutomirski, David S. Miller, andrew,
Eric Biggers, Greg KH, LKML, Network Development, Samuel Neves,
Jean-Philippe Aumasson, Linux Crypto Mailing List
In-Reply-To: <CAHmME9o2sB-rQo0DRh56AFe7ZLJJh+CU6Z1m5XZ4tA8vATCy1g@mail.gmail.com>
On Mon, Sep 17, 2018 at 8:32 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> On Mon, Sep 17, 2018 at 4:52 PM Andy Lutomirski <luto@amacapital.net> wrote:
> > I think the module organization needs to change. It needs to be possible to have chacha20 built in but AES or whatever as a module.
>
> Okay, I'll do that for v5.
>
> > I might have agreed before Spectre :(. Unfortunately, unless we do some magic, I think the code would look something like:
> >
> > if (static_branch_likely(have_simd)) arch_chacha20();
> >
> > ...where arch_chacha20 is a *pointer*. And that will generate a retpoline and run very, very slowly. (I just rewrote some of the x86 entry code to eliminate one retpoline. I got a 5% speedup on some tests according to the kbuild bot.)
>
> Actually, the way it works now benefits from the compilers inliner and
> the branch predictor. I benchmarked this without any retpoline
> slowdowns, and the branch predictor becomes correct pretty much all
> the time. We can tinker with this after the initial merge, if you
> really want, but avoiding function pointers and instead using ordinary
> branches really winds up being quite fast.
Indeed. What I'm saying is that you shouldn't refactor it this way
because it will be slow. I agree it would be conceptually nice to be
able to blacklist a chacha20_x86_64 module to disable the asm, but I
think it would be very hard to get good performance.
--Andy
^ permalink raw reply
* Re: What is the best forum (mailing list, irc etc) to ask questions about the usage of AF_XDP sockets.
From: Konrad Djimeli @ 2018-09-17 16:04 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev, bjorn.topel
In-Reply-To: <20180913095227.26b7305d@cakuba.netronome.com>
On 2018-09-13 18:52, Jakub Kicinski wrote:
> On Thu, 13 Sep 2018 18:31:55 +0200, Konrad Djimeli wrote:
>> Hello,
>>
>> I have been working on trying to make use of AF_XDP sockets as part of a
>> project I working on, and I have been facing some issues but I am not
>> sure where to ask questions related to the usage of AF_XDP, since this
>> is a development mailing list.
>
> IMHO AF_XDP is quite fresh so it should be okay to ask questions on
> netdev. There is also xdp-newbies mailing list which seems very
> appropriate for less advanced questions!
Thanks a lot for the suggestions. Though I am still very new to the
technology, I really enjoy working with AF_XDP and I hope to share my
experience the and queries with the community. Below is link to what I
have been working on, so far.
-
https://github.com/djkonro/snabb/commit/74310142882a5f09487c676f61f75b3bd686783c
Thanks
Konrad
www.djimeli.me
^ permalink raw reply
* KASAN: slab-out-of-bounds Read in ip6_tnl_parse_tlv_enc_lim
From: syzbot @ 2018-09-17 21:23 UTC (permalink / raw)
To: davem, kuznet, linux-kernel, netdev, syzkaller-bugs, yoshfuji
Hello,
syzbot found the following crash on:
HEAD commit: 3a5af36b6d0e Merge tag '4.19-rc3-smb3-cifs' of git://git.s..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=15c727be400000
kernel config: https://syzkaller.appspot.com/x/.config?x=9c4a80625153107e
dashboard link: https://syzkaller.appspot.com/bug?extid=68dce7caebd8543121de
compiler: gcc (GCC) 8.0.1 20180413 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1068a44e400000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=146386c6400000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+68dce7caebd8543121de@syzkaller.appspotmail.com
IPv6: ADDRCONF(NETDEV_UP): veth1: link is not ready
IPv6: ADDRCONF(NETDEV_CHANGE): veth1: link becomes ready
IPv6: ADDRCONF(NETDEV_CHANGE): veth0: link becomes ready
8021q: adding VLAN 0 to HW filter on device team0
==================================================================
BUG: KASAN: slab-out-of-bounds in ip6_tnl_parse_tlv_enc_lim+0x5df/0x660
net/ipv6/ip6_tunnel.c:417
Read of size 1 at addr ffff8801d21187c7 by task syz-executor281/5345
CPU: 1 PID: 5345 Comm: syz-executor281 Not tainted 4.19.0-rc3+ #238
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c4/0x2b4 lib/dump_stack.c:113
print_address_description.cold.8+0x9/0x1ff mm/kasan/report.c:256
kasan_report_error mm/kasan/report.c:354 [inline]
kasan_report.cold.9+0x242/0x309 mm/kasan/report.c:412
__asan_report_load1_noabort+0x14/0x20 mm/kasan/report.c:430
ip6_tnl_parse_tlv_enc_lim+0x5df/0x660 net/ipv6/ip6_tunnel.c:417
ip6ip6_tnl_xmit net/ipv6/ip6_tunnel.c:1339 [inline]
ip6_tnl_start_xmit+0x3e2/0x2370 net/ipv6/ip6_tunnel.c:1403
__netdev_start_xmit include/linux/netdevice.h:4287 [inline]
netdev_start_xmit include/linux/netdevice.h:4296 [inline]
xmit_one net/core/dev.c:3216 [inline]
dev_hard_start_xmit+0x27f/0xc70 net/core/dev.c:3232
__dev_queue_xmit+0x2f3b/0x3980 net/core/dev.c:3802
dev_queue_xmit+0x17/0x20 net/core/dev.c:3835
__bpf_tx_skb net/core/filter.c:2012 [inline]
__bpf_redirect_common net/core/filter.c:2050 [inline]
__bpf_redirect+0x5cf/0xb20 net/core/filter.c:2057
____bpf_clone_redirect net/core/filter.c:2090 [inline]
bpf_clone_redirect+0x2f6/0x490 net/core/filter.c:2062
bpf_prog_759a992c578a3894+0x741/0x1000
Allocated by task 5345:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
kasan_kmalloc+0xc7/0xe0 mm/kasan/kasan.c:553
__do_kmalloc_node mm/slab.c:3682 [inline]
__kmalloc_node_track_caller+0x47/0x70 mm/slab.c:3696
__kmalloc_reserve.isra.39+0x41/0xe0 net/core/skbuff.c:137
pskb_expand_head+0x230/0x10f0 net/core/skbuff.c:1460
skb_ensure_writable+0x3dd/0x640 net/core/skbuff.c:5126
__bpf_try_make_writable net/core/filter.c:1633 [inline]
bpf_try_make_writable net/core/filter.c:1639 [inline]
bpf_try_make_head_writable net/core/filter.c:1647 [inline]
____bpf_clone_redirect net/core/filter.c:2084 [inline]
bpf_clone_redirect+0x14a/0x490 net/core/filter.c:2062
bpf_prog_759a992c578a3894+0x741/0x1000
Freed by task 3895:
save_stack+0x43/0xd0 mm/kasan/kasan.c:448
set_track mm/kasan/kasan.c:460 [inline]
__kasan_slab_free+0x102/0x150 mm/kasan/kasan.c:521
kasan_slab_free+0xe/0x10 mm/kasan/kasan.c:528
__cache_free mm/slab.c:3498 [inline]
kfree+0xcf/0x230 mm/slab.c:3813
load_elf_binary+0x25b4/0x5620 fs/binfmt_elf.c:1118
search_binary_handler+0x17d/0x570 fs/exec.c:1653
exec_binprm fs/exec.c:1695 [inline]
__do_execve_file.isra.33+0x162f/0x2540 fs/exec.c:1819
do_execveat_common fs/exec.c:1866 [inline]
do_execve fs/exec.c:1883 [inline]
__do_sys_execve fs/exec.c:1964 [inline]
__se_sys_execve fs/exec.c:1959 [inline]
__x64_sys_execve+0x8f/0xc0 fs/exec.c:1959
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
The buggy address belongs to the object at ffff8801d21185c0
which belongs to the cache kmalloc-512 of size 512
The buggy address is located 7 bytes to the right of
512-byte region [ffff8801d21185c0, ffff8801d21187c0)
The buggy address belongs to the page:
page:ffffea0007484600 count:1 mapcount:0 mapping:ffff8801da800940 index:0x0
flags: 0x2fffc0000000100(slab)
raw: 02fffc0000000100 ffffea0007484008 ffffea0007487e88 ffff8801da800940
raw: 0000000000000000 ffff8801d21180c0 0000000100000006 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8801d2118680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff8801d2118700: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff8801d2118780: 00 00 00 00 00 00 00 00 fc fc fc fc fc fc fc fc
^
ffff8801d2118800: fc fc fc fc fc fc fc fc fb fb fb fb fb fb fb fb
ffff8801d2118880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH] net: phy: phylink: fix SFP interface autodetection
From: Baruch Siach @ 2018-09-17 15:39 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Andrew Lunn, Florian Fainelli, netdev, Antoine Tenart,
Gregory CLEMENT
In-Reply-To: <20180917151230.GF30658@n2100.armlinux.org.uk>
Hi Russell,
Russell King - ARM Linux writes:
> On Mon, Sep 17, 2018 at 05:19:57PM +0300, Baruch Siach wrote:
>> When the switching to the SFP detected link mode update the main
>> link_interface field as well. Otherwise, the link fails to come up when
>> the configured 'phy-mode' defers from the SFP detected mode.
>>
>> This fixes 1GB SFP module link up on eth3 of the Macchiatobin board that
>> is configured in the DT to "2500base-x" phy-mode.
>
> link_interface isn't supposed to track the SFP link mode. In any case,
> this is only used when a PHY is attached. For a PHY on a SFP,
> phylink_connect_phy() should be using link_config.interface and not
> link_interface there.
You mean something like this?
@@ -750,7 +750,7 @@ int phylink_connect_phy(struct phylink *pl, struct phy_device *phy)
pl->link_config.interface = pl->link_interface;
}
- ret = phy_attach_direct(pl->netdev, phy, 0, pl->link_interface);
+ ret = phy_attach_direct(pl->netdev, phy, 0, pl->link_config.interface);
if (ret)
return ret;
This fixes a similar issue on the Armada 8040 based Clearfog GT-8K where
the SFP interface phy-mode is set to "10gbase-kr". But the Macchiatobin
PHY-less interface has phy-mode set to "2500base-x", so I hit a separate
problem:
[ 49.599455] WARNING: CPU: 1 PID: 1223 at drivers/net/phy/phylink.c:741 phylink_connect_phy+0x34/0xb4
[ 49.608629] CPU: 1 PID: 1223 Comm: ifconfig Not tainted 4.19.0-rc4-00006-g18cf7eb4b625-dirty #952
[ 49.617537] Hardware name: Marvell 8040 MACCHIATOBin (DT)
[ 49.622957] pstate: 60000005 (nZCv daif -PAN -UAO)
[ 49.627767] pc : phylink_connect_phy+0x34/0xb4
[ 49.632229] lr : phylink_sfp_connect_phy+0xc/0x14
[ 49.636950] sp : ffff00000b273830
[ 49.640276] x29: ffff00000b273830 x28: ffff80007a342df0
[ 49.645610] x27: 00000000000004b0 x26: 0000000000000004
[ 49.650945] x25: ffff80007a342940 x24: ffff80007b90cc18
[ 49.656278] x23: ffff8000799b2538 x22: ffff000008bda000
[ 49.661613] x21: ffff80007a342000 x20: ffff80007bca4000
[ 49.666947] x19: ffff80007b814a80 x18: 000000000000000a
[ 49.672282] x17: 0000000000000000 x16: 0000000000000004
[ 49.677616] x15: 0000000000000416 x14: 0000000000000400
[ 49.682951] x13: 0000000000000400 x12: 0000000000000000
[ 49.688284] x11: 0000000000000000 x10: 0000000000000000
[ 49.693618] x9 : 00000000000002c6 x8 : 0000000000000000
[ 49.698952] x7 : ffff80007ff88b40 x6 : 0000000000000001
[ 49.704286] x5 : 0000000000000003 x4 : ffff80007bca4048
[ 49.709621] x3 : 0000000000000004 x2 : 0000000000000001
[ 49.714955] x1 : ffff80007bca4000 x0 : ffff80007a337400
[ 49.720289] Call trace:
[ 49.722745] phylink_connect_phy+0x34/0xb4
[ 49.726857] phylink_sfp_connect_phy+0xc/0x14
[ 49.731231] sfp_add_phy+0x48/0x50
[ 49.734647] sfp_sm_event+0x6e0/0x86c
[ 49.738323] sfp_start+0x10/0x18
[ 49.741563] sfp_upstream_start+0x28/0x3c
[ 49.745588] phylink_start+0x120/0x164
[ 49.749352] mvpp2_start_dev+0x184/0x260
[ 49.753290] mvpp2_open+0x394/0x3e8
[ 49.756793] __dev_open+0x110/0x124
[ 49.760294] __dev_change_flags+0xe8/0x190
[ 49.764406] dev_change_flags+0x20/0x5c
[ 49.768258] devinet_ioctl+0x270/0x528
[ 49.772020] inet_ioctl+0x13c/0x2e4
[ 49.775524] sock_do_ioctl+0x4c/0x1f4
[ 49.779200] sock_ioctl+0xf4/0x360
[ 49.782616] vfs_ioctl+0x24/0x40
[ 49.785855] do_vfs_ioctl+0xa4/0x7d4
[ 49.789443] ksys_ioctl+0x44/0x74
[ 49.792770] __arm64_sys_ioctl+0x18/0x24
[ 49.796708] el0_svc_common+0x8c/0xd0
[ 49.800384] el0_svc_handler+0x3c/0x6c
[ 49.804148] el0_svc+0x8/0xc
[ 49.807039] ---[ end trace 0d1e0898561fce6e ]---
[ 49.811729] sfp sfp-eth3: sfp_add_phy failed: -22
This hunk fixes that one:
@@ -738,7 +738,7 @@ int phylink_connect_phy(struct phylink *pl, struct phy_device *phy)
if (WARN_ON(pl->link_an_mode == MLO_AN_FIXED ||
(pl->link_an_mode == MLO_AN_INBAND &&
- phy_interface_mode_is_8023z(pl->link_interface))))
+ phy_interface_mode_is_8023z(pl->link_config.interface))))
return -EINVAL;
if (pl->phydev)
Is that the correct fix?
Thanks,
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
^ permalink raw reply
* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
From: Andrew Lunn @ 2018-09-17 15:38 UTC (permalink / raw)
To: Simon Horman; +Cc: David Miller, netdev, Florian Fainelli
In-Reply-To: <20180917151302.l6mzzy5xmqlbgejj@verge.net.au>
On Mon, Sep 17, 2018 at 05:13:07PM +0200, Simon Horman wrote:
> On Wed, Sep 12, 2018 at 01:53:14AM +0200, Andrew Lunn wrote:
> > Some MAC hardware cannot support a subset of link modes. e.g. often
> > 1Gbps Full duplex is supported, but Half duplex is not. Add a helper
> > to remove such a link mode.
> >
> > Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> > Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> > ---
> > drivers/net/ethernet/apm/xgene/xgene_enet_hw.c | 6 +++---
> > drivers/net/ethernet/cadence/macb_main.c | 5 ++---
> > drivers/net/ethernet/freescale/fec_main.c | 3 ++-
> > drivers/net/ethernet/microchip/lan743x_main.c | 2 +-
> > drivers/net/ethernet/renesas/ravb_main.c | 3 ++-
> > .../net/ethernet/stmicro/stmmac/stmmac_main.c | 12 ++++++++----
> > drivers/net/phy/phy_device.c | 18 ++++++++++++++++++
> > drivers/net/usb/lan78xx.c | 2 +-
> > include/linux/phy.h | 1 +
> > 9 files changed, 38 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
> > index 078a04dc1182..4831f9de5945 100644
>
> ...
>
> > diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> > index aff5516b781e..fb2a1125780d 100644
> > --- a/drivers/net/ethernet/renesas/ravb_main.c
> > +++ b/drivers/net/ethernet/renesas/ravb_main.c
> > @@ -1074,7 +1074,8 @@ static int ravb_phy_init(struct net_device *ndev)
> > }
> >
> > /* 10BASE is not supported */
> > - phydev->supported &= ~PHY_10BT_FEATURES;
> > + phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
> > + phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
> >
> > phy_attached_info(phydev);
> >
>
> ...
>
> > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> > index db1172db1e7c..e9ca83a438b0 100644
> > --- a/drivers/net/phy/phy_device.c
> > +++ b/drivers/net/phy/phy_device.c
> > @@ -1765,6 +1765,24 @@ int phy_set_max_speed(struct phy_device *phydev, u32 max_speed)
> > }
> > EXPORT_SYMBOL(phy_set_max_speed);
> >
> > +/**
> > + * phy_remove_link_mode - Remove a supported link mode
> > + * @phydev: phy_device structure to remove link mode from
> > + * @link_mode: Link mode to be removed
> > + *
> > + * Description: Some MACs don't support all link modes which the PHY
> > + * does. e.g. a 1G MAC often does not support 1000Half. Add a helper
> > + * to remove a link mode.
> > + */
> > +void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode)
> > +{
> > + WARN_ON(link_mode > 31);
> > +
> > + phydev->supported &= ~BIT(link_mode);
> > + phydev->advertising = phydev->supported;
> > +}
> > +EXPORT_SYMBOL(phy_remove_link_mode);
> > +
> > static void of_set_phy_supported(struct phy_device *phydev)
> > {
> > struct device_node *node = phydev->mdio.dev.of_node;
>
> Hi Andrew,
>
> I believe that for the RAVB the overall effect of this change is that
> 10-BaseT modes are no longer advertised (although both with and without
> this patch they are not supported).
>
> Unfortunately on R-Car Gen3 M3-W (r8a7796) based Salvator-X board
> I have observed that this results in the link no longer being negotiated
> on one switch (the one I usually use) while it seemed fine on another.
Hi Simon
Thanks for testing this.
Could you dump the PHY registers with and without this patch:
$ mii-tool -vv eth0
Once difference is that phy_remove_link_mode() does
phydev->advertising = phydev->supported where as the old code does
not. I though phylib would do this anyway, it does at some point in
time, but i didn't check when. It could be you are actually
advertising 10, even if you don't support it.
Thanks
Andrew
^ permalink raw reply
* [PATCH net-next 01/15] s390/qeth: move L2 xmit code to core module
From: Julian Wiedmann @ 2018-09-17 15:35 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180917153609.94628-1-jwi@linux.ibm.com>
We need the exact same transmit path for non-offload-eligible traffic on
L3 OSAs. So make it accessible from both sub-drivers.
Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
drivers/s390/net/qeth_core.h | 5 +++
drivers/s390/net/qeth_core_main.c | 59 +++++++++++++++++++++++++++++++
drivers/s390/net/qeth_l2_main.c | 74 ++++++---------------------------------
3 files changed, 75 insertions(+), 63 deletions(-)
diff --git a/drivers/s390/net/qeth_core.h b/drivers/s390/net/qeth_core.h
index 34e0d476c5c6..2110fabdcc7a 100644
--- a/drivers/s390/net/qeth_core.h
+++ b/drivers/s390/net/qeth_core.h
@@ -1052,6 +1052,11 @@ int qeth_vm_request_mac(struct qeth_card *card);
int qeth_add_hw_header(struct qeth_card *card, struct sk_buff *skb,
struct qeth_hdr **hdr, unsigned int hdr_len,
unsigned int proto_len, unsigned int *elements);
+int qeth_xmit(struct qeth_card *card, struct sk_buff *skb,
+ struct qeth_qdio_out_q *queue, int ipv, int cast_type,
+ void (*fill_header)(struct qeth_card *card, struct qeth_hdr *hdr,
+ struct sk_buff *skb, int ipv, int cast_type,
+ unsigned int data_len));
/* exports for OSN */
int qeth_osn_assist(struct net_device *, void *, int);
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index de8282420f96..d2ca33a9330a 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -4176,6 +4176,65 @@ int qeth_do_send_packet(struct qeth_card *card, struct qeth_qdio_out_q *queue,
}
EXPORT_SYMBOL_GPL(qeth_do_send_packet);
+int qeth_xmit(struct qeth_card *card, struct sk_buff *skb,
+ struct qeth_qdio_out_q *queue, int ipv, int cast_type,
+ void (*fill_header)(struct qeth_card *card, struct qeth_hdr *hdr,
+ struct sk_buff *skb, int ipv, int cast_type,
+ unsigned int data_len))
+{
+ const unsigned int proto_len = IS_IQD(card) ? ETH_HLEN : 0;
+ const unsigned int hw_hdr_len = sizeof(struct qeth_hdr);
+ unsigned int frame_len = skb->len;
+ unsigned int data_offset = 0;
+ struct qeth_hdr *hdr = NULL;
+ unsigned int hd_len = 0;
+ unsigned int elements;
+ int push_len, rc;
+ bool is_sg;
+
+ rc = skb_cow_head(skb, hw_hdr_len);
+ if (rc)
+ return rc;
+
+ push_len = qeth_add_hw_header(card, skb, &hdr, hw_hdr_len, proto_len,
+ &elements);
+ if (push_len < 0)
+ return push_len;
+ if (!push_len) {
+ /* HW header needs its own buffer element. */
+ hd_len = hw_hdr_len + proto_len;
+ data_offset = proto_len;
+ }
+ fill_header(card, hdr, skb, ipv, cast_type, frame_len);
+
+ is_sg = skb_is_nonlinear(skb);
+ if (IS_IQD(card)) {
+ rc = qeth_do_send_packet_fast(queue, skb, hdr, data_offset,
+ hd_len);
+ } else {
+ /* TODO: drop skb_orphan() once TX completion is fast enough */
+ skb_orphan(skb);
+ rc = qeth_do_send_packet(card, queue, skb, hdr, data_offset,
+ hd_len, elements);
+ }
+
+ if (!rc) {
+ if (card->options.performance_stats) {
+ card->perf_stats.buf_elements_sent += elements;
+ if (is_sg)
+ card->perf_stats.sg_skbs_sent++;
+ }
+ } else {
+ if (!push_len)
+ kmem_cache_free(qeth_core_header_cache, hdr);
+ if (rc == -EBUSY)
+ /* roll back to ETH header */
+ skb_pull(skb, push_len);
+ }
+ return rc;
+}
+EXPORT_SYMBOL_GPL(qeth_xmit);
+
static int qeth_setadp_promisc_mode_cb(struct qeth_card *card,
struct qeth_reply *reply, unsigned long data)
{
diff --git a/drivers/s390/net/qeth_l2_main.c b/drivers/s390/net/qeth_l2_main.c
index b5e38531733f..715d58af5fc4 100644
--- a/drivers/s390/net/qeth_l2_main.c
+++ b/drivers/s390/net/qeth_l2_main.c
@@ -193,8 +193,9 @@ static int qeth_l2_get_cast_type(struct qeth_card *card, struct sk_buff *skb)
return RTN_UNICAST;
}
-static void qeth_l2_fill_header(struct qeth_hdr *hdr, struct sk_buff *skb,
- int cast_type, unsigned int data_len)
+static void qeth_l2_fill_header(struct qeth_card *card, struct qeth_hdr *hdr,
+ struct sk_buff *skb, int ipv, int cast_type,
+ unsigned int data_len)
{
struct vlan_ethhdr *veth = (struct vlan_ethhdr *)skb_mac_header(skb);
@@ -202,6 +203,12 @@ static void qeth_l2_fill_header(struct qeth_hdr *hdr, struct sk_buff *skb,
hdr->hdr.l2.id = QETH_HEADER_TYPE_LAYER2;
hdr->hdr.l2.pkt_length = data_len;
+ if (skb->ip_summed == CHECKSUM_PARTIAL) {
+ qeth_tx_csum(skb, &hdr->hdr.l2.flags[1], ipv);
+ if (card->options.performance_stats)
+ card->perf_stats.tx_csum++;
+ }
+
/* set byte byte 3 to casting flags */
if (cast_type == RTN_MULTICAST)
hdr->hdr.l2.flags[2] |= QETH_LAYER2_FLAG_MULTICAST;
@@ -641,66 +648,6 @@ static void qeth_l2_set_rx_mode(struct net_device *dev)
qeth_promisc_to_bridge(card);
}
-static int qeth_l2_xmit(struct qeth_card *card, struct sk_buff *skb,
- struct qeth_qdio_out_q *queue, int cast_type, int ipv)
-{
- const unsigned int proto_len = IS_IQD(card) ? ETH_HLEN : 0;
- const unsigned int hw_hdr_len = sizeof(struct qeth_hdr);
- unsigned int frame_len = skb->len;
- unsigned int data_offset = 0;
- struct qeth_hdr *hdr = NULL;
- unsigned int hd_len = 0;
- unsigned int elements;
- int push_len, rc;
- bool is_sg;
-
- rc = skb_cow_head(skb, hw_hdr_len);
- if (rc)
- return rc;
-
- push_len = qeth_add_hw_header(card, skb, &hdr, hw_hdr_len, proto_len,
- &elements);
- if (push_len < 0)
- return push_len;
- if (!push_len) {
- /* HW header needs its own buffer element. */
- hd_len = hw_hdr_len + proto_len;
- data_offset = proto_len;
- }
- qeth_l2_fill_header(hdr, skb, cast_type, frame_len);
- if (skb->ip_summed == CHECKSUM_PARTIAL) {
- qeth_tx_csum(skb, &hdr->hdr.l2.flags[1], ipv);
- if (card->options.performance_stats)
- card->perf_stats.tx_csum++;
- }
-
- is_sg = skb_is_nonlinear(skb);
- if (IS_IQD(card)) {
- rc = qeth_do_send_packet_fast(queue, skb, hdr, data_offset,
- hd_len);
- } else {
- /* TODO: drop skb_orphan() once TX completion is fast enough */
- skb_orphan(skb);
- rc = qeth_do_send_packet(card, queue, skb, hdr, data_offset,
- hd_len, elements);
- }
-
- if (!rc) {
- if (card->options.performance_stats) {
- card->perf_stats.buf_elements_sent += elements;
- if (is_sg)
- card->perf_stats.sg_skbs_sent++;
- }
- } else {
- if (!push_len)
- kmem_cache_free(qeth_core_header_cache, hdr);
- if (rc == -EBUSY)
- /* roll back to ETH header */
- skb_pull(skb, push_len);
- }
- return rc;
-}
-
static int qeth_l2_xmit_osn(struct qeth_card *card, struct sk_buff *skb,
struct qeth_qdio_out_q *queue)
{
@@ -745,7 +692,8 @@ static netdev_tx_t qeth_l2_hard_start_xmit(struct sk_buff *skb,
if (IS_OSN(card))
rc = qeth_l2_xmit_osn(card, skb, queue);
else
- rc = qeth_l2_xmit(card, skb, queue, cast_type, ipv);
+ rc = qeth_xmit(card, skb, queue, ipv, cast_type,
+ qeth_l2_fill_header);
if (!rc) {
card->stats.tx_packets++;
--
2.16.4
^ permalink raw reply related
* [PATCH net-next 07/15] s390/qeth: check size of required HW header cache object
From: Julian Wiedmann @ 2018-09-17 15:36 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
Stefan Raspl, Ursula Braun, Julian Wiedmann
In-Reply-To: <20180917153609.94628-1-jwi@linux.ibm.com>
When qeth_add_hw_header() falls back to the header cache, ensure that
the requested length doesn't exceed the object size.
For current usage this is a no-brainer, but TSO transmission will
introduce protocol headers of varying length.
Signed-off-by: Julian Wiedmann <jwi@linux.ibm.com>
---
drivers/s390/net/qeth_core_main.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/s390/net/qeth_core_main.c b/drivers/s390/net/qeth_core_main.c
index eaf01dc62e91..79ebe8a5687b 100644
--- a/drivers/s390/net/qeth_core_main.c
+++ b/drivers/s390/net/qeth_core_main.c
@@ -16,6 +16,7 @@
#include <linux/string.h>
#include <linux/errno.h>
#include <linux/kernel.h>
+#include <linux/log2.h>
#include <linux/ip.h>
#include <linux/tcp.h>
#include <linux/mii.h>
@@ -3844,6 +3845,8 @@ int qeth_hdr_chk_and_bounce(struct sk_buff *skb, struct qeth_hdr **hdr, int len)
}
EXPORT_SYMBOL_GPL(qeth_hdr_chk_and_bounce);
+#define QETH_HDR_CACHE_OBJ_SIZE (sizeof(struct qeth_hdr) + ETH_HLEN)
+
/**
* qeth_add_hw_header() - add a HW header to an skb.
* @skb: skb that the HW header should be added to.
@@ -3918,6 +3921,8 @@ int qeth_add_hw_header(struct qeth_card *card, struct sk_buff *skb,
return hdr_len;
}
/* fall back */
+ if (hdr_len + proto_len > QETH_HDR_CACHE_OBJ_SIZE)
+ return -E2BIG;
*hdr = kmem_cache_alloc(qeth_core_header_cache, GFP_ATOMIC);
if (!*hdr)
return -ENOMEM;
@@ -6661,8 +6666,10 @@ static int __init qeth_core_init(void)
rc = PTR_ERR_OR_ZERO(qeth_core_root_dev);
if (rc)
goto register_err;
- qeth_core_header_cache = kmem_cache_create("qeth_hdr",
- sizeof(struct qeth_hdr) + ETH_HLEN, 64, 0, NULL);
+ qeth_core_header_cache =
+ kmem_cache_create("qeth_hdr", QETH_HDR_CACHE_OBJ_SIZE,
+ roundup_pow_of_two(QETH_HDR_CACHE_OBJ_SIZE),
+ 0, NULL);
if (!qeth_core_header_cache) {
rc = -ENOMEM;
goto slab_err;
--
2.16.4
^ 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