* [PATCH] selftests: Update fib_nexthop_multiprefix to handle missing ping6
From: David Ahern @ 2019-09-17 17:30 UTC (permalink / raw)
To: davem; +Cc: netdev, David Ahern
From: David Ahern <dsahern@gmail.com>
Some distributions (e.g., debian buster) do not install ping6. Re-use
the hook in pmtu.sh to detect this and fallback to ping.
Fixes: 735ab2f65dce ("selftests: Add test with multiple prefixes using single nexthop")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
tools/testing/selftests/net/fib_nexthop_multiprefix.sh | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/net/fib_nexthop_multiprefix.sh b/tools/testing/selftests/net/fib_nexthop_multiprefix.sh
index e6828732843e..9dc35a16e415 100755
--- a/tools/testing/selftests/net/fib_nexthop_multiprefix.sh
+++ b/tools/testing/selftests/net/fib_nexthop_multiprefix.sh
@@ -15,6 +15,8 @@
PAUSE_ON_FAIL=no
VERBOSE=0
+which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
+
################################################################################
# helpers
@@ -200,7 +202,7 @@ validate_v6_exception()
local rc
if [ ${ping_sz} != "0" ]; then
- run_cmd ip netns exec h0 ping6 -s ${ping_sz} -c5 -w5 ${dst}
+ run_cmd ip netns exec h0 ${ping6} -s ${ping_sz} -c5 -w5 ${dst}
fi
if [ "$VERBOSE" = "1" ]; then
@@ -243,7 +245,7 @@ do
run_cmd taskset -c ${c} ip netns exec h0 ping -c1 -w1 172.16.10${i}.1
[ $? -ne 0 ] && printf "\nERROR: ping to h${i} failed\n" && ret=1
- run_cmd taskset -c ${c} ip netns exec h0 ping6 -c1 -w1 2001:db8:10${i}::1
+ run_cmd taskset -c ${c} ip netns exec h0 ${ping6} -c1 -w1 2001:db8:10${i}::1
[ $? -ne 0 ] && printf "\nERROR: ping6 to h${i} failed\n" && ret=1
[ $ret -ne 0 ] && break
--
2.11.0
^ permalink raw reply related
* [PATCH net] ipv4: Revert removal of rt_uses_gateway
From: David Ahern @ 2019-09-17 17:39 UTC (permalink / raw)
To: davem; +Cc: netdev, ja, David Ahern
From: David Ahern <dsahern@gmail.com>
Julian noted that rt_uses_gateway has a more subtle use than 'is gateway
set':
https://lore.kernel.org/netdev/alpine.LFD.2.21.1909151104060.2546@ja.home.ssi.bg/
Revert that part of the commit referenced in the Fixes tag.
Currently, there are no u8 holes in 'struct rtable'. There is a 4-byte hole
in the second cacheline which contains the gateway declaration. So move
rt_gw_family down to the gateway declarations since they are always used
together, and then re-use that u8 for rt_uses_gateway. End result is that
rtable size is unchanged.
Fixes: 1550c171935d ("ipv4: Prepare rtable for IPv6 gateway")
Reported-by: Julian Anastasov <ja@ssi.bg>
Signed-off-by: David Ahern <dsahern@gmail.com>
---
drivers/infiniband/core/addr.c | 2 +-
include/net/route.h | 3 ++-
net/ipv4/inet_connection_sock.c | 4 ++--
net/ipv4/ip_forward.c | 2 +-
net/ipv4/ip_output.c | 2 +-
net/ipv4/route.c | 36 +++++++++++++++++++++---------------
net/ipv4/xfrm4_policy.c | 1 +
7 files changed, 29 insertions(+), 21 deletions(-)
diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
index 9b76a8fcdd24..bf539c34ccd3 100644
--- a/drivers/infiniband/core/addr.c
+++ b/drivers/infiniband/core/addr.c
@@ -352,7 +352,7 @@ static bool has_gateway(const struct dst_entry *dst, sa_family_t family)
if (family == AF_INET) {
rt = container_of(dst, struct rtable, dst);
- return rt->rt_gw_family == AF_INET;
+ return rt->rt_uses_gateway;
}
rt6 = container_of(dst, struct rt6_info, dst);
diff --git a/include/net/route.h b/include/net/route.h
index dfce19c9fa96..6c516840380d 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -53,10 +53,11 @@ struct rtable {
unsigned int rt_flags;
__u16 rt_type;
__u8 rt_is_input;
- u8 rt_gw_family;
+ __u8 rt_uses_gateway;
int rt_iif;
+ u8 rt_gw_family;
/* Info on neighbour */
union {
__be32 rt_gw4;
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index f5c163d4771b..a9183543ca30 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -560,7 +560,7 @@ struct dst_entry *inet_csk_route_req(const struct sock *sk,
rt = ip_route_output_flow(net, fl4, sk);
if (IS_ERR(rt))
goto no_route;
- if (opt && opt->opt.is_strictroute && rt->rt_gw_family)
+ if (opt && opt->opt.is_strictroute && rt->rt_uses_gateway)
goto route_err;
rcu_read_unlock();
return &rt->dst;
@@ -598,7 +598,7 @@ struct dst_entry *inet_csk_route_child_sock(const struct sock *sk,
rt = ip_route_output_flow(net, fl4, sk);
if (IS_ERR(rt))
goto no_route;
- if (opt && opt->opt.is_strictroute && rt->rt_gw_family)
+ if (opt && opt->opt.is_strictroute && rt->rt_uses_gateway)
goto route_err;
return &rt->dst;
diff --git a/net/ipv4/ip_forward.c b/net/ipv4/ip_forward.c
index 06f6f280b9ff..00ec819f949b 100644
--- a/net/ipv4/ip_forward.c
+++ b/net/ipv4/ip_forward.c
@@ -123,7 +123,7 @@ int ip_forward(struct sk_buff *skb)
rt = skb_rtable(skb);
- if (opt->is_strictroute && rt->rt_gw_family)
+ if (opt->is_strictroute && rt->rt_uses_gateway)
goto sr_failed;
IPCB(skb)->flags |= IPSKB_FORWARDED;
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index cc7ef0d05bbd..da521790cd63 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -499,7 +499,7 @@ int __ip_queue_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl,
skb_dst_set_noref(skb, &rt->dst);
packet_routed:
- if (inet_opt && inet_opt->opt.is_strictroute && rt->rt_gw_family)
+ if (inet_opt && inet_opt->opt.is_strictroute && rt->rt_uses_gateway)
goto no_route;
/* OK, we know where to send it, allocate and build IP header. */
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index b6a6f18c3dd1..7dcce724c78b 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -635,6 +635,7 @@ static void fill_route_from_fnhe(struct rtable *rt, struct fib_nh_exception *fnh
if (fnhe->fnhe_gw) {
rt->rt_flags |= RTCF_REDIRECTED;
+ rt->rt_uses_gateway = 1;
rt->rt_gw_family = AF_INET;
rt->rt_gw4 = fnhe->fnhe_gw;
}
@@ -1313,7 +1314,7 @@ static unsigned int ipv4_mtu(const struct dst_entry *dst)
mtu = READ_ONCE(dst->dev->mtu);
if (unlikely(ip_mtu_locked(dst))) {
- if (rt->rt_gw_family && mtu > 576)
+ if (rt->rt_uses_gateway && mtu > 576)
mtu = 576;
}
@@ -1569,6 +1570,7 @@ static void rt_set_nexthop(struct rtable *rt, __be32 daddr,
struct fib_nh_common *nhc = FIB_RES_NHC(*res);
if (nhc->nhc_gw_family && nhc->nhc_scope == RT_SCOPE_LINK) {
+ rt->rt_uses_gateway = 1;
rt->rt_gw_family = nhc->nhc_gw_family;
/* only INET and INET6 are supported */
if (likely(nhc->nhc_gw_family == AF_INET))
@@ -1634,6 +1636,7 @@ struct rtable *rt_dst_alloc(struct net_device *dev,
rt->rt_iif = 0;
rt->rt_pmtu = 0;
rt->rt_mtu_locked = 0;
+ rt->rt_uses_gateway = 0;
rt->rt_gw_family = 0;
rt->rt_gw4 = 0;
INIT_LIST_HEAD(&rt->rt_uncached);
@@ -2694,6 +2697,7 @@ struct dst_entry *ipv4_blackhole_route(struct net *net, struct dst_entry *dst_or
rt->rt_genid = rt_genid_ipv4(net);
rt->rt_flags = ort->rt_flags;
rt->rt_type = ort->rt_type;
+ rt->rt_uses_gateway = ort->rt_uses_gateway;
rt->rt_gw_family = ort->rt_gw_family;
if (rt->rt_gw_family == AF_INET)
rt->rt_gw4 = ort->rt_gw4;
@@ -2778,21 +2782,23 @@ static int rt_fill_info(struct net *net, __be32 dst, __be32 src,
if (nla_put_in_addr(skb, RTA_PREFSRC, fl4->saddr))
goto nla_put_failure;
}
- if (rt->rt_gw_family == AF_INET &&
- nla_put_in_addr(skb, RTA_GATEWAY, rt->rt_gw4)) {
- goto nla_put_failure;
- } else if (rt->rt_gw_family == AF_INET6) {
- int alen = sizeof(struct in6_addr);
- struct nlattr *nla;
- struct rtvia *via;
-
- nla = nla_reserve(skb, RTA_VIA, alen + 2);
- if (!nla)
+ if (rt->rt_uses_gateway) {
+ if (rt->rt_gw_family == AF_INET &&
+ nla_put_in_addr(skb, RTA_GATEWAY, rt->rt_gw4)) {
goto nla_put_failure;
-
- via = nla_data(nla);
- via->rtvia_family = AF_INET6;
- memcpy(via->rtvia_addr, &rt->rt_gw6, alen);
+ } else if (rt->rt_gw_family == AF_INET6) {
+ int alen = sizeof(struct in6_addr);
+ struct nlattr *nla;
+ struct rtvia *via;
+
+ nla = nla_reserve(skb, RTA_VIA, alen + 2);
+ if (!nla)
+ goto nla_put_failure;
+
+ via = nla_data(nla);
+ via->rtvia_family = AF_INET6;
+ memcpy(via->rtvia_addr, &rt->rt_gw6, alen);
+ }
}
expires = rt->dst.expires;
diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
index cdef8f9a3b01..35b84b52b702 100644
--- a/net/ipv4/xfrm4_policy.c
+++ b/net/ipv4/xfrm4_policy.c
@@ -85,6 +85,7 @@ static int xfrm4_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
xdst->u.rt.rt_flags = rt->rt_flags & (RTCF_BROADCAST | RTCF_MULTICAST |
RTCF_LOCAL);
xdst->u.rt.rt_type = rt->rt_type;
+ xdst->u.rt.rt_uses_gateway = rt->rt_uses_gateway;
xdst->u.rt.rt_gw_family = rt->rt_gw_family;
if (rt->rt_gw_family == AF_INET)
xdst->u.rt.rt_gw4 = rt->rt_gw4;
--
2.11.0
^ permalink raw reply related
* [PATCH bpf 0/2] bpf: BTF fixes
From: Alexei Starovoitov @ 2019-09-17 17:45 UTC (permalink / raw)
To: davem; +Cc: daniel, netdev, bpf, kernel-team
Two trivial BTF fixes.
Alexei Starovoitov (2):
bpf: fix BTF verification of enums
bpf: fix BTF limits
include/uapi/linux/btf.h | 4 ++--
kernel/bpf/btf.c | 5 ++---
2 files changed, 4 insertions(+), 5 deletions(-)
--
2.20.0
^ permalink raw reply
* [PATCH bpf 1/2] bpf: fix BTF verification of enums
From: Alexei Starovoitov @ 2019-09-17 17:45 UTC (permalink / raw)
To: davem; +Cc: daniel, netdev, bpf, kernel-team
In-Reply-To: <20190917174538.1295523-1-ast@kernel.org>
vmlinux BTF has enums that are 8 byte and 1 byte in size.
2 byte enum is a valid construct as well.
Fix BTF enum verification to accept those sizes.
Fixes: 69b693f0aefa ("bpf: btf: Introduce BPF Type Format (BTF)")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
kernel/bpf/btf.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index adb3adcebe3c..722d38e543e9 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -2377,9 +2377,8 @@ static s32 btf_enum_check_meta(struct btf_verifier_env *env,
return -EINVAL;
}
- if (t->size != sizeof(int)) {
- btf_verifier_log_type(env, t, "Expected size:%zu",
- sizeof(int));
+ if (t->size > 8 || !is_power_of_2(t->size)) {
+ btf_verifier_log_type(env, t, "Unexpected size");
return -EINVAL;
}
--
2.20.0
^ permalink raw reply related
* [PATCH bpf 2/2] bpf: fix BTF limits
From: Alexei Starovoitov @ 2019-09-17 17:45 UTC (permalink / raw)
To: davem; +Cc: daniel, netdev, bpf, kernel-team
In-Reply-To: <20190917174538.1295523-1-ast@kernel.org>
vmlinux BTF has more than 64k types.
Its string section is also at the offset larger than 64k.
Adjust both limits to make in-kernel BTF verifier successfully parse in-kernel BTF.
Fixes: 69b693f0aefa ("bpf: btf: Introduce BPF Type Format (BTF)")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Acked-by: Martin KaFai Lau <kafai@fb.com>
---
include/uapi/linux/btf.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/btf.h b/include/uapi/linux/btf.h
index 63ae4a39e58b..c02dec97e1ce 100644
--- a/include/uapi/linux/btf.h
+++ b/include/uapi/linux/btf.h
@@ -22,9 +22,9 @@ struct btf_header {
};
/* Max # of type identifier */
-#define BTF_MAX_TYPE 0x0000ffff
+#define BTF_MAX_TYPE 0x000fffff
/* Max offset into the string section */
-#define BTF_MAX_NAME_OFFSET 0x0000ffff
+#define BTF_MAX_NAME_OFFSET 0x00ffffff
/* Max # of struct/union/enum members or func args */
#define BTF_MAX_VLEN 0xffff
--
2.20.0
^ permalink raw reply related
* Re: [RESENT PATCH v2] ixgbe: Use memzero_explicit directly in crypto cases
From: Jakub Kicinski @ 2019-09-17 18:11 UTC (permalink / raw)
To: zhong jiang; +Cc: davem, anna.schumaker, trond.myklebust, netdev, linux-kernel
In-Reply-To: <1568731462-46758-1-git-send-email-zhongjiang@huawei.com>
On Tue, 17 Sep 2019 22:44:22 +0800, zhong jiang wrote:
> It's better to use memzero_explicit() to replace memset() in crypto cases.
>
> Signed-off-by: zhong jiang <zhongjiang@huawei.com>
Thank you for the follow up! Your previous patch to use kzfree()
has been applied on its own merit, could you rebase this one on top
of current net-next/master?
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> index 31629fc..7e4f32f 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ipsec.c
> @@ -960,10 +960,10 @@ int ixgbe_ipsec_vf_add_sa(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
> return 0;
>
> err_aead:
> - memset(xs->aead, 0, sizeof(*xs->aead));
> + memzero_explicit(xs->aead, sizeof(*xs->aead));
> kfree(xs->aead);
> err_xs:
> - memset(xs, 0, sizeof(*xs));
> + memzero_explicit(xs, sizeof(*xs));
> kfree(xs);
> err_out:
> msgbuf[1] = err;
> @@ -1049,7 +1049,7 @@ int ixgbe_ipsec_vf_del_sa(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
> ixgbe_ipsec_del_sa(xs);
>
> /* remove the xs that was made-up in the add request */
> - memset(xs, 0, sizeof(*xs));
> + memzero_explicit(xs, sizeof(*xs));
> kfree(xs);
>
> return 0;
^ permalink raw reply
* Re: [PATCH v2] tcp: Add TCP_INFO counter for packets received out-of-order
From: Neal Cardwell @ 2019-09-17 18:23 UTC (permalink / raw)
To: Eric Dumazet
Cc: Jason Baron, Thomas Higdon, netdev, Jonathan Lemon, Dave Jones,
Yuchung Cheng
In-Reply-To: <CANn89iKJdMGD6kdKrQJk_sHO4ec=Ko3iAiBy_oDzU2UPGtvJNg@mail.gmail.com>
On Tue, Sep 17, 2019 at 1:22 PM Eric Dumazet <edumazet@google.com> wrote:
>
> Tue, Sep 17, 2019 at 10:13 AM Jason Baron <jbaron@akamai.com> wrote:
> >
> >
> > Hi,
> >
> > I was interested in adding a field to tcp_info around the TFO state of a
> > socket. So for the server side it would indicate if TFO was used to
> > create the socket and on the client side it would report whether TFO
> > worked and if not that it failed with maybe some additional states
> > around why it failed. I'm thinking it would be maybe 3 bits.
BTW, one aspect of that "did TFO work" info is available already in
tcp_info in the tcpi_options field.
Kernel side is:
if (tp->syn_data_acked)
info->tcpi_options |= TCPI_OPT_SYN_DATA;
We use this bit in packetdrill tests on client and server side to
check that the TFO data-in-SYN succeeded:
+0 %{ assert (tcpi_options & TCPI_OPT_SYN_DATA) != 0, tcpi_options }%
These TFO bits were added much later than the other bits, so IMHO it
would be OK to add more bits somewhere unused in tcp_info to indicate
reasons for TFO failure. Especially if, as you suggest, "0" as a code
point could indicate that the code point is undefined, and all
meaningful code points were non-zero.
neal
> > My question is whether its reasonable to use the unused bits of
> > __u8 tcpi_delivery_rate_app_limited:1;. Or is this not good because
> > the size hasn't changed? What if I avoided using 0 for the new field to
> > avoid the possibility of not knowing if 0 because its the old kernel or
> > 0 because that's now its a TFO state? IE the new field could always be >
> > 0 for the new kernel.
> >
>
> I guess that storing the 'why it has failed' would need more bits.
>
> I suggest maybe using an event for this, instead of TCP_INFO ?
>
> As of using the bits, maybe the monitoring application does not really care
> if running on an old kernel where the bits would be zero.
>
> Commit eb8329e0a04db0061f714f033b4454326ba147f4 reserved a single
> bit and did not bother about making sure the monitoring would detect if this
> runs on an old kernel.
^ permalink raw reply
* Re: [PATCH net-next] net: ethernet: ti: use devm_platform_ioremap_resource() to simplify code
From: Geert Uytterhoeven @ 2019-09-17 18:35 UTC (permalink / raw)
To: YueHaibing
Cc: David S. Miller, Grygorii Strashko, ivan.khoronzhuk, Andrew Lunn,
Petr Štetiar, Linux Kernel Mailing List, netdev,
open list:TI ETHERNET SWITCH DRIVER (CPSW)
In-Reply-To: <20190821124850.9592-1-yuehaibing@huawei.com>
Hi YueHaibing,
On Wed, Aug 21, 2019 at 2:51 PM YueHaibing <yuehaibing@huawei.com> wrote:
> Use devm_platform_ioremap_resource() to simplify the code a bit.
> This is detected by coccinelle.
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
> ---
> drivers/net/ethernet/ti/cpsw.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/ti/cpsw.c b/drivers/net/ethernet/ti/cpsw.c
> index 32a8974..5401095 100644
> --- a/drivers/net/ethernet/ti/cpsw.c
> +++ b/drivers/net/ethernet/ti/cpsw.c
> @@ -2764,7 +2764,7 @@ static int cpsw_probe(struct platform_device *pdev)
> struct net_device *ndev;
> struct cpsw_priv *priv;
> void __iomem *ss_regs;
> - struct resource *res, *ss_res;
> + struct resource *ss_res;
> struct gpio_descs *mode;
> const struct soc_device_attribute *soc;
> struct cpsw_common *cpsw;
> @@ -2798,8 +2798,7 @@ static int cpsw_probe(struct platform_device *pdev)
And just out-of-context, we also have:
ss_res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
ss_regs = devm_ioremap_resource(dev, ss_res);
if (IS_ERR(ss_regs))
which was not detected as being the same pattern?
Interesting...
> return PTR_ERR(ss_regs);
> cpsw->regs = ss_regs;
>
> - res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> - cpsw->wr_regs = devm_ioremap_resource(dev, res);
> + cpsw->wr_regs = devm_platform_ioremap_resource(pdev, 1);
> if (IS_ERR(cpsw->wr_regs))
> return PTR_ERR(cpsw->wr_regs);
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [patch iproute2-next v2] devlink: add reload failed indication
From: Jiri Pirko @ 2019-09-17 18:36 UTC (permalink / raw)
To: David Ahern; +Cc: netdev, stephen, idosch, jakub.kicinski, tariqt, mlxsw
In-Reply-To: <c9b57141-2caf-71c6-7590-a4783796e037@gmail.com>
Tue, Sep 17, 2019 at 06:46:31PM CEST, dsahern@gmail.com wrote:
>On 9/16/19 3:44 AM, Jiri Pirko wrote:
>> From: Jiri Pirko <jiri@mellanox.com>
>>
>> Add indication about previous failed devlink reload.
>>
>> Example outputs:
>>
>> $ devlink dev
>> netdevsim/netdevsim10: reload_failed true
>
>odd output to user. Why not just "reload failed"?
Well it is common to have "name value". The extra space would seem
confusing for the reader..
Also it is common to have "_" instead of space for the output in cases
like this.
^ permalink raw reply
* KASAN: slab-out-of-bounds Read in bpf_prog_create
From: syzbot @ 2019-09-17 18:49 UTC (permalink / raw)
To: arnd, ast, bpf, daniel, davem, hawk, jakub.kicinski,
john.fastabend, kafai, linux-fsdevel, linux-kernel, linux-ppp,
netdev, paulus, songliubraving, syzkaller-bugs, viro, yhs
Hello,
syzbot found the following crash on:
HEAD commit: 2015a28f Add linux-next specific files for 20190915
git tree: linux-next
console output: https://syzkaller.appspot.com/x/log.txt?x=11880d69600000
kernel config: https://syzkaller.appspot.com/x/.config?x=110691c2286b679a
dashboard link: https://syzkaller.appspot.com/bug?extid=eb853b51b10f1befa0b7
compiler: gcc (GCC) 9.0.0 20181231 (experimental)
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=127c3481600000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1150a70d600000
The bug was bisected to:
commit 2f4fa2db75e26995709043c8d3de4632ebed5c4b
Author: Al Viro <viro@zeniv.linux.org.uk>
Date: Thu Apr 18 03:48:01 2019 +0000
compat_ioctl: unify copy-in of ppp filters
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=145eee1d600000
final crash: https://syzkaller.appspot.com/x/report.txt?x=165eee1d600000
console output: https://syzkaller.appspot.com/x/log.txt?x=125eee1d600000
IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+eb853b51b10f1befa0b7@syzkaller.appspotmail.com
Fixes: 2f4fa2db75e2 ("compat_ioctl: unify copy-in of ppp filters")
==================================================================
BUG: KASAN: slab-out-of-bounds in memcpy include/linux/string.h:404 [inline]
BUG: KASAN: slab-out-of-bounds in bpf_prog_create+0xe9/0x250
net/core/filter.c:1351
Read of size 32768 at addr ffff88809cf74000 by task syz-executor183/8575
CPU: 0 PID: 8575 Comm: syz-executor183 Not tainted 5.3.0-rc8-next-20190915
#0
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+0x172/0x1f0 lib/dump_stack.c:113
print_address_description.constprop.0.cold+0xd4/0x30b mm/kasan/report.c:374
__kasan_report.cold+0x1b/0x41 mm/kasan/report.c:506
kasan_report+0x12/0x20 mm/kasan/common.c:634
check_memory_region_inline mm/kasan/generic.c:185 [inline]
check_memory_region+0x134/0x1a0 mm/kasan/generic.c:192
memcpy+0x24/0x50 mm/kasan/common.c:122
memcpy include/linux/string.h:404 [inline]
bpf_prog_create+0xe9/0x250 net/core/filter.c:1351
get_filter.isra.0+0x108/0x1a0 drivers/net/ppp/ppp_generic.c:572
ppp_get_filter drivers/net/ppp/ppp_generic.c:584 [inline]
ppp_ioctl+0x129d/0x2590 drivers/net/ppp/ppp_generic.c:801
vfs_ioctl fs/ioctl.c:47 [inline]
file_ioctl fs/ioctl.c:539 [inline]
do_vfs_ioctl+0xdb6/0x13e0 fs/ioctl.c:726
ksys_ioctl+0xab/0xd0 fs/ioctl.c:743
__do_sys_ioctl fs/ioctl.c:750 [inline]
__se_sys_ioctl fs/ioctl.c:748 [inline]
__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:748
do_syscall_64+0xfa/0x760 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x4401a9
Code: 18 89 d0 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 fb 13 fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffebb37d0a8 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00000000004002c8 RCX: 00000000004401a9
RDX: 00000000200000c0 RSI: 0000000040107447 RDI: 0000000000000003
RBP: 00000000006ca018 R08: 00000000004002c8 R09: 00000000004002c8
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000401a30
R13: 0000000000401ac0 R14: 0000000000000000 R15: 0000000000000000
Allocated by task 8575:
save_stack+0x23/0x90 mm/kasan/common.c:69
set_track mm/kasan/common.c:77 [inline]
__kasan_kmalloc mm/kasan/common.c:510 [inline]
__kasan_kmalloc.constprop.0+0xcf/0xe0 mm/kasan/common.c:483
kasan_kmalloc+0x9/0x10 mm/kasan/common.c:524
__do_kmalloc mm/slab.c:3655 [inline]
__kmalloc_track_caller+0x15f/0x760 mm/slab.c:3670
memdup_user+0x26/0xb0 mm/util.c:172
get_filter.isra.0+0xd7/0x1a0 drivers/net/ppp/ppp_generic.c:568
ppp_get_filter drivers/net/ppp/ppp_generic.c:584 [inline]
ppp_ioctl+0x129d/0x2590 drivers/net/ppp/ppp_generic.c:801
vfs_ioctl fs/ioctl.c:47 [inline]
file_ioctl fs/ioctl.c:539 [inline]
do_vfs_ioctl+0xdb6/0x13e0 fs/ioctl.c:726
ksys_ioctl+0xab/0xd0 fs/ioctl.c:743
__do_sys_ioctl fs/ioctl.c:750 [inline]
__se_sys_ioctl fs/ioctl.c:748 [inline]
__x64_sys_ioctl+0x73/0xb0 fs/ioctl.c:748
do_syscall_64+0xfa/0x760 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
Freed by task 0:
(stack is not available)
The buggy address belongs to the object at ffff88809cf74000
which belongs to the cache kmalloc-4k of size 4096
The buggy address is located 0 bytes inside of
4096-byte region [ffff88809cf74000, ffff88809cf75000)
The buggy address belongs to the page:
page:ffffea000273dd00 refcount:1 mapcount:0 mapping:ffff8880aa402000
index:0x0 compound_mapcount: 0
flags: 0x1fffc0000010200(slab|head)
raw: 01fffc0000010200 ffffea0002672988 ffffea00027e7788 ffff8880aa402000
raw: 0000000000000000 ffff88809cf74000 0000000100000001 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff88809cf74f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
ffff88809cf74f80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> ffff88809cf75000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
^
ffff88809cf75080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
ffff88809cf75100: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
==================================================================
---
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#status for how to communicate with syzbot.
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
syzbot can test patches for this bug, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH net] ipv4: Revert removal of rt_uses_gateway
From: Julian Anastasov @ 2019-09-17 18:50 UTC (permalink / raw)
To: David Ahern; +Cc: davem, netdev, David Ahern
In-Reply-To: <20190917173949.19982-1-dsahern@kernel.org>
Hello,
On Tue, 17 Sep 2019, David Ahern wrote:
> From: David Ahern <dsahern@gmail.com>
>
> Julian noted that rt_uses_gateway has a more subtle use than 'is gateway
> set':
> https://lore.kernel.org/netdev/alpine.LFD.2.21.1909151104060.2546@ja.home.ssi.bg/
>
> Revert that part of the commit referenced in the Fixes tag.
>
> Currently, there are no u8 holes in 'struct rtable'. There is a 4-byte hole
> in the second cacheline which contains the gateway declaration. So move
> rt_gw_family down to the gateway declarations since they are always used
> together, and then re-use that u8 for rt_uses_gateway. End result is that
> rtable size is unchanged.
>
> Fixes: 1550c171935d ("ipv4: Prepare rtable for IPv6 gateway")
> Reported-by: Julian Anastasov <ja@ssi.bg>
> Signed-off-by: David Ahern <dsahern@gmail.com>
Looks good to me, thanks!
Reviewed-by: Julian Anastasov <ja@ssi.bg>
> ---
> drivers/infiniband/core/addr.c | 2 +-
> include/net/route.h | 3 ++-
> net/ipv4/inet_connection_sock.c | 4 ++--
> net/ipv4/ip_forward.c | 2 +-
> net/ipv4/ip_output.c | 2 +-
> net/ipv4/route.c | 36 +++++++++++++++++++++---------------
> net/ipv4/xfrm4_policy.c | 1 +
> 7 files changed, 29 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/infiniband/core/addr.c b/drivers/infiniband/core/addr.c
> index 9b76a8fcdd24..bf539c34ccd3 100644
> --- a/drivers/infiniband/core/addr.c
> +++ b/drivers/infiniband/core/addr.c
> @@ -352,7 +352,7 @@ static bool has_gateway(const struct dst_entry *dst, sa_family_t family)
>
> if (family == AF_INET) {
> rt = container_of(dst, struct rtable, dst);
> - return rt->rt_gw_family == AF_INET;
> + return rt->rt_uses_gateway;
> }
>
> rt6 = container_of(dst, struct rt6_info, dst);
> diff --git a/include/net/route.h b/include/net/route.h
> index dfce19c9fa96..6c516840380d 100644
> --- a/include/net/route.h
> +++ b/include/net/route.h
> @@ -53,10 +53,11 @@ struct rtable {
> unsigned int rt_flags;
> __u16 rt_type;
> __u8 rt_is_input;
> - u8 rt_gw_family;
> + __u8 rt_uses_gateway;
>
> int rt_iif;
>
> + u8 rt_gw_family;
> /* Info on neighbour */
> union {
> __be32 rt_gw4;
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index f5c163d4771b..a9183543ca30 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -560,7 +560,7 @@ struct dst_entry *inet_csk_route_req(const struct sock *sk,
> rt = ip_route_output_flow(net, fl4, sk);
> if (IS_ERR(rt))
> goto no_route;
> - if (opt && opt->opt.is_strictroute && rt->rt_gw_family)
> + if (opt && opt->opt.is_strictroute && rt->rt_uses_gateway)
> goto route_err;
> rcu_read_unlock();
> return &rt->dst;
> @@ -598,7 +598,7 @@ struct dst_entry *inet_csk_route_child_sock(const struct sock *sk,
> rt = ip_route_output_flow(net, fl4, sk);
> if (IS_ERR(rt))
> goto no_route;
> - if (opt && opt->opt.is_strictroute && rt->rt_gw_family)
> + if (opt && opt->opt.is_strictroute && rt->rt_uses_gateway)
> goto route_err;
> return &rt->dst;
>
> diff --git a/net/ipv4/ip_forward.c b/net/ipv4/ip_forward.c
> index 06f6f280b9ff..00ec819f949b 100644
> --- a/net/ipv4/ip_forward.c
> +++ b/net/ipv4/ip_forward.c
> @@ -123,7 +123,7 @@ int ip_forward(struct sk_buff *skb)
>
> rt = skb_rtable(skb);
>
> - if (opt->is_strictroute && rt->rt_gw_family)
> + if (opt->is_strictroute && rt->rt_uses_gateway)
> goto sr_failed;
>
> IPCB(skb)->flags |= IPSKB_FORWARDED;
> diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
> index cc7ef0d05bbd..da521790cd63 100644
> --- a/net/ipv4/ip_output.c
> +++ b/net/ipv4/ip_output.c
> @@ -499,7 +499,7 @@ int __ip_queue_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl,
> skb_dst_set_noref(skb, &rt->dst);
>
> packet_routed:
> - if (inet_opt && inet_opt->opt.is_strictroute && rt->rt_gw_family)
> + if (inet_opt && inet_opt->opt.is_strictroute && rt->rt_uses_gateway)
> goto no_route;
>
> /* OK, we know where to send it, allocate and build IP header. */
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index b6a6f18c3dd1..7dcce724c78b 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -635,6 +635,7 @@ static void fill_route_from_fnhe(struct rtable *rt, struct fib_nh_exception *fnh
>
> if (fnhe->fnhe_gw) {
> rt->rt_flags |= RTCF_REDIRECTED;
> + rt->rt_uses_gateway = 1;
> rt->rt_gw_family = AF_INET;
> rt->rt_gw4 = fnhe->fnhe_gw;
> }
> @@ -1313,7 +1314,7 @@ static unsigned int ipv4_mtu(const struct dst_entry *dst)
> mtu = READ_ONCE(dst->dev->mtu);
>
> if (unlikely(ip_mtu_locked(dst))) {
> - if (rt->rt_gw_family && mtu > 576)
> + if (rt->rt_uses_gateway && mtu > 576)
> mtu = 576;
> }
>
> @@ -1569,6 +1570,7 @@ static void rt_set_nexthop(struct rtable *rt, __be32 daddr,
> struct fib_nh_common *nhc = FIB_RES_NHC(*res);
>
> if (nhc->nhc_gw_family && nhc->nhc_scope == RT_SCOPE_LINK) {
> + rt->rt_uses_gateway = 1;
> rt->rt_gw_family = nhc->nhc_gw_family;
> /* only INET and INET6 are supported */
> if (likely(nhc->nhc_gw_family == AF_INET))
> @@ -1634,6 +1636,7 @@ struct rtable *rt_dst_alloc(struct net_device *dev,
> rt->rt_iif = 0;
> rt->rt_pmtu = 0;
> rt->rt_mtu_locked = 0;
> + rt->rt_uses_gateway = 0;
> rt->rt_gw_family = 0;
> rt->rt_gw4 = 0;
> INIT_LIST_HEAD(&rt->rt_uncached);
> @@ -2694,6 +2697,7 @@ struct dst_entry *ipv4_blackhole_route(struct net *net, struct dst_entry *dst_or
> rt->rt_genid = rt_genid_ipv4(net);
> rt->rt_flags = ort->rt_flags;
> rt->rt_type = ort->rt_type;
> + rt->rt_uses_gateway = ort->rt_uses_gateway;
> rt->rt_gw_family = ort->rt_gw_family;
> if (rt->rt_gw_family == AF_INET)
> rt->rt_gw4 = ort->rt_gw4;
> @@ -2778,21 +2782,23 @@ static int rt_fill_info(struct net *net, __be32 dst, __be32 src,
> if (nla_put_in_addr(skb, RTA_PREFSRC, fl4->saddr))
> goto nla_put_failure;
> }
> - if (rt->rt_gw_family == AF_INET &&
> - nla_put_in_addr(skb, RTA_GATEWAY, rt->rt_gw4)) {
> - goto nla_put_failure;
> - } else if (rt->rt_gw_family == AF_INET6) {
> - int alen = sizeof(struct in6_addr);
> - struct nlattr *nla;
> - struct rtvia *via;
> -
> - nla = nla_reserve(skb, RTA_VIA, alen + 2);
> - if (!nla)
> + if (rt->rt_uses_gateway) {
> + if (rt->rt_gw_family == AF_INET &&
> + nla_put_in_addr(skb, RTA_GATEWAY, rt->rt_gw4)) {
> goto nla_put_failure;
> -
> - via = nla_data(nla);
> - via->rtvia_family = AF_INET6;
> - memcpy(via->rtvia_addr, &rt->rt_gw6, alen);
> + } else if (rt->rt_gw_family == AF_INET6) {
> + int alen = sizeof(struct in6_addr);
> + struct nlattr *nla;
> + struct rtvia *via;
> +
> + nla = nla_reserve(skb, RTA_VIA, alen + 2);
> + if (!nla)
> + goto nla_put_failure;
> +
> + via = nla_data(nla);
> + via->rtvia_family = AF_INET6;
> + memcpy(via->rtvia_addr, &rt->rt_gw6, alen);
> + }
> }
>
> expires = rt->dst.expires;
> diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
> index cdef8f9a3b01..35b84b52b702 100644
> --- a/net/ipv4/xfrm4_policy.c
> +++ b/net/ipv4/xfrm4_policy.c
> @@ -85,6 +85,7 @@ static int xfrm4_fill_dst(struct xfrm_dst *xdst, struct net_device *dev,
> xdst->u.rt.rt_flags = rt->rt_flags & (RTCF_BROADCAST | RTCF_MULTICAST |
> RTCF_LOCAL);
> xdst->u.rt.rt_type = rt->rt_type;
> + xdst->u.rt.rt_uses_gateway = rt->rt_uses_gateway;
> xdst->u.rt.rt_gw_family = rt->rt_gw_family;
> if (rt->rt_gw_family == AF_INET)
> xdst->u.rt.rt_gw4 = rt->rt_gw4;
> --
> 2.11.0
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [PATCH net] ipv4: Revert removal of rt_uses_gateway
From: David Ahern @ 2019-09-17 18:55 UTC (permalink / raw)
To: Julian Anastasov, David Ahern; +Cc: davem, netdev
In-Reply-To: <alpine.LFD.2.21.1909172148220.2649@ja.home.ssi.bg>
On 9/17/19 12:50 PM, Julian Anastasov wrote:
>
> Looks good to me, thanks!
>
> Reviewed-by: Julian Anastasov <ja@ssi.bg>
>
BTW, do you have any tests for the rt_uses_gateway paths - showing why
it is needed?
All of the pmtu, redirect, fib tests, etc worked fine without the
special flag. Sure, the 'ip ro get' had extra data; it seems like that
could be handled.
^ permalink raw reply
* Re: [RFC PATCH 2/4] mdev: introduce helper to set per device dma ops
From: Alex Williamson @ 2019-09-17 19:00 UTC (permalink / raw)
To: Jason Wang
Cc: mst, kvm, virtualization, netdev, linux-kernel, kwankhede, cohuck,
tiwei.bie, maxime.coquelin, cunming.liang, zhihong.wang,
rob.miller, idos, xiao.w.wang, haotian.wang
In-Reply-To: <20190910081935.30516-3-jasowang@redhat.com>
On Tue, 10 Sep 2019 16:19:33 +0800
Jason Wang <jasowang@redhat.com> wrote:
> This patch introduces mdev_set_dma_ops() which allows parent to set
> per device DMA ops. This help for the kernel driver to setup a correct
> DMA mappings.
>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
> drivers/vfio/mdev/mdev_core.c | 7 +++++++
> include/linux/mdev.h | 2 ++
> 2 files changed, 9 insertions(+)
>
> diff --git a/drivers/vfio/mdev/mdev_core.c b/drivers/vfio/mdev/mdev_core.c
> index b558d4cfd082..eb28552082d7 100644
> --- a/drivers/vfio/mdev/mdev_core.c
> +++ b/drivers/vfio/mdev/mdev_core.c
> @@ -13,6 +13,7 @@
> #include <linux/uuid.h>
> #include <linux/sysfs.h>
> #include <linux/mdev.h>
> +#include <linux/dma-mapping.h>
>
> #include "mdev_private.h"
>
> @@ -27,6 +28,12 @@ static struct class_compat *mdev_bus_compat_class;
> static LIST_HEAD(mdev_list);
> static DEFINE_MUTEX(mdev_list_lock);
>
> +void mdev_set_dma_ops(struct mdev_device *mdev, struct dma_map_ops *ops)
> +{
> + set_dma_ops(&mdev->dev, ops);
> +}
> +EXPORT_SYMBOL(mdev_set_dma_ops);
> +
Why does mdev need to be involved here? Your sample driver in 4/4 calls
this from its create callback, where it could just as easily call:
set_dma_ops(mdev_dev(mdev), ops);
Thanks,
Alex
> struct device *mdev_parent_dev(struct mdev_device *mdev)
> {
> return mdev->parent->dev;
> diff --git a/include/linux/mdev.h b/include/linux/mdev.h
> index 0ce30ca78db0..7195f40bf8bf 100644
> --- a/include/linux/mdev.h
> +++ b/include/linux/mdev.h
> @@ -145,4 +145,6 @@ struct device *mdev_parent_dev(struct mdev_device *mdev);
> struct device *mdev_dev(struct mdev_device *mdev);
> struct mdev_device *mdev_from_dev(struct device *dev);
>
> +void mdev_set_dma_ops(struct mdev_device *mdev, struct dma_map_ops *ops);
> +
> #endif /* MDEV_H */
^ permalink raw reply
* Re: [PATCH v2] ftgmac100: Disable HW checksum generation on AST2500
From: Vijay Khemka @ 2019-09-17 19:34 UTC (permalink / raw)
To: David S. Miller, Florian Fainelli, YueHaibing, Andrew Lunn,
Kate Stewart, Mauro Carvalho Chehab, Luis Chamberlain,
Thomas Gleixner, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-aspeed@lists.ozlabs.org,
joel@jms.id.au
Cc: openbmc @ lists . ozlabs . org, Sai Dasari
In-Reply-To: <20190911194453.2595021-1-vijaykhemka@fb.com>
Please review below patch and provide your valuable feedback.
Regards
-Vijay
On 9/11/19, 1:05 PM, "Vijay Khemka" <vijaykhemka@fb.com> wrote:
HW checksum generation is not working for AST2500, specially with IPV6
over NCSI. All TCP packets with IPv6 get dropped. By disabling this
it works perfectly fine with IPV6. As it works for IPV4 so enabled
hw checksum back for IPV4.
Verified with IPV6 enabled and can do ssh.
Signed-off-by: Vijay Khemka <vijaykhemka@fb.com>
---
Changes since v1:
Enabled IPV4 hw checksum generation as it works for IPV4.
drivers/net/ethernet/faraday/ftgmac100.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/faraday/ftgmac100.c b/drivers/net/ethernet/faraday/ftgmac100.c
index 030fed65393e..0255a28d2958 100644
--- a/drivers/net/ethernet/faraday/ftgmac100.c
+++ b/drivers/net/ethernet/faraday/ftgmac100.c
@@ -1842,8 +1842,19 @@ static int ftgmac100_probe(struct platform_device *pdev)
/* AST2400 doesn't have working HW checksum generation */
if (np && (of_device_is_compatible(np, "aspeed,ast2400-mac")))
netdev->hw_features &= ~NETIF_F_HW_CSUM;
+
+ /* AST2500 doesn't have working HW checksum generation for IPV6
+ * but it works for IPV4, so disabling hw checksum and enabling
+ * it for only IPV4.
+ */
+ if (np && (of_device_is_compatible(np, "aspeed,ast2500-mac"))) {
+ netdev->hw_features &= ~NETIF_F_HW_CSUM;
+ netdev->hw_features |= NETIF_F_IP_CSUM;
+ }
+
if (np && of_get_property(np, "no-hw-checksum", NULL))
- netdev->hw_features &= ~(NETIF_F_HW_CSUM | NETIF_F_RXCSUM);
+ netdev->hw_features &= ~(NETIF_F_HW_CSUM | NETIF_F_RXCSUM
+ | NETIF_F_IP_CSUM);
netdev->features |= netdev->hw_features;
/* register network device */
--
2.17.1
^ permalink raw reply
* Re: [PATCH bpf 0/2] bpf: BTF fixes
From: Andrii Nakryiko @ 2019-09-17 19:35 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: David S. Miller, Daniel Borkmann, Networking, bpf, Kernel Team
In-Reply-To: <20190917174538.1295523-1-ast@kernel.org>
On Tue, Sep 17, 2019 at 12:27 PM Alexei Starovoitov <ast@kernel.org> wrote:
>
> Two trivial BTF fixes.
>
> Alexei Starovoitov (2):
> bpf: fix BTF verification of enums
> bpf: fix BTF limits
>
> include/uapi/linux/btf.h | 4 ++--
> kernel/bpf/btf.c | 5 ++---
> 2 files changed, 4 insertions(+), 5 deletions(-)
>
> --
> 2.20.0
>
For the series:
Acked-by: Andrii Nakryiko <andriin@fb.com>
^ permalink raw reply
* Re: BUG: sleeping function called from invalid context in tcf_chain0_head_change_cb_del
From: Vlad Buslov @ 2019-09-17 19:57 UTC (permalink / raw)
To: Cong Wang
Cc: Vlad Buslov, syzbot, Alexei Starovoitov, Daniel Borkmann,
David Miller, David Ahern, Florian Fainelli, hawk@kernel.org,
Ido Schimmel, Jakub Kicinski, Jamal Hadi Salim, Jiri Pirko,
Jiri Pirko, John Fastabend, Martin KaFai Lau, LKML,
Linux Kernel Network Developers, Nikolay Aleksandrov,
Petr Machata, Roopa Prabhu, Song Liu, syzkaller-bugs,
xdp-newbies@vger.kernel.org, yhs@fb.com
In-Reply-To: <CAM_iQpWNSdx59iTTNO6GdyZ6NBAMD8=wON6Q7dvnhiX50pwEvQ@mail.gmail.com>
On Tue 17 Sep 2019 at 20:03, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Tue, Sep 17, 2019 at 1:27 AM Vlad Buslov <vladbu@mellanox.com> wrote:
>> Hi Cong,
>>
>> Don't see why we would need qdisc tree lock while releasing the
>> reference to (or destroying) previous Qdisc. I've skimmed through other
>> scheds and it looks like sch_multiq, sch_htb and sch_tbf are also
>> affected. Do you want me to send patches?
>
> Yes, please do.
It looks like tbf is not affected by the bug after all. Relevant part of
code from tbf_change():
if (q->qdisc != &noop_qdisc) {
err = fifo_set_limit(q->qdisc, qopt->limit);
if (err)
goto done;
} else if (qopt->limit > 0) {
child = fifo_create_dflt(sch, &bfifo_qdisc_ops, qopt->limit,
extack);
if (IS_ERR(child)) {
err = PTR_ERR(child);
goto done;
}
/* child is fifo, no need to check for noop_qdisc */
qdisc_hash_add(child, true);
}
sch_tree_lock(sch);
if (child) {
qdisc_tree_flush_backlog(q->qdisc);
qdisc_put(q->qdisc);
q->qdisc = child;
}
It seems that qdisc_put() is redundant here because it is only called
q->qdisc == &noop_qdisc, which is a noop.
^ permalink raw reply
* udp sendmsg ENOBUFS clarification
From: Josh Hunt @ 2019-09-17 20:20 UTC (permalink / raw)
To: netdev, Eric Dumazet, David Miller, Willem de Bruijn
I was running some tests recently with the udpgso_bench_tx benchmark in
selftests and noticed that in some configurations it reported sending
more than line rate! Looking into it more I found that I was overflowing
the qdisc queue and so it was sending back NET_XMIT_DROP however this
error did not propagate back up to the application and so it assumed
whatever it sent was done successfully. That's when I learned about
IP_RECVERR and saw that the benchmark isn't using that socket option.
That's all fairly straightforward, but what I was hoping to get
clarification on is where is the line drawn on when or when not to send
ENOBUFS back to the application if IP_RECVERR is *not* set? My guess
based on going through the code is that as long as the packet leaves the
stack (in this case sent to the qdisc) that's where we stop reporting
ENOBUFS back to the application, but can someone confirm?
For example, we sanitize the error in udp_send_skb():
send:
err = ip_send_skb(sock_net(sk), skb);
if (err) {
if (err == -ENOBUFS && !inet->recverr) {
UDP_INC_STATS(sock_net(sk),
UDP_MIB_SNDBUFERRORS, is_udplite);
err = 0;
}
} else
but in udp_sendmsg() we don't:
if (err == -ENOBUFS || test_bit(SOCK_NOSPACE,
&sk->sk_socket->flags)) {
UDP_INC_STATS(sock_net(sk),
UDP_MIB_SNDBUFERRORS, is_udplite);
}
return err;
In the case above it looks like we may only get ENOBUFS for allocation
failures inside of the stack in udp_sendmsg() and so that's why we
propagate the error back up to the application?
Somewhat related, while I was trying to find answer to the above I came
across this thread https://patchwork.ozlabs.org/patch/32857/ It looks
like the man send() man page still only says the following about -ENOBUFS:
"The output queue for a network interface was full.
This generally indicates that the interface has stopped sending,
but may be caused by transient congestion.
(Normally, this does not occur in Linux. Packets are just silently
dropped when a device queue overflows.) "
but as Eric points out that's not true when IP_RECVERR is set on the
socket. Was there an attempt to update the man page to reflect this, but
it was rejected? I couldn't find any discussion on this.
Thanks
Josh
^ permalink raw reply
* [PATCH net-next] net: dsa: mv88e6xxx: Add support for port mirroring
From: Iwan R Timmer @ 2019-09-17 20:23 UTC (permalink / raw)
To: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
netdev
Add support for configuring port mirroring through the cls_matchall
classifier. We do a full ingress and/or egress capture towards the
capture port, configured with set_egress_port.
Signed-off-by: Iwan R Timmer <irtimmer@gmail.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 36 ++++++++++++++++++++++++++++++++
drivers/net/dsa/mv88e6xxx/port.c | 21 +++++++++++++++++++
drivers/net/dsa/mv88e6xxx/port.h | 2 ++
3 files changed, 59 insertions(+)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index d0a97eb73a37..3a48f9d07cbf 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -4595,6 +4595,40 @@ static int mv88e6xxx_port_mdb_del(struct dsa_switch *ds, int port,
return err;
}
+static int mv88e6xxx_port_mirror_add(struct dsa_switch *ds, int port,
+ struct dsa_mall_mirror_tc_entry *mirror,
+ bool ingress)
+{
+ struct mv88e6xxx_chip *chip = ds->priv;
+ int err = -EOPNOTSUPP;
+
+ mutex_lock(&chip->reg_lock);
+ if (chip->info->ops->set_egress_port)
+ err = chip->info->ops->set_egress_port(chip,
+ mirror->to_local_port);
+
+ if (err)
+ goto out;
+
+ err = mv88e6xxx_port_set_mirror(chip, port, true, ingress);
+out:
+ mutex_unlock(&chip->reg_lock);
+
+ return err;
+}
+
+static void mv88e6xxx_port_mirror_del(struct dsa_switch *ds, int port,
+ struct dsa_mall_mirror_tc_entry *mirror)
+{
+ struct mv88e6xxx_chip *chip = ds->priv;
+
+ mutex_lock(&chip->reg_lock);
+ if (mv88e6xxx_port_set_mirror(chip, port, false, false))
+ dev_err(ds->dev, "p%d: failed to disable mirroring\n", port);
+
+ mutex_unlock(&chip->reg_lock);
+}
+
static int mv88e6xxx_port_egress_floods(struct dsa_switch *ds, int port,
bool unicast, bool multicast)
{
@@ -4647,6 +4681,8 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
.port_mdb_prepare = mv88e6xxx_port_mdb_prepare,
.port_mdb_add = mv88e6xxx_port_mdb_add,
.port_mdb_del = mv88e6xxx_port_mdb_del,
+ .port_mirror_add = mv88e6xxx_port_mirror_add,
+ .port_mirror_del = mv88e6xxx_port_mirror_del,
.crosschip_bridge_join = mv88e6xxx_crosschip_bridge_join,
.crosschip_bridge_leave = mv88e6xxx_crosschip_bridge_leave,
.port_hwtstamp_set = mv88e6xxx_port_hwtstamp_set,
diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
index 04309ef0a1cc..301bf704c877 100644
--- a/drivers/net/dsa/mv88e6xxx/port.c
+++ b/drivers/net/dsa/mv88e6xxx/port.c
@@ -1086,6 +1086,27 @@ int mv88e6095_port_set_upstream_port(struct mv88e6xxx_chip *chip, int port,
return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_CTL2, reg);
}
+int mv88e6xxx_port_set_mirror(struct mv88e6xxx_chip *chip, int port,
+ bool mirror, bool ingress)
+{
+ u16 reg;
+ u16 bit;
+ int err;
+
+ err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_CTL2, ®);
+ if (err)
+ return err;
+
+ bit = ingress ? MV88E6XXX_PORT_CTL2_INGRESS_MONITOR :
+ MV88E6XXX_PORT_CTL2_EGRESS_MONITOR;
+ reg &= ~bit;
+
+ if (mirror)
+ reg |= bit;
+
+ return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_CTL2, reg);
+}
+
int mv88e6xxx_port_set_8021q_mode(struct mv88e6xxx_chip *chip, int port,
u16 mode)
{
diff --git a/drivers/net/dsa/mv88e6xxx/port.h b/drivers/net/dsa/mv88e6xxx/port.h
index 8d5a6cd6fb19..40ed60a2099b 100644
--- a/drivers/net/dsa/mv88e6xxx/port.h
+++ b/drivers/net/dsa/mv88e6xxx/port.h
@@ -347,6 +347,8 @@ int mv88e6352_port_link_state(struct mv88e6xxx_chip *chip, int port,
int mv88e6xxx_port_set_map_da(struct mv88e6xxx_chip *chip, int port);
int mv88e6095_port_set_upstream_port(struct mv88e6xxx_chip *chip, int port,
int upstream_port);
+int mv88e6xxx_port_set_mirror(struct mv88e6xxx_chip *chip, int port,
+ bool mirror, bool ingress);
int mv88e6xxx_port_disable_learn_limit(struct mv88e6xxx_chip *chip, int port);
int mv88e6xxx_port_disable_pri_override(struct mv88e6xxx_chip *chip, int port);
--
2.23.0
^ permalink raw reply related
* Re: [PATCH net-next] net: dsa: mv88e6xxx: Add support for port mirroring
From: Andrew Lunn @ 2019-09-17 20:55 UTC (permalink / raw)
To: Iwan R Timmer; +Cc: Vivien Didelot, Florian Fainelli, David S. Miller, netdev
In-Reply-To: <20190917202301.GA29966@i5wan>
On Tue, Sep 17, 2019 at 10:23:01PM +0200, Iwan R Timmer wrote:
> Add support for configuring port mirroring through the cls_matchall
> classifier. We do a full ingress and/or egress capture towards the
> capture port, configured with set_egress_port.
Hi Iwan
This looks good as far as it goes.
Have you tried adding/deleting multiple port mirrors? Do we need to
limit how many are added. A quick look at the datasheet, you can
define one egress mirror port and one ingress mirror port. I think you
can have multiple ports mirroring ingress to that one ingress mirror
port. And you can have multiple port mirroring egress to the one
egress mirror port. We should add code to check this, and return
-EBUSY if the existing configuration prevents a new mirror being
configured.
Thanks
Andrew
^ permalink raw reply
* [PATCH V2 net 1/1] net/tls(TLS_SW): Fix list_del double free caused by a race condition in tls_tx_records
From: Pooja Trivedi @ 2019-09-17 21:13 UTC (permalink / raw)
To: netdev
Cc: davem, jakub.kicinski, daniel, john.fastabend, davejwatson,
aviadye, borisp, Pooja Trivedi
In-Reply-To: <CAOrEdsmiz-ssFUpcT_43JfASLYRbt60R7Ta0KxuhrMN35cP0Sw@mail.gmail.com>
From: Pooja Trivedi <pooja.trivedi@stackpath.com>
Enclosing tls_tx_records within lock_sock/release_sock pair to ensure
write-synchronization is not sufficient because socket lock gets released
under memory pressure situation by sk_wait_event while it sleeps waiting
for memory, allowing another writer into tls_tx_records. This causes a
race condition with record deletion post transmission.
To fix this bug, use a flag set in tx_bitmask field of TLS context to
ensure single writer in tls_tx_records at a time
The bug resulted in the following crash:
[ 270.888952] ------------[ cut here ]------------
[ 270.890450] list_del corruption, ffff91cc3753a800->prev is
LIST_POISON2 (dead000000000122)
[ 270.891194] WARNING: CPU: 1 PID: 7387 at lib/list_debug.c:50
__list_del_entry_valid+0x62/0x90
[ 270.892037] Modules linked in: n5pf(OE) netconsole tls(OE) bonding
intel_rapl_msr intel_rapl_common sb_edac x86_pkg_temp_thermal
intel_powerclamp coretemp kvm_intel kvm iTCO_wdt iTCO_vendor_support
irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel
aesni_intel crypto_simd mei_me cryptd glue_helper ipmi_si sg mei
lpc_ich pcspkr joydev ioatdma i2c_i801 ipmi_devintf ipmi_msghandler
wmi ip_tables xfs libcrc32c sd_mod mgag200 drm_vram_helper ttm
drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm isci
libsas ahci scsi_transport_sas libahci crc32c_intel serio_raw igb
libata ptp pps_core dca i2c_algo_bit dm_mirror dm_region_hash dm_log
dm_mod [last unloaded: nitrox_drv]
[ 270.896836] CPU: 1 PID: 7387 Comm: uperf Kdump: loaded Tainted: G
OE 5.3.0-rc4 #1
[ 270.897711] Hardware name: Supermicro SYS-1027R-N3RF/X9DRW, BIOS
3.0c 03/24/2014
[ 270.898597] RIP: 0010:__list_del_entry_valid+0x62/0x90
[ 270.899478] Code: 00 00 00 c3 48 89 fe 48 89 c2 48 c7 c7 e0 f9 ee
8d e8 b2 cf c8 ff 0f 0b 31 c0 c3 48 89 fe 48 c7 c7 18 fa ee 8d e8 9e
cf c8 ff <0f> 0b 31 c0 c3 48 89 f2 48 89 fe 48 c7 c7 50 fa ee 8d e8 87
cf c8
[ 270.901321] RSP: 0018:ffffb6ea86eb7c20 EFLAGS: 00010282
[ 270.902240] RAX: 0000000000000000 RBX: ffff91cc3753c000 RCX: 0000000000000000
[ 270.903157] RDX: ffff91bc3f867080 RSI: ffff91bc3f857738 RDI: ffff91bc3f857738
[ 270.904074] RBP: ffff91bc36020940 R08: 0000000000000560 R09: 0000000000000000
[ 270.904988] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
[ 270.905902] R13: ffff91cc3753a800 R14: ffff91cc37cc6400 R15: ffff91cc3753a800
[ 270.906809] FS: 00007f454a88d700(0000) GS:ffff91bc3f840000(0000)
knlGS:0000000000000000
[ 270.907715] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 270.908606] CR2: 00007f453c00292c CR3: 000000103554e003 CR4: 00000000001606e0
[ 270.909490] Call Trace:
[ 270.910373] tls_tx_records+0x138/0x1c0 [tls]
[ 270.911262] tls_sw_sendpage+0x3e0/0x420 [tls]
[ 270.912154] inet_sendpage+0x52/0x90
[ 270.913045] ? direct_splice_actor+0x40/0x40
[ 270.913941] kernel_sendpage+0x1a/0x30
[ 270.914831] sock_sendpage+0x20/0x30
[ 270.915714] pipe_to_sendpage+0x62/0x90
[ 270.916592] __splice_from_pipe+0x80/0x180
[ 270.917461] ? direct_splice_actor+0x40/0x40
[ 270.918334] splice_from_pipe+0x5d/0x90
[ 270.919208] direct_splice_actor+0x35/0x40
[ 270.920086] splice_direct_to_actor+0x103/0x230
[ 270.920966] ? generic_pipe_buf_nosteal+0x10/0x10
[ 270.921850] do_splice_direct+0x9a/0xd0
[ 270.922733] do_sendfile+0x1c9/0x3d0
[ 270.923612] __x64_sys_sendfile64+0x5c/0xc0
Signed-off-by: Pooja Trivedi <pooja.trivedi@stackpath.com>
---
include/net/tls.h | 1 +
net/tls/tls_sw.c | 7 +++++++
2 files changed, 8 insertions(+)
diff --git a/include/net/tls.h b/include/net/tls.h
index 41b2d41..f346a54 100644
--- a/include/net/tls.h
+++ b/include/net/tls.h
@@ -161,6 +161,7 @@ struct tls_sw_context_tx {
#define BIT_TX_SCHEDULED 0
#define BIT_TX_CLOSING 1
+#define BIT_TX_IN_PROGRESS 2
unsigned long tx_bitmask;
};
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 91d21b0..6e99c61 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -367,6 +367,10 @@ int tls_tx_records(struct sock *sk, int flags)
struct sk_msg *msg_en;
int tx_flags, rc = 0;
+ /* If another writer is already in tls_tx_records, backoff and leave */
+ if (test_and_set_bit(BIT_TX_IN_PROGRESS, &ctx->tx_bitmask))
+ return 0;
+
if (tls_is_partially_sent_record(tls_ctx)) {
rec = list_first_entry(&ctx->tx_list,
struct tls_rec, list);
@@ -415,6 +419,9 @@ int tls_tx_records(struct sock *sk, int flags)
if (rc < 0 && rc != -EAGAIN)
tls_err_abort(sk, EBADMSG);
+ /* clear the bit so another writer can get into tls_tx_records */
+ clear_bit(BIT_TX_IN_PROGRESS, &ctx->tx_bitmask);
+
return rc;
}
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH V2 net 1/1] net/tls(TLS_SW): Fix list_del double free caused by a race condition in tls_tx_records
From: Pooja Trivedi @ 2019-09-17 21:24 UTC (permalink / raw)
To: netdev
Cc: davem, jakub.kicinski, daniel, john.fastabend, davejwatson,
aviadye, borisp, Pooja Trivedi
In-Reply-To: <1568754836-25124-1-git-send-email-poojatrivedi@gmail.com>
Hello,
I see that the 'State' of the initially submitted patch has changed to
"Changes Requested" but I have not seen a follow-up email regarding
what needs to be changed. I also realized that the tabs had not been
preserved in my initial submission, hence the resubmission.
Thanks,
Pooja.
On Tue, Sep 17, 2019 at 5:14 PM Pooja Trivedi <poojatrivedi@gmail.com> wrote:
>
> From: Pooja Trivedi <pooja.trivedi@stackpath.com>
>
> Enclosing tls_tx_records within lock_sock/release_sock pair to ensure
> write-synchronization is not sufficient because socket lock gets released
> under memory pressure situation by sk_wait_event while it sleeps waiting
> for memory, allowing another writer into tls_tx_records. This causes a
> race condition with record deletion post transmission.
>
> To fix this bug, use a flag set in tx_bitmask field of TLS context to
> ensure single writer in tls_tx_records at a time
>
> The bug resulted in the following crash:
>
> [ 270.888952] ------------[ cut here ]------------
> [ 270.890450] list_del corruption, ffff91cc3753a800->prev is
> LIST_POISON2 (dead000000000122)
> [ 270.891194] WARNING: CPU: 1 PID: 7387 at lib/list_debug.c:50
> __list_del_entry_valid+0x62/0x90
> [ 270.892037] Modules linked in: n5pf(OE) netconsole tls(OE) bonding
> intel_rapl_msr intel_rapl_common sb_edac x86_pkg_temp_thermal
> intel_powerclamp coretemp kvm_intel kvm iTCO_wdt iTCO_vendor_support
> irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel
> aesni_intel crypto_simd mei_me cryptd glue_helper ipmi_si sg mei
> lpc_ich pcspkr joydev ioatdma i2c_i801 ipmi_devintf ipmi_msghandler
> wmi ip_tables xfs libcrc32c sd_mod mgag200 drm_vram_helper ttm
> drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops drm isci
> libsas ahci scsi_transport_sas libahci crc32c_intel serio_raw igb
> libata ptp pps_core dca i2c_algo_bit dm_mirror dm_region_hash dm_log
> dm_mod [last unloaded: nitrox_drv]
> [ 270.896836] CPU: 1 PID: 7387 Comm: uperf Kdump: loaded Tainted: G
> OE 5.3.0-rc4 #1
> [ 270.897711] Hardware name: Supermicro SYS-1027R-N3RF/X9DRW, BIOS
> 3.0c 03/24/2014
> [ 270.898597] RIP: 0010:__list_del_entry_valid+0x62/0x90
> [ 270.899478] Code: 00 00 00 c3 48 89 fe 48 89 c2 48 c7 c7 e0 f9 ee
> 8d e8 b2 cf c8 ff 0f 0b 31 c0 c3 48 89 fe 48 c7 c7 18 fa ee 8d e8 9e
> cf c8 ff <0f> 0b 31 c0 c3 48 89 f2 48 89 fe 48 c7 c7 50 fa ee 8d e8 87
> cf c8
> [ 270.901321] RSP: 0018:ffffb6ea86eb7c20 EFLAGS: 00010282
> [ 270.902240] RAX: 0000000000000000 RBX: ffff91cc3753c000 RCX: 0000000000000000
> [ 270.903157] RDX: ffff91bc3f867080 RSI: ffff91bc3f857738 RDI: ffff91bc3f857738
> [ 270.904074] RBP: ffff91bc36020940 R08: 0000000000000560 R09: 0000000000000000
> [ 270.904988] R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
> [ 270.905902] R13: ffff91cc3753a800 R14: ffff91cc37cc6400 R15: ffff91cc3753a800
> [ 270.906809] FS: 00007f454a88d700(0000) GS:ffff91bc3f840000(0000)
> knlGS:0000000000000000
> [ 270.907715] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 270.908606] CR2: 00007f453c00292c CR3: 000000103554e003 CR4: 00000000001606e0
> [ 270.909490] Call Trace:
> [ 270.910373] tls_tx_records+0x138/0x1c0 [tls]
> [ 270.911262] tls_sw_sendpage+0x3e0/0x420 [tls]
> [ 270.912154] inet_sendpage+0x52/0x90
> [ 270.913045] ? direct_splice_actor+0x40/0x40
> [ 270.913941] kernel_sendpage+0x1a/0x30
> [ 270.914831] sock_sendpage+0x20/0x30
> [ 270.915714] pipe_to_sendpage+0x62/0x90
> [ 270.916592] __splice_from_pipe+0x80/0x180
> [ 270.917461] ? direct_splice_actor+0x40/0x40
> [ 270.918334] splice_from_pipe+0x5d/0x90
> [ 270.919208] direct_splice_actor+0x35/0x40
> [ 270.920086] splice_direct_to_actor+0x103/0x230
> [ 270.920966] ? generic_pipe_buf_nosteal+0x10/0x10
> [ 270.921850] do_splice_direct+0x9a/0xd0
> [ 270.922733] do_sendfile+0x1c9/0x3d0
> [ 270.923612] __x64_sys_sendfile64+0x5c/0xc0
>
> Signed-off-by: Pooja Trivedi <pooja.trivedi@stackpath.com>
> ---
> include/net/tls.h | 1 +
> net/tls/tls_sw.c | 7 +++++++
> 2 files changed, 8 insertions(+)
>
> diff --git a/include/net/tls.h b/include/net/tls.h
> index 41b2d41..f346a54 100644
> --- a/include/net/tls.h
> +++ b/include/net/tls.h
> @@ -161,6 +161,7 @@ struct tls_sw_context_tx {
>
> #define BIT_TX_SCHEDULED 0
> #define BIT_TX_CLOSING 1
> +#define BIT_TX_IN_PROGRESS 2
> unsigned long tx_bitmask;
> };
>
> diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
> index 91d21b0..6e99c61 100644
> --- a/net/tls/tls_sw.c
> +++ b/net/tls/tls_sw.c
> @@ -367,6 +367,10 @@ int tls_tx_records(struct sock *sk, int flags)
> struct sk_msg *msg_en;
> int tx_flags, rc = 0;
>
> + /* If another writer is already in tls_tx_records, backoff and leave */
> + if (test_and_set_bit(BIT_TX_IN_PROGRESS, &ctx->tx_bitmask))
> + return 0;
> +
> if (tls_is_partially_sent_record(tls_ctx)) {
> rec = list_first_entry(&ctx->tx_list,
> struct tls_rec, list);
> @@ -415,6 +419,9 @@ int tls_tx_records(struct sock *sk, int flags)
> if (rc < 0 && rc != -EAGAIN)
> tls_err_abort(sk, EBADMSG);
>
> + /* clear the bit so another writer can get into tls_tx_records */
> + clear_bit(BIT_TX_IN_PROGRESS, &ctx->tx_bitmask);
> +
> return rc;
> }
>
> --
> 1.8.3.1
>
^ permalink raw reply
* Re: Bug report (with fix) for DEC Tulip driver (de2104x.c)
From: Andrew Lunn @ 2019-09-17 21:28 UTC (permalink / raw)
To: Arlie Davis; +Cc: netdev, linux-parisc
In-Reply-To: <CAK-9enMxA68mRYFG=2zD02guvCqe-aa3NO0YZuJcTdBWn5MPqg@mail.gmail.com>
On Mon, Sep 16, 2019 at 02:50:53PM -0700, Arlie Davis wrote:
> Hello. I'm a developer on GCE, Google's virtual machine platform. As
> part of my work, we needed to emulate a DEC Tulip 2104x NIC, so I
> implemented a basic virtual device for it.
>
> While doing so, I believe I found a bug in the Linux driver for this
> device, in de2104x.c. I see in MAINTAINERS that this is an orphaned
> device driver, but I was wondering if the kernel would still accept a
> patch for it. Should I submit this patch, and if so, where should I
> submit it?
>
> Below is the commit text from my local repo, and the patch diffs
> (they're quite short).
>
> Fix a bug in DEC Tulip driver (de2104x.c)
>
> The DEC Tulip Ethernet controller uses a 16-byte transfer descriptor for
> both its transmit (tx) and receive (rx) rings. Each descriptor has a
> "status" uint32 field (called opts1 in de2104x.c, and called TDES0 /
> Status in the DEC hardware specifications) and a "control" field (called
> opts2 in de2104x.c and called TDES1 / Control in the DEC
> specifications). In the "control" field, bit 30 is the LastSegment bit,
> which indicates that this is the last transfer descriptor in a sequence
> of descriptors (in case a single Ethernet frame spans more than one
> descriptor).
>
> The de2104x driver correctly sets LastSegment, in the de_start_xmit
> function. (The code calls it LastFrag, not LastSegment). However, in the
> interrupt handler (in function de_tx), the driver incorrectly checks for
> this bit in the status field, not the control field. This means that the
> driver is reading bits that are undefined in the specification; the
> spec does not make any guarantees at all about the contents of bits 29
> and bits 30 in the "status" field.
>
> The effect of the bug is that the driver may think that a TX ring entry
> is never finished, even though a compliant DEC Tulip hardware device (or
> a virtualized device, in a VM) actually did finish sending the Ethernet
> frame.
>
> The fix is to read the correct "control" field from the TX descriptor.
>
> DEC Tulip programming specification:
>
> https://web.archive.org/web/20050805091751/http://www.intel.com/design/network/manuals/21140ahm.pdf
Hi Arlie
Without having access to real hardware, it is hard to verify
this. Maybe the programming specification is wrong? It could be, the
hardware designer thought the control field should be write only from
the CPU side, and the status field read only from the CPU side, to
avoid race conditions. So in practice it does mirror the LastSegment
bit from control to status?
Are there any other emulators of this out there? Any silicon vendor
who produces devices which claim to be compatible?
Andrew
^ permalink raw reply
* Re: Bug report (with fix) for DEC Tulip driver (de2104x.c)
From: Arlie Davis @ 2019-09-17 21:36 UTC (permalink / raw)
To: Andrew Lunn; +Cc: netdev, linux-parisc
In-Reply-To: <20190917212844.GJ9591@lunn.ch>
I checked QEMU v3.1, and I don't see any Tulip implementation for it.
I haven't checked any other emulators. A few cursory searches didn't
turn up anything.
I checked the FreeBSD driver for the same device. It just treats the
control field as a write-only field; the driver just uses its own
internal state, rather than reading anything from the transfer
descriptor, aside from the relevant status bits.
My guess is that the hardware just always sets bit 30 = 1, in the
status field. In this Linux driver, for a normal packet (non-SETUP,
non-DUMMY), all packets use a single TX descriptor, so LastFrag=1 is
always true. Because of this, I considered changing the Linux driver
to just remove the "if (status & LastFrag)" check, and make it
unconditional, since this driver never uses more than 1 descriptor per
transmitted packet. Would you support that change?
Likewise, I'm at a loss for testing with real hardware. It's hard to
find such things, now.
On Tue, Sep 17, 2019 at 2:28 PM Andrew Lunn <andrew@lunn.ch> wrote:
>
> On Mon, Sep 16, 2019 at 02:50:53PM -0700, Arlie Davis wrote:
> > Hello. I'm a developer on GCE, Google's virtual machine platform. As
> > part of my work, we needed to emulate a DEC Tulip 2104x NIC, so I
> > implemented a basic virtual device for it.
> >
> > While doing so, I believe I found a bug in the Linux driver for this
> > device, in de2104x.c. I see in MAINTAINERS that this is an orphaned
> > device driver, but I was wondering if the kernel would still accept a
> > patch for it. Should I submit this patch, and if so, where should I
> > submit it?
> >
> > Below is the commit text from my local repo, and the patch diffs
> > (they're quite short).
> >
> > Fix a bug in DEC Tulip driver (de2104x.c)
> >
> > The DEC Tulip Ethernet controller uses a 16-byte transfer descriptor for
> > both its transmit (tx) and receive (rx) rings. Each descriptor has a
> > "status" uint32 field (called opts1 in de2104x.c, and called TDES0 /
> > Status in the DEC hardware specifications) and a "control" field (called
> > opts2 in de2104x.c and called TDES1 / Control in the DEC
> > specifications). In the "control" field, bit 30 is the LastSegment bit,
> > which indicates that this is the last transfer descriptor in a sequence
> > of descriptors (in case a single Ethernet frame spans more than one
> > descriptor).
> >
> > The de2104x driver correctly sets LastSegment, in the de_start_xmit
> > function. (The code calls it LastFrag, not LastSegment). However, in the
> > interrupt handler (in function de_tx), the driver incorrectly checks for
> > this bit in the status field, not the control field. This means that the
> > driver is reading bits that are undefined in the specification; the
> > spec does not make any guarantees at all about the contents of bits 29
> > and bits 30 in the "status" field.
> >
> > The effect of the bug is that the driver may think that a TX ring entry
> > is never finished, even though a compliant DEC Tulip hardware device (or
> > a virtualized device, in a VM) actually did finish sending the Ethernet
> > frame.
> >
> > The fix is to read the correct "control" field from the TX descriptor.
> >
> > DEC Tulip programming specification:
> >
> > https://web.archive.org/web/20050805091751/http://www.intel.com/design/network/manuals/21140ahm.pdf
>
> Hi Arlie
>
> Without having access to real hardware, it is hard to verify
> this. Maybe the programming specification is wrong? It could be, the
> hardware designer thought the control field should be write only from
> the CPU side, and the status field read only from the CPU side, to
> avoid race conditions. So in practice it does mirror the LastSegment
> bit from control to status?
>
> Are there any other emulators of this out there? Any silicon vendor
> who produces devices which claim to be compatible?
>
> Andrew
^ permalink raw reply
* Re: [PATCH net] ipv4: Revert removal of rt_uses_gateway
From: Julian Anastasov @ 2019-09-17 22:02 UTC (permalink / raw)
To: David Ahern; +Cc: David Ahern, davem, netdev
In-Reply-To: <9afca894-3807-632a-529b-7ceee4227bcb@gmail.com>
Hello,
On Tue, 17 Sep 2019, David Ahern wrote:
> On 9/17/19 12:50 PM, Julian Anastasov wrote:
> >
> > Looks good to me, thanks!
> >
> > Reviewed-by: Julian Anastasov <ja@ssi.bg>
> >
>
> BTW, do you have any tests for the rt_uses_gateway paths - showing why
> it is needed?
No special tests.
> All of the pmtu, redirect, fib tests, etc worked fine without the
> special flag. Sure, the 'ip ro get' had extra data; it seems like that
> could be handled.
I'll explain. In the period before the route cache
was removed in 3.6, there were two fields: rt_dst and rt_gateway.
For targets on LAN both contained the target. For targets via
gateway (nh_gw), rt_dst still stores the target but rt_gateway
stored the nh_gw. In short, rt_gateway always contained the
next hop IP for neigh resolving.
In 3.6, rt_dst was removed and only rt_gateway remained
to store nh_gw. As fnhe_rth/nh_pcpu_rth_output were used to
cache the output route, rt_gateway can not contain any IP
because the route can be used for any target on the LAN.
This is true even now, rt_gateway is 0 for cached routes
that are not DST_HOST, i.e. not for single target IP.
Why this matters? There are users such as IPVS,
TEE, raw.c (IP_HDRINCL) that use FLOWI_FLAG_KNOWN_NH to
request route with rt_gateway != 0 to be returned, i.e.
they want rt_gateway to store a next hop IP for neigh
resolving but to put different IP in iph->daddr. This is
honoured by rt_nexthop(), it prefers rt_gateway before the
iph->daddr. With this FLOWI flag the routing will avoid
returning routes with rt_gateway = 0 (cached in NH), instead
it will allocate DST_HOST route which can safely hold IP
in rt_gateway.
So, in 3.7 commit 155e8336c373 ("ipv4: introduce
rt_uses_gateway") was created to restore the knowledge
that rt_dst != rt_gateway means route via GW and also
commit c92b96553a80 ("ipv4: Add FLOWI_FLAG_KNOWN_NH")
to make sure packets are routed by requested next hop and
not by iph->daddr.
You see the places that need to know if rt_gateway
contains nh_gw (via GW) or just a requested next hop (when
nh_gw is 0). It matters for cases where strict source routes
should be on connected network.
In simple tests things are working without rt_uses_gateway
flag because it is a corner case to see above cases combined
with strict source routing or MTU locking. Not sure if we can
use some trick to support it differently without such flag.
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply
* Re: [PATCH net-next] net: dsa: mv88e6xxx: Add support for port mirroring
From: Iwan R Timmer @ 2019-09-17 22:32 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Vivien Didelot, Florian Fainelli, David S. Miller, netdev
In-Reply-To: <20190917205505.GF9591@lunn.ch>
On Tue, Sep 17, 2019 at 10:55:05PM +0200, Andrew Lunn wrote:
> On Tue, Sep 17, 2019 at 10:23:01PM +0200, Iwan R Timmer wrote:
> > Add support for configuring port mirroring through the cls_matchall
> > classifier. We do a full ingress and/or egress capture towards the
> > capture port, configured with set_egress_port.
>
> Hi Iwan
>
> This looks good as far as it goes.
>
> Have you tried adding/deleting multiple port mirrors? Do we need to
> limit how many are added. A quick look at the datasheet, you can
> define one egress mirror port and one ingress mirror port. I think you
> can have multiple ports mirroring ingress to that one ingress mirror
> port. And you can have multiple port mirroring egress to the one
> egress mirror port. We should add code to check this, and return
> -EBUSY if the existing configuration prevents a new mirror being
> configured.
>
> Thanks
> Andrew
Hi Andrew,
I only own a simple 5 ports switch (88E6176) which has no problem of
mirroring the other ports to a single port. Except for a bandwith
shortage ofcourse. While I thought I checked adding and removing ports,
I seemed to forgot to check removing ingress traffic as it will now
disable mirroring egress traffic. Searching for how I can distinct
ingress from egress mirroring in port_mirror_del, I saw there is a
variable in the mirror struct called ingress. Which seems strange,
because why is it a seperate argument to the port_mirror_add function?
Origally I planned to be able to set the egress and ingress mirror
seperatly. But in my laziness when I saw there already was a function
to configure the destination port this functionality was lost.
Because the other drivers which implemented the port_mirror_add (b53 and
ksz9477) also lacks additional checks to prevent new mirror filters from
breaking previous ones I assumed they were not necessary.
At least I will soon sent a new version with at least the issue of
removing mirror ingress traffic fixed and the ability to define a
seperate ingress and egress port.
Regards,
Iwan
^ 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