* Re: [PATCH net-next 0/3] net: Allow more drivers with COMPILE_TEST
From: Florian Fainelli @ 2018-05-16 18:18 UTC (permalink / raw)
To: David Miller; +Cc: netdev, andrew, linux-kernel
In-Reply-To: <20180516.141738.494765267490482500.davem@davemloft.net>
On 05/16/2018 11:17 AM, David Miller wrote:
> From: David Miller <davem@davemloft.net>
> Date: Wed, 16 May 2018 14:15:14 -0400 (EDT)
>
>> From: Florian Fainelli <f.fainelli@gmail.com>
>> Date: Tue, 15 May 2018 16:48:22 -0700
>>
>>> This patch series includes more drivers to be build tested with COMPILE_TEST
>>> enabled. This helps cover some of the issues I just ran into with missing
>>> a driver *sigh*.
>>
>> Thank you for doing this.
>>
>> Series applied.
>
> Actually, I have to revert.
>
> The FEC driver cannot be built tested without the appropriate platform
> specific register set defines and x86-64 for example won't provide that.
Ah, yes, it built okay for ARM 32-bit, let me fix that.
>
> drivers/net/ethernet/freescale/fec_main.c: In function ‘fec_restart’:
> drivers/net/ethernet/freescale/fec_main.c:959:26: error: ‘FEC_RACC’ undeclared (first use in this function); did you mean ‘FEC_RXIC0’?
> val = readl(fep->hwp + FEC_RACC);
> ^~~~~~~~
> FEC_RXIC0
> drivers/net/ethernet/freescale/fec_main.c:959:26: note: each undeclared identifier is reported only once for each function it appears in
> drivers/net/ethernet/freescale/fec_main.c:968:38: error: ‘FEC_FTRL’ undeclared (first use in this function); did you mean ‘FEC_ECNTRL’?
> writel(PKT_MAXBUF_SIZE, fep->hwp + FEC_FTRL);
> ^~~~~~~~
> FEC_ECNTRL
> ...
>
--
Florian
^ permalink raw reply
* [PATCH net] net/ipv4: Initialize proto and ports in flow struct
From: David Ahern @ 2018-05-16 18:18 UTC (permalink / raw)
To: netdev; +Cc: roopa, David Ahern
Updating the FIB tracepoint for the recent change to allow rules using
the protocol and ports exposed a few places where the entries in the flow
struct are not initialized.
For __fib_validate_source add the call to fib4_rules_early_flow_dissect
since it is invoked for the input path. For netfilter, add the memset on
the flow struct to avoid future problems like this.
Fixes: bfff4862653b ("net: fib_rules: support for match on ip_proto, sport and dport")
Signed-off-by: David Ahern <dsahern@gmail.com>
---
net/ipv4/fib_frontend.c | 10 +++++++---
net/ipv4/netfilter/ipt_rpfilter.c | 2 +-
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index f05afaf3235c..58696b829065 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -326,10 +326,11 @@ static int __fib_validate_source(struct sk_buff *skb, __be32 src, __be32 dst,
u8 tos, int oif, struct net_device *dev,
int rpf, struct in_device *idev, u32 *itag)
{
+ struct net *net = dev_net(dev);
+ struct flow_keys flkeys;
int ret, no_addr;
struct fib_result res;
struct flowi4 fl4;
- struct net *net = dev_net(dev);
bool dev_match;
fl4.flowi4_oif = 0;
@@ -347,8 +348,11 @@ static int __fib_validate_source(struct sk_buff *skb, __be32 src, __be32 dst,
no_addr = idev->ifa_list == NULL;
fl4.flowi4_mark = IN_DEV_SRC_VMARK(idev) ? skb->mark : 0;
-
- trace_fib_validate_source(dev, &fl4);
+ if (!fib4_rules_early_flow_dissect(net, skb, &fl4, &flkeys)) {
+ fl4.flowi4_proto = 0;
+ fl4.fl4_sport = 0;
+ fl4.fl4_dport = 0;
+ }
if (fib_lookup(net, &fl4, &res, 0))
goto last_resort;
diff --git a/net/ipv4/netfilter/ipt_rpfilter.c b/net/ipv4/netfilter/ipt_rpfilter.c
index fd01f13c896a..12843c9ef142 100644
--- a/net/ipv4/netfilter/ipt_rpfilter.c
+++ b/net/ipv4/netfilter/ipt_rpfilter.c
@@ -89,10 +89,10 @@ static bool rpfilter_mt(const struct sk_buff *skb, struct xt_action_param *par)
return true ^ invert;
}
+ memset(&flow, 0, sizeof(flow));
flow.flowi4_iif = LOOPBACK_IFINDEX;
flow.daddr = iph->saddr;
flow.saddr = rpfilter_get_saddr(iph->daddr);
- flow.flowi4_oif = 0;
flow.flowi4_mark = info->flags & XT_RPFILTER_VALID_MARK ? skb->mark : 0;
flow.flowi4_tos = RT_TOS(iph->tos);
flow.flowi4_scope = RT_SCOPE_UNIVERSE;
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next v2 0/2] of: mdio: Fall back to mdiobus_register() with NULL device_node
From: David Miller @ 2018-05-16 18:21 UTC (permalink / raw)
To: f.fainelli
Cc: netdev, andrew, vivien.didelot, nicolas.ferre, fugang.duan,
sergei.shtylyov, peppe.cavallaro, alexandre.torgue, joabreu,
grygorii.strashko, woojung.huh, UNGLinuxDriver, robh+dt,
frowand.list, antoine.tenart, Tobias.Jordan, rmk+kernel,
geert+renesas, thomas.petazzoni, niklas.soderlund+renesas,
horms+renesas, muvarov, nsekhar, linux-kernel, linux-renesas-soc,
linux-omap, linux-usb, devicetree
In-Reply-To: <20180515235619.27773-1-f.fainelli@gmail.com>
From: Florian Fainelli <f.fainelli@gmail.com>
Date: Tue, 15 May 2018 16:56:17 -0700
> This patch series updates of_mdiobus_register() such that when the device_node
> argument is NULL, it calls mdiobus_register() directly. This is consistent with
> the behavior of of_mdiobus_register() when CONFIG_OF=n.
>
> I only converted the most obvious drivers, there are others that have a much
> less obvious behavior and specifically attempt to deal with CONFIG_ACPI.
>
> Changes in v2:
>
> - fixed build error in davincin_mdio.c (Grygorii)
> - reworked first patch a bit: commit message, subject and removed useless
> code comment
Based upon Andrew's response to Geert's feedback, I'm applying this series.
Thanks.
^ permalink raw reply
* Re: [PATCH net-next v2 0/3] net: qualcomm: rmnet: Updates 2018-05-14
From: David Miller @ 2018-05-16 18:23 UTC (permalink / raw)
To: subashab; +Cc: netdev, fengguang.wu
In-Reply-To: <1526431922-4132-1-git-send-email-subashab@codeaurora.org>
From: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
Date: Tue, 15 May 2018 18:51:59 -0600
> Patch 1 adds tx_drops counter to more places.
> Patch 2 adds ethtool private stats support to make it easy to debug
> the checksum offload path.
> Patch 3 is a cleanup in command packet processing path.
>
> v1->v2: Fix the incorrect if / else statement in
> rmnet_map_checksum_downlink_packet() and define rmnet_ethtool_ops
> as static as mentioned by kbuild test robot.
Series applied, thank you.
^ permalink raw reply
* [BUG] net: stmmac: dwmac-sun8i broken in linux-next
From: Corentin Labbe @ 2018-05-16 18:32 UTC (permalink / raw)
To: Jose.Abreu-HKixBCOQz3hWk0Htik3J/w, davem-fT/PcQaiUtIeIZ0/mPfg9Q,
alexandre.torgue-qxv4g6HH51o, peppe.cavallaro-qxv4g6HH51o
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
Hello
The dwmac-sun8i driver is broken in next-20180515, symptom are no RX and TX errors as shown by ifconfig:
eth0: flags=4163<UP,BROADCAST,RUNNING,MULTICAST> mtu 1500
inet 192.168.1.204 netmask 255.255.255.0 broadcast 192.168.1.255
ether 96:75:ff:0d:f6:d8 txqueuelen 1000 (Ethernet)
RX packets 0 bytes 0 (0.0 B)
RX errors 0 dropped 0 overruns 0 frame 0
TX packets 0 bytes 4956 (4.8 KiB)
TX errors 118 dropped 0 overruns 0 carrier 0 collisions 0
Reverting the following commit made the driver working:
4dbbe8dde8485b89bce8bbbe7564337fd7eed69f ("net: stmmac: Add support for U32 TC filter using Flexible RX Parser")
5f0456b43140af9413397cc11d03d18b9f2fc2fc ("net: stmmac: Implement logic to automatically select HW Interface")
Note that reverting only 4dbbe8dde8485b89bce8bbbe7564337fd7eed69f lead to crash:
[ 31.385110] Backtrace:
[ 31.387576] [<c0510cd4>] (stmmac_open) from [<c062cf48>] (__dev_open+0xe4/0x180)
[ 31.394972] r10:ed447d04 r9:edc5d010 r8:ef02002c r7:c08670a4 r6:00000000 r5:c0c08488
[ 31.402793] r4:ef020000
[ 31.405335] [<c062ce64>] (__dev_open) from [<c062d374>] (__dev_change_flags+0x190/0x1e8)
[ 31.413421] r8:00001002 r7:c0c08488 r6:00001003 r5:00000001 r4:ef020000
[ 31.420122] [<c062d1e4>] (__dev_change_flags) from [<c062d3ec>] (dev_change_flags+0x20/0x50)
[ 31.428555] r9:edc5d010 r8:ed447c18 r7:ef020134 r6:00000000 r5:00001002 r4:ef020000
[ 31.436300] [<c062d3cc>] (dev_change_flags) from [<c0646ef0>] (do_setlink+0x28c/0xbdc)
[ 31.444213] r9:edc5d010 r8:ed447c18 r7:00000000 r6:c0c08488 r5:ed447b50 r4:ef020000
[ 31.451955] [<c0646c64>] (do_setlink) from [<c0648064>] (rtnl_newlink+0x54c/0x7a8)
[ 31.459522] r10:ed447d04 r9:00000000 r8:00000000 r7:00000000 r6:00000000 r5:00000000
[ 31.467343] r4:ef020000
[ 31.469885] [<c0647b18>] (rtnl_newlink) from [<c06449f8>] (rtnetlink_rcv_msg+0x38c/0x544)
[ 31.478058] r10:ed447d04 r9:00000000 r8:ee242840 r7:00000000 r6:edc5d000 r5:c0c08488
[ 31.485879] r4:00000000
[ 31.488422] [<c064466c>] (rtnetlink_rcv_msg) from [<c066e458>] (netlink_rcv_skb+0xc0/0x118)
[ 31.496768] r10:c0c08488 r9:00000000 r8:00000020 r7:edc5d000 r6:c064466c r5:c0c08488
[ 31.504589] r4:ee242840
[ 31.507129] [<c066e398>] (netlink_rcv_skb) from [<c0644668>] (rtnetlink_rcv+0x18/0x1c)
[ 31.515042] r8:ed447d60 r7:ee242840 r6:00000020 r5:ee37d800 r4:ee5fac00
[ 31.521742] [<c0644650>] (rtnetlink_rcv) from [<c066db98>] (netlink_unicast+0x190/0x1fc)
[ 31.529829] [<c066da08>] (netlink_unicast) from [<c066e098>] (netlink_sendmsg+0x3cc/0x410)
[ 31.538089] r10:00000000 r9:00000020 r8:014000c0 r7:ee242840 r6:ee37d800 r5:c0c08488
[ 31.545910] r4:ed447f44
[ 31.548452] [<c066dccc>] (netlink_sendmsg) from [<c0604460>] (sock_sendmsg+0x1c/0x2c)
[ 31.556279] r10:00000000 r9:ed447edc r8:00000000 r7:eefce640 r6:00000000 r5:c0c08488
[ 31.564100] r4:ed447f44
[ 31.566640] [<c0604444>] (sock_sendmsg) from [<c0604d9c>] (___sys_sendmsg+0x250/0x264)
[ 31.574555] [<c0604b4c>] (___sys_sendmsg) from [<c0605e98>] (__sys_sendmsg+0x58/0x94)
[ 31.582382] r10:00000000 r9:ed446000 r8:c01011c4 r7:eefce640 r6:00000000 r5:bec25150
[ 31.590203] r4:c0c08488
[ 31.592743] [<c0605e40>] (__sys_sendmsg) from [<c0605ee8>] (sys_sendmsg+0x14/0x18)
[ 31.600307] r7:00000128 r6:bec2d17c r5:bec25144 r4:00093ee0
[ 31.605969] [<c0605ed4>] (sys_sendmsg) from [<c0101000>] (ret_fast_syscall+0x0/0x28)
[ 31.613704] Exception stack(0xed447fa8 to 0xed447ff0)
[ 31.618756] 7fa0: 00093ee0 bec25144 00000003 bec25150 00000000 85ce0000
[ 31.626929] 7fc0: 00093ee0 bec25144 bec2d17c 00000128 000942a8 5afc783a 00094000 bec25150
[ 31.635099] 7fe0: 00000000 bec250f0 0000012c b6e10b5c
[ 31.640152] Code: e59a261c e59a013c e50b306c e592300c (e593300c)
[ 31.646632] ---[ end trace 407964b7deb937bf ]---
For the moment, I stil didnt find the issue.
What to we do now ? do you want that I send revert patchs ?
Regards
^ permalink raw reply
* Re: [PATCH net-next v2 0/2] of: mdio: Fall back to mdiobus_register() with NULL device_node
From: Geert Uytterhoeven @ 2018-05-16 18:32 UTC (permalink / raw)
To: David Miller
Cc: Florian Fainelli, netdev, Andrew Lunn, Vivien Didelot,
Nicolas Ferre, Fugang Duan, Sergei Shtylyov, Giuseppe Cavallaro,
Alexandre Torgue, Jose Abreu, Grygorii Strashko, Woojung Huh,
Microchip Linux Driver Support, Rob Herring, Frank Rowand,
Antoine Ténart, Tobias Jordan
In-Reply-To: <20180516.142102.1650229402143291732.davem@davemloft.net>
Hi David,
On Wed, May 16, 2018 at 8:21 PM, David Miller <davem@davemloft.net> wrote:
> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Tue, 15 May 2018 16:56:17 -0700
>
>> This patch series updates of_mdiobus_register() such that when the device_node
>> argument is NULL, it calls mdiobus_register() directly. This is consistent with
>> the behavior of of_mdiobus_register() when CONFIG_OF=n.
>>
>> I only converted the most obvious drivers, there are others that have a much
>> less obvious behavior and specifically attempt to deal with CONFIG_ACPI.
>>
>> Changes in v2:
>>
>> - fixed build error in davincin_mdio.c (Grygorii)
>> - reworked first patch a bit: commit message, subject and removed useless
>> code comment
>
> Based upon Andrew's response to Geert's feedback, I'm applying this series.
Thanks, his feedback made perfect sense.
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] net: qcom/emac: Encapsulate sgmii ops under one structure
From: David Miller @ 2018-05-16 18:33 UTC (permalink / raw)
To: hpuranik; +Cc: netdev, linux-kernel, timur
In-Reply-To: <1526433053-16308-1-git-send-email-hpuranik@codeaurora.org>
From: Hemanth Puranik <hpuranik@codeaurora.org>
Date: Wed, 16 May 2018 06:40:53 +0530
> This patch introduces ops structure for sgmii, This by ensures that
> we do not need dummy functions in case of emulation platforms.
>
> Signed-off-by: Hemanth Puranik <hpuranik@codeaurora.org>
Applied to net-next.
^ permalink raw reply
* Re: [PATCH net-next v11 1/7] sched: Add Common Applications Kept Enhanced (cake) qdisc
From: Toke Høiland-Jørgensen @ 2018-05-16 18:34 UTC (permalink / raw)
To: David Miller; +Cc: netdev, cake
In-Reply-To: <20180516.140203.2185491961233997679.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
> From: Toke Høiland-Jørgensen <toke@toke.dk>
> Date: Tue, 15 May 2018 17:12:44 +0200
>
>> +typedef u64 cobalt_time_t;
>> +typedef s64 cobalt_tdiff_t;
> ...
>> +static cobalt_time_t cobalt_get_time(void)
>> +{
>> + return ktime_get_ns();
>> +}
>> +
>> +static u32 cobalt_time_to_us(cobalt_time_t val)
>> +{
>> + do_div(val, NSEC_PER_USEC);
>> + return (u32)val;
>> +}
>
> If fundamentally you are working with ktime_t values, please use that type
> and the associated helpers.
>
> This is a valid argument that using custom typedefs provide documentation
> and an aid to understanding, but I think it doesn't serve that purpose
> very well here.
>
> So please just use ktime_t throughout instead of this cobalt_time_t
> and cobalt_tdiff_t. And then use helpers like ktime_to_us() which
> properly optimize for 64-bit vs. 32-bit hosts.
Can do :)
-Toke
^ permalink raw reply
* Re: [PATCH net-next v3 1/3] ipv4: support sport, dport and ip_proto in RTM_GETROUTE
From: David Miller @ 2018-05-16 18:37 UTC (permalink / raw)
To: roopa; +Cc: netdev, dsa, nikolay, idosch
In-Reply-To: <1526442908-13183-2-git-send-email-roopa@cumulusnetworks.com>
From: Roopa Prabhu <roopa@cumulusnetworks.com>
Date: Tue, 15 May 2018 20:55:06 -0700
> +static int inet_rtm_getroute_reply(struct sk_buff *in_skb, struct nlmsghdr *nlh,
> + __be32 dst, __be32 src, struct flowi4 *fl4,
> + struct rtable *rt, struct fib_result *res)
> +{
> + struct net *net = sock_net(in_skb->sk);
> + struct rtmsg *rtm = nlmsg_data(nlh);
> + u32 table_id = RT_TABLE_MAIN;
> + struct sk_buff *skb;
> + int err = 0;
> +
> + skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> + if (!skb)
> + return -ENOMEM;
If the caller can use GFP_KERNEL, so can this allocation.
^ permalink raw reply
* Re: [PATCH] net: 8390: ne: Fix accidentally removed RBTX4927 support
From: David Miller @ 2018-05-16 18:40 UTC (permalink / raw)
To: geert; +Cc: arnd, linux, anemo, netdev, linux-mips, linux-kernel
In-Reply-To: <1526462281-20772-1-git-send-email-geert@linux-m68k.org>
From: Geert Uytterhoeven <geert@linux-m68k.org>
Date: Wed, 16 May 2018 11:18:01 +0200
> The configuration settings for RBTX4927 were accidentally removed,
> leading to a silently broken network interface.
>
> Re-add the missing settings to fix this.
>
> Fixes: 8eb97ff5a4ec941d ("net: 8390: remove m32r specific bits")
> Signed-off-by: Geert Uytterhoeven <geert@linux-m68k.org>
> ---
> Bisected between v4.9-rc2 (doh) and v4.17-rc5.
>
> Note to myself: I should do more boot testing on RBTX4927.
> Fortunately I caught it before it ends up in a point release ;-)
Better to go on a wild bisect ride than not be able to find the
bug at all :-)
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net] net/sched: fix refcnt leak in the error path of tcf_vlan_init()
From: David Miller @ 2018-05-16 18:42 UTC (permalink / raw)
To: dcaratti; +Cc: netdev, jiri, jhs, mrv
In-Reply-To: <b2e7cfab172461a824959d683d8c868fe04119a3.1526467698.git.dcaratti@redhat.com>
From: Davide Caratti <dcaratti@redhat.com>
Date: Wed, 16 May 2018 12:54:29 +0200
> Similarly to what was done with commit a52956dfc503 ("net sched actions:
> fix refcnt leak in skbmod"), fix the error path of tcf_vlan_init() to avoid
> refcnt leaks when wrong value of TCA_VLAN_PUSH_VLAN_PROTOCOL is given.
>
> Fixes: 5026c9b1bafc ("net sched: vlan action fix late binding")
> CC: Roman Mashak <mrv@mojatatu.com>
> Signed-off-by: Davide Caratti <dcaratti@redhat.com>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net-next] net: stmmac: Remove useless test before clk_disable_unprepare
From: David Miller @ 2018-05-16 18:44 UTC (permalink / raw)
To: yuehaibing; +Cc: peppe.cavallaro, netdev, linux-kernel, alexandre.torgue
In-Reply-To: <20180516105919.25436-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Wed, 16 May 2018 18:59:19 +0800
> clk_disable_unprepare() already checks that the clock pointer is valid.
> No need to test it before calling it.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: ethoc: Remove useless test before clk_disable_unprepare
From: David Miller @ 2018-05-16 18:47 UTC (permalink / raw)
To: yuehaibing; +Cc: tklauser, netdev, linux-kernel
In-Reply-To: <20180516111822.26440-1-yuehaibing@huawei.com>
From: YueHaibing <yuehaibing@huawei.com>
Date: Wed, 16 May 2018 19:18:22 +0800
> clk_disable_unprepare() already checks that the clock pointer is valid.
> No need to test it before calling it.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH net] net/ipv4: Initialize proto and ports in flow struct
From: David Ahern @ 2018-05-16 18:48 UTC (permalink / raw)
To: netdev; +Cc: roopa
In-Reply-To: <20180516181834.31915-1-dsahern@gmail.com>
On 5/16/18 12:18 PM, David Ahern wrote:
> @@ -347,8 +348,11 @@ static int __fib_validate_source(struct sk_buff *skb, __be32 src, __be32 dst,
> no_addr = idev->ifa_list == NULL;
>
> fl4.flowi4_mark = IN_DEV_SRC_VMARK(idev) ? skb->mark : 0;
> -
> - trace_fib_validate_source(dev, &fl4);
Looks like I sent the net-next version; did not mean to remove that
tracepoint in net. Will send v2
> + if (!fib4_rules_early_flow_dissect(net, skb, &fl4, &flkeys)) {
> + fl4.flowi4_proto = 0;
> + fl4.fl4_sport = 0;
> + fl4.fl4_dport = 0;
> + }
>
> if (fib_lookup(net, &fl4, &res, 0))
> goto last_resort;
^ permalink raw reply
* Re: [PATCH net 0/3] qed: LL2 fixes
From: David Miller @ 2018-05-16 18:50 UTC (permalink / raw)
To: Michal.Kalderon; +Cc: netdev, chad.dupuis, Ariel.Elior
In-Reply-To: <20180516114440.5048-1-Michal.Kalderon@cavium.com>
From: Michal Kalderon <Michal.Kalderon@cavium.com>
Date: Wed, 16 May 2018 14:44:37 +0300
> This series fixes some issues in ll2 related to synchronization
> and resource freeing
>
> Signed-off-by: Ariel Elior <Ariel.Elior@cavium.com>
> Signed-off-by: Michal Kalderon <Michal.Kalderon@cavium.com>
Series applied and queued up for -stable, thank you.
^ permalink raw reply
* [PATCH net-next v2 1/3] net: ethernet: ti: Allow most drivers with COMPILE_TEST
From: Florian Fainelli @ 2018-05-16 18:52 UTC (permalink / raw)
To: netdev
Cc: fugang.duan, Florian Fainelli, David S. Miller, Andrew Lunn,
open list
In-Reply-To: <20180516185258.20508-1-f.fainelli@gmail.com>
Most of the TI drivers build just fine with COMPILE_TEST, cpmac (AR7) is
the exception because it uses a header file from
arch/mips/include/asm/mach-ar7/ar7.h.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/ti/Kconfig | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/ti/Kconfig b/drivers/net/ethernet/ti/Kconfig
index 48a541eb0af2..1f8626999ac6 100644
--- a/drivers/net/ethernet/ti/Kconfig
+++ b/drivers/net/ethernet/ti/Kconfig
@@ -18,7 +18,7 @@ if NET_VENDOR_TI
config TI_DAVINCI_EMAC
tristate "TI DaVinci EMAC Support"
- depends on ARM && ( ARCH_DAVINCI || ARCH_OMAP3 )
+ depends on ARM && ( ARCH_DAVINCI || ARCH_OMAP3 ) || COMPILE_TEST
select TI_DAVINCI_MDIO
select TI_DAVINCI_CPDMA
select PHYLIB
@@ -30,7 +30,7 @@ config TI_DAVINCI_EMAC
config TI_DAVINCI_MDIO
tristate "TI DaVinci MDIO Support"
- depends on ARCH_DAVINCI || ARCH_OMAP2PLUS || ARCH_KEYSTONE
+ depends on ARCH_DAVINCI || ARCH_OMAP2PLUS || ARCH_KEYSTONE || COMPILE_TEST
select PHYLIB
---help---
This driver supports TI's DaVinci MDIO module.
@@ -40,7 +40,7 @@ config TI_DAVINCI_MDIO
config TI_DAVINCI_CPDMA
tristate "TI DaVinci CPDMA Support"
- depends on ARCH_DAVINCI || ARCH_OMAP2PLUS
+ depends on ARCH_DAVINCI || ARCH_OMAP2PLUS || COMPILE_TEST
---help---
This driver supports TI's DaVinci CPDMA dma engine.
@@ -60,7 +60,7 @@ config TI_CPSW_ALE
config TI_CPSW
tristate "TI CPSW Switch Support"
- depends on ARCH_DAVINCI || ARCH_OMAP2PLUS
+ depends on ARCH_DAVINCI || ARCH_OMAP2PLUS || COMPILE_TEST
select TI_DAVINCI_CPDMA
select TI_DAVINCI_MDIO
select TI_CPSW_PHY_SEL
@@ -75,7 +75,7 @@ config TI_CPSW
config TI_CPTS
bool "TI Common Platform Time Sync (CPTS) Support"
- depends on TI_CPSW || TI_KEYSTONE_NETCP
+ depends on TI_CPSW || TI_KEYSTONE_NETCP || COMPILE_TEST
depends on POSIX_TIMERS
---help---
This driver supports the Common Platform Time Sync unit of
@@ -95,8 +95,8 @@ config TI_KEYSTONE_NETCP
tristate "TI Keystone NETCP Core Support"
select TI_CPSW_ALE
select TI_DAVINCI_MDIO
- depends on OF
- depends on KEYSTONE_NAVIGATOR_DMA && KEYSTONE_NAVIGATOR_QMSS
+ depends on OF && KEYSTONE_NAVIGATOR_DMA && KEYSTONE_NAVIGATOR_QMSS || \
+ COMPILE_TEST
---help---
This driver supports TI's Keystone NETCP Core.
--
2.14.1
^ permalink raw reply related
* [PATCH net-next v2 2/3] net: ethernet: freescale: Allow FEC with COMPILE_TEST
From: Florian Fainelli @ 2018-05-16 18:52 UTC (permalink / raw)
To: netdev
Cc: fugang.duan, Florian Fainelli, David S. Miller, Andrew Lunn,
open list
In-Reply-To: <20180516185258.20508-1-f.fainelli@gmail.com>
The Freescale FEC driver builds fine with COMPILE_TEST, so make that
possible.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/ethernet/freescale/Kconfig | 2 +-
drivers/net/ethernet/freescale/fec.h | 2 +-
drivers/net/ethernet/freescale/fec_main.c | 2 +-
3 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/freescale/Kconfig b/drivers/net/ethernet/freescale/Kconfig
index 6e490fd2345d..a580a3dcbe59 100644
--- a/drivers/net/ethernet/freescale/Kconfig
+++ b/drivers/net/ethernet/freescale/Kconfig
@@ -22,7 +22,7 @@ if NET_VENDOR_FREESCALE
config FEC
tristate "FEC ethernet controller (of ColdFire and some i.MX CPUs)"
depends on (M523x || M527x || M5272 || M528x || M520x || M532x || \
- ARCH_MXC || SOC_IMX28)
+ ARCH_MXC || SOC_IMX28 || COMPILE_TEST)
default ARCH_MXC || SOC_IMX28 if ARM
select PHYLIB
imply PTP_1588_CLOCK
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index e7381f8ef89d..4778b663653e 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -21,7 +21,7 @@
#if defined(CONFIG_M523x) || defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM) || \
- defined(CONFIG_ARM64)
+ defined(CONFIG_ARM64) || defined(CONFIG_COMPILE_TEST)
/*
* Just figures, Motorola would have to change the offsets for
* registers in the same peripheral device on different models
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index d4604bc8eb5b..91e334b5a836 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2111,7 +2111,7 @@ static int fec_enet_get_regs_len(struct net_device *ndev)
/* List of registers that can be safety be read to dump them with ethtool */
#if defined(CONFIG_M523x) || defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM) || \
- defined(CONFIG_ARM64)
+ defined(CONFIG_ARM64) || defined(CONFIG_COMPILE_TEST)
static u32 fec_enet_register_offset[] = {
FEC_IEVENT, FEC_IMASK, FEC_R_DES_ACTIVE_0, FEC_X_DES_ACTIVE_0,
FEC_ECNTRL, FEC_MII_DATA, FEC_MII_SPEED, FEC_MIB_CTRLSTAT, FEC_R_CNTRL,
--
2.14.1
^ permalink raw reply related
* [PATCH net-next v2 3/3] net: phy: Allow MDIO_MOXART and MDIO_SUN4I with COMPILE_TEST
From: Florian Fainelli @ 2018-05-16 18:52 UTC (permalink / raw)
To: netdev
Cc: fugang.duan, Florian Fainelli, David S. Miller, Andrew Lunn,
open list
In-Reply-To: <20180516185258.20508-1-f.fainelli@gmail.com>
Those drivers build just fine with COMPILE_TEST, so make that possible.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
drivers/net/phy/Kconfig | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 0e2305ccc91f..343989f9f9d9 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -118,7 +118,7 @@ config MDIO_I2C
config MDIO_MOXART
tristate "MOXA ART MDIO interface support"
- depends on ARCH_MOXART
+ depends on ARCH_MOXART || COMPILE_TEST
help
This driver supports the MDIO interface found in the network
interface units of the MOXA ART SoC
@@ -142,7 +142,7 @@ config MDIO_OCTEON
config MDIO_SUN4I
tristate "Allwinner sun4i MDIO interface support"
- depends on ARCH_SUNXI
+ depends on ARCH_SUNXI || COMPILE_TEST
help
This driver supports the MDIO interface found in the network
interface units of the Allwinner SoC that have an EMAC (A10,
--
2.14.1
^ permalink raw reply related
* [PATCH net-next v2 0/3] net: Allow more drivers with COMPILE_TEST
From: Florian Fainelli @ 2018-05-16 18:52 UTC (permalink / raw)
To: netdev
Cc: fugang.duan, Florian Fainelli, David S. Miller, Andrew Lunn,
open list
Hi David,
This patch series includes more drivers to be build tested with COMPILE_TEST
enabled. This helps cover some of the issues I just ran into with missing
a driver *sigh*.
Changes in v2:
- allow FEC to build outside of CONFIG_ARM/ARM64 by defining a layout of
registers, this is not meant to run, so this is not a real issue if we
are not matching the correct register layout
Florian Fainelli (3):
net: ethernet: ti: Allow most drivers with COMPILE_TEST
net: ethernet: freescale: Allow FEC with COMPILE_TEST
net: phy: Allow MDIO_MOXART and MDIO_SUN4I with COMPILE_TEST
drivers/net/ethernet/freescale/Kconfig | 2 +-
drivers/net/ethernet/freescale/fec.h | 2 +-
drivers/net/ethernet/freescale/fec_main.c | 2 +-
drivers/net/ethernet/ti/Kconfig | 14 +++++++-------
drivers/net/phy/Kconfig | 4 ++--
5 files changed, 12 insertions(+), 12 deletions(-)
--
2.14.1
^ permalink raw reply
* Re: [PATCH net] tuntap: fix use after free during release
From: David Miller @ 2018-05-16 18:53 UTC (permalink / raw)
To: jasowang; +Cc: netdev, linux-kernel, xiyou.wangcong, avagin, mst
In-Reply-To: <1526474373-16685-1-git-send-email-jasowang@redhat.com>
From: Jason Wang <jasowang@redhat.com>
Date: Wed, 16 May 2018 20:39:33 +0800
> After commit b196d88aba8a ("tun: fix use after free for ptr_ring") we
> need clean up tx ring during release(). But unfortunately, it tries to
> do the cleanup blindly after socket were destroyed which will lead
> another use-after-free. Fix this by doing the cleanup before dropping
> the last reference of the socket in __tun_detach().
>
> Reported-by: Andrei Vagin <avagin@virtuozzo.com>
> Acked-by: Andrei Vagin <avagin@virtuozzo.com>
> Fixes: b196d88aba8a ("tun: fix use after free for ptr_ring")
> Signed-off-by: Jason Wang <jasowang@redhat.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [RFC PATCH bpf-next 12/12] i40e: implement Tx zero-copy
From: Jesper Dangaard Brouer @ 2018-05-16 18:53 UTC (permalink / raw)
To: Magnus Karlsson
Cc: Björn Töpel, Karlsson, Magnus, Alexander Duyck,
Alexander Duyck, John Fastabend, Alexei Starovoitov,
Willem de Bruijn, Daniel Borkmann, Michael S. Tsirkin,
Network Development, michael.lundkvist, Brandeburg, Jesse,
Singhai, Anjali, Zhang, Qi Z, intel-wired-lan, brouer
In-Reply-To: <CAJ8uoz2K+5WR37yoNcEsZhC9JyPbb4CuxoFbpdQBkAccnG+Row@mail.gmail.com>
On Wed, 16 May 2018 17:38:12 +0200
Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
> On Wed, May 16, 2018 at 4:38 PM, Magnus Karlsson
> <magnus.karlsson@gmail.com> wrote:
> > On Wed, May 16, 2018 at 4:28 PM, Jesper Dangaard Brouer
> > <brouer@redhat.com> wrote:
> >> On Tue, 15 May 2018 21:06:15 +0200
> >> Björn Töpel <bjorn.topel@gmail.com> wrote:
> >>
> >>> From: Magnus Karlsson <magnus.karlsson@intel.com>
> >>>
> >>> Here, the zero-copy ndo is implemented. As a shortcut, the existing
> >>> XDP Tx rings are used for zero-copy. This means that and XDP program
> >>> cannot redirect to an AF_XDP enabled XDP Tx ring.
> >>
> >> I've changed i40e1 to only have one queue via:
> >> $ ethtool -L i40e1 combined 1
> >>
> >> And then, I'm sending on queue 1, which is/should not be avail... and then crash/BUG:
> >>
> >> $ sudo taskset -c 2 ./xdpsock --tx --interface=i40e1 --queue=1
> >>
> >> [ 3799.936877] Number of in use tx queues changed invalidating tc mappings. Priority traffic
> >> classification disabled!
> >> [ 3799.972970] BUG: unable to handle kernel NULL pointer dereference at 0000000000000008
> >> [ 3799.980790] PGD 80000007b0992067 P4D 80000007b0992067 PUD 7b62d4067 PMD 0
> >> [ 3799.987654] Oops: 0002 [#1] PREEMPT SMP PTI
> >> [ 3799.991831] Modules linked in: nf_nat_masquerade_ipv4 tun nfnetlink bridge stp llc nf_nat
> >> nf_conntrack rpcrdma ib_ipoib rdma_ucm ib_ucm ib_umad rdma_cm ib_cm iw_cm sunrpc mlx5_ib ib
> >> _uverbs ib_core coretemp kvm_intel kvm irqbypass intel_cstate intel_uncore intel_rapl_perf p
> >> cspkr i2c_i801 shpchp wmi ipmi_si ipmi_devintf ipmi_msghandler acpi_pad sch_fq_codel i40e ml
> >> x5_core hid_generic ixgbe igb devlink mdio ptp sd_mod i2c_algo_bit i2c_core pps_core [last u
> >> nloaded: x_tables]
> >> [ 3800.033472] CPU: 2 PID: 2006 Comm: xdpsock Not tainted 4.17.0-rc3-af_xdp03_ZC_rfc+ #155
> >> [ 3800.041465] Hardware name: Supermicro Super Server/X10SRi-F, BIOS 2.0a 08/01/2016
> >> [ 3800.048943] RIP: 0010:i40e_xmit_frame_ring+0xd4/0x1490 [i40e]
> >> [ 3800.054683] RSP: 0018:ffffc9000407bcd0 EFLAGS: 00010293
> >> [ 3800.059900] RAX: 0000000000000000 RBX: ffff88084f0fd200 RCX: 0000000000000000
> >> [ 3800.067022] RDX: 0000000000000000 RSI: 0000000000000006 RDI: ffff8807b6e710c0
> >> [ 3800.074148] RBP: ffff8807c6397800 R08: 00000000000000c0 R09: 0000000000000001
> >> [ 3800.081270] R10: 0000000000000800 R11: 0000000000000010 R12: 0000000000000001
> >> [ 3800.088396] R13: 0000000000000000 R14: 0000000000000001 R15: 000000000000003c
> >> [ 3800.095520] FS: 00007f1d1e00bb80(0000) GS:ffff88087fc80000(0000) knlGS:0000000000000000
> >> [ 3800.103597] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> >> [ 3800.109335] CR2: 0000000000000008 CR3: 000000087d542001 CR4: 00000000003606e0
> >> [ 3800.116458] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> >> [ 3800.123583] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> >> [ 3800.130706] Call Trace:
> >> [ 3800.133157] ? validate_xmit_skb.isra.116+0x1c/0x270
> >> [ 3800.138118] dev_direct_xmit+0xec/0x1d0
> >> [ 3800.141949] xsk_sendmsg+0x1f4/0x380
> >> [ 3800.145521] sock_sendmsg+0x30/0x40
> >> [ 3800.149005] __sys_sendto+0x10e/0x140
> >> [ 3800.152662] ? __do_page_fault+0x283/0x500
> >> [ 3800.156751] __x64_sys_sendto+0x24/0x30
> >> [ 3800.160585] do_syscall_64+0x42/0xf0
> >> [ 3800.164156] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> >> [ 3800.169204] RIP: 0033:0x7f1d1d9db430
> >> [ 3800.172774] RSP: 002b:00007fffb7278610 EFLAGS: 00000293 ORIG_RAX: 000000000000002c
> >> [ 3800.180333] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f1d1d9db430
> >> [ 3800.187456] RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000003
> >> [ 3800.194582] RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
> >> [ 3800.201705] R10: 0000000000000040 R11: 0000000000000293 R12: 0000000000000000
> >> [ 3800.208830] R13: 0000000000000000 R14: 0000000000755510 R15: 00007f1d0d3fc000
> >> [ 3800.215953] Code: d0 0f 86 db 05 00 00 01 c8 0f b7 ca 29 c8 83 e8 01 39 c6 0f 8f ea 06 00 00 48 8b 45 28 48 8d 14 92 41 b9 01 00 00 00 4c 8d 2c d0 <49> 89 5d 08 8b 83 80 00 00 00 66 45 89 4d 14 41 89 45 10 0f b7
> >> [ 3800.234798] RIP: i40e_xmit_frame_ring+0xd4/0x1490 [i40e] RSP: ffffc9000407bcd0
> >> [ 3800.242005] CR2: 0000000000000008
> >> [ 3800.245320] ---[ end trace f169e36f468e0c59 ]---
> >> [ 3801.726719] Kernel panic - not syncing: Fatal exception in interrupt
> >> [ 3801.733097] Kernel Offset: disabled
> >> [ 3801.785836] ---[ end Kernel panic - not syncing: Fatal exception in interrupt ]---
> >> [ 3801.793403] ------------[ cut here ]------------
> >>
> >> (gdb) list *(i40e_xmit_frame_ring)+0xd4
> >> 0x2ccd4 is in i40e_xmit_frame_ring (drivers/net/ethernet/intel/i40e/i40e_txrx.c:4048).
> >> warning: Source file is more recent than executable.
> >> 4043 return NETDEV_TX_BUSY;
> >> 4044 }
> >> 4045
> >> 4046 /* record the location of the first descriptor for this packet */
> >> 4047 first = &tx_ring->tx_bi[tx_ring->next_to_use];
> >> 4048 first->skb = skb;
> >> 4049 first->bytecount = skb->len;
> >> 4050 first->gso_segs = 1;
> >> 4051
> >> 4052 /* prepare the xmit flags */
> >>
> >>
> >> (gdb) list *(xsk_sendmsg)+0x1f4
> >> 0xffffffff81800c34 is in xsk_sendmsg (net/xdp/xsk.c:251).
> >> warning: Source file is more recent than executable.
> >> 246 skb_shinfo(skb)->destructor_arg = (void *)(long)id;
> >> 247 skb->destructor = xsk_destruct_skb;
> >> 248
> >> 249 err = dev_direct_xmit(skb, xs->queue_id);
> >> 250 /* Ignore NET_XMIT_CN as packet might have been sent */
> >> 251 if (err == NET_XMIT_DROP || err == NETDEV_TX_BUSY) {
> >> 252 err = -EAGAIN;
> >> 253 /* SKB consumed by dev_direct_xmit() */
> >> 254 goto out;
> >> 255 }
> >
>
> Found it. Checked num_rx_queues in the xsk_bind code instead of
> real_num_rx_queues. The code below will solve the problem. Will post a
> proper patch for it tomorrow. Thanks Jesper for reporting this.
> Appreciated.
>
> /Magnus
>
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index ac97902..4b62a1e 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -391,7 +391,8 @@ static int xsk_bind(struct socket *sock, struct
> sockaddr *addr, int addr_len)
> goto out_unlock;
> }
>
> - if (sxdp->sxdp_queue_id >= dev->num_rx_queues) {
> + if ((xs->rx && sxdp->sxdp_queue_id >= dev->real_num_rx_queues) ||
> + (xs->tx && sxdp->sxdp_queue_id >= dev->real_num_tx_queues)) {
> err = -EINVAL;
> goto out_unlock;
> }
Tried this patch... it fixed/catched the problem :-)
$ sudo ./xdpsock --interface=i40e1 --queue=42 --txonly
samples/bpf/xdpsock_user.c:xsk_configure:528: Assertion failed: bind(sfd, (struct sockaddr *)&sxdp, sizeof(sxdp)) == 0: errno: 22/"Invalid argument"
Segmentation fault
You can add:
Tested-by: Jesper Dangaard Brouer <brouer@redhat.com>
Notice: this bug is not related to this zero-copy patch, but to your
previous patchset, which are in bpf-next. Thus, you need to send a fix
patch to bpf-next...
--
Best regards,
Jesper Dangaard Brouer
MSc.CS, Principal Kernel Engineer at Red Hat
LinkedIn: http://www.linkedin.com/in/brouer
^ permalink raw reply
* Re: [RFC bpf-next 07/11] bpf: Add helper to retrieve socket in BPF
From: Joe Stringer @ 2018-05-16 18:55 UTC (permalink / raw)
To: Martin KaFai Lau
Cc: Alexei Starovoitov, Joe Stringer, daniel, netdev, ast,
john fastabend
In-Reply-To: <20180515164801.lp5oyabhnukagczc@kafai-mbp.dhcp.thefacebook.com>
On 15 May 2018 at 09:48, Martin KaFai Lau <kafai@fb.com> wrote:
> On Mon, May 14, 2018 at 08:16:59PM -0700, Alexei Starovoitov wrote:
>> On Fri, May 11, 2018 at 05:54:33PM -0700, Joe Stringer wrote:
>> > On 11 May 2018 at 14:41, Martin KaFai Lau <kafai@fb.com> wrote:
>> > > On Fri, May 11, 2018 at 02:08:01PM -0700, Joe Stringer wrote:
>> > >> On 10 May 2018 at 22:00, Martin KaFai Lau <kafai@fb.com> wrote:
>> > >> > On Wed, May 09, 2018 at 02:07:05PM -0700, Joe Stringer wrote:
>> > >> >> This patch adds a new BPF helper function, sk_lookup() which allows BPF
>> > >> >> programs to find out if there is a socket listening on this host, and
>> > >> >> returns a socket pointer which the BPF program can then access to
>> > >> >> determine, for instance, whether to forward or drop traffic. sk_lookup()
>> > >> >> takes a reference on the socket, so when a BPF program makes use of this
>> > >> >> function, it must subsequently pass the returned pointer into the newly
>> > >> >> added sk_release() to return the reference.
>> > >> >>
>> > >> >> By way of example, the following pseudocode would filter inbound
>> > >> >> connections at XDP if there is no corresponding service listening for
>> > >> >> the traffic:
>> > >> >>
>> > >> >> struct bpf_sock_tuple tuple;
>> > >> >> struct bpf_sock_ops *sk;
>> > >> >>
>> > >> >> populate_tuple(ctx, &tuple); // Extract the 5tuple from the packet
>> > >> >> sk = bpf_sk_lookup(ctx, &tuple, sizeof tuple, netns, 0);
>> > >> >> if (!sk) {
>> > >> >> // Couldn't find a socket listening for this traffic. Drop.
>> > >> >> return TC_ACT_SHOT;
>> > >> >> }
>> > >> >> bpf_sk_release(sk, 0);
>> > >> >> return TC_ACT_OK;
>> > >> >>
>> > >> >> Signed-off-by: Joe Stringer <joe@wand.net.nz>
>> > >> >> ---
>> > >>
>> > >> ...
>> > >>
>> > >> >> @@ -4032,6 +4036,96 @@ static const struct bpf_func_proto bpf_skb_get_xfrm_state_proto = {
>> > >> >> };
>> > >> >> #endif
>> > >> >>
>> > >> >> +struct sock *
>> > >> >> +sk_lookup(struct net *net, struct bpf_sock_tuple *tuple) {
>> > >> > Would it be possible to have another version that
>> > >> > returns a sk without taking its refcnt?
>> > >> > It may have performance benefit.
>> > >>
>> > >> Not really. The sockets are not RCU-protected, and established sockets
>> > >> may be torn down without notice. If we don't take a reference, there's
>> > >> no guarantee that the socket will continue to exist for the duration
>> > >> of running the BPF program.
>> > >>
>> > >> From what I follow, the comment below has a hidden implication which
>> > >> is that sockets without SOCK_RCU_FREE, eg established sockets, may be
>> > >> directly freed regardless of RCU.
>> > > Right, SOCK_RCU_FREE sk is the one I am concern about.
>> > > For example, TCP_LISTEN socket does not require taking a refcnt
>> > > now. Doing a bpf_sk_lookup() may have a rather big
>> > > impact on handling TCP syn flood. or the usual intention
>> > > is to redirect instead of passing it up to the stack?
>> >
>> > I see, if you're only interested in listen sockets then probably this
>> > series could be extended with a new flag, eg something like
>> > BPF_F_SK_FIND_LISTENERS which restricts the set of possible sockets
>> > found to only listen sockets, then the implementation would call into
>> > __inet_lookup_listener() instead of inet_lookup(). The presence of
>> > that flag in the relevant register during CALL instruction would show
>> > that the verifier should not reference-track the result, then there'd
>> > need to be a check on the release to ensure that this unreferenced
>> > socket is never released. Just a thought, completely untested and I
>> > could still be missing some detail..
>> >
>> > That said, I don't completely follow how you would expect to handle
>> > the traffic for sockets that are already established - the helper
>> > would no longer find those sockets, so you wouldn't know whether to
>> > pass the traffic up the stack for established traffic or not.
>>
>> I think Martin has a valid concern here that if somebody starts using
>> this helper on the rx traffic the bpf program (via these two new
>> helpers) will be doing refcnt++ and refcnt-- even for listener
>> sockets which will cause synflood to suffer.
>> One can argue that this is bpf author mistake, but without fixes
>> (and api changes) to the helper the programmer doesn't really have a way
>> of avoiding this situation.
>> Also udp sockets don't need refcnt at all.
>> How about we split this single helper into three:
>> - bpf_sk_lookup_tcp_established() that will return refcnt-ed socket
>> and has to be bpf_sk_release()d by the program.
>> - bpf_sk_lookup_tcp_listener() that doesn't refcnt, since progs
>> run in rcu.
>> - bpf_sk_lookup_udp() that also doesn't refcnt.
>> The logic you want to put into this helper can be easily
>> replicated with these three helpers and the whole thing will
>> be much faster.
>> Thoughts?
> Just came to my mind.
>
> or can we explore something like:
>
> On the bpf_sk_lookup() side, use __inet[6]_lookup()
> and __udp[46]_lib_lookup() instead. That should
> only take refcnt if it has to.
>
> On the bpf_sk_release() side, it skips refcnt--
> if sk is SOCK_RCU_FREE.
Reflecting the discussion from IOVisor call:
I voiced a concern with the above proposal by Alexei that it leaks
kernel implementation detail (established sockets are refcnted) into
the BPF API.
Martin's proposal here addresses this concern. We can force all
sk_lookup()s to match with a bpf_sk_release(), then inside the
bpf_sk_release() we can deal with the details of whether any freeing
is actually required.
It's still useful to split the helpers out into bpf_sk_lookup_tcp()
and bpf_sk_lookup_udp() because then we don't need to deal with the
forward-compatibility concern of adding support for different socket
types (eg SCTP). That said, the TCP established/listener split does
not have an immediate user, so we don't need to split these at this
time. If there is a use case for only finding the listener sockets, we
can always add a flag to the bpf_sk_lookup_tcp() helper to only find
the listener sockets.
I'll respin, thanks for the feedback all.
^ permalink raw reply
* Re: [PATCH v2 net-next 00/12] net: stmmac: Clean-up and tune-up
From: David Miller @ 2018-05-16 18:56 UTC (permalink / raw)
To: Jose.Abreu
Cc: netdev, Joao.Pinto, Vitor.Soares, peppe.cavallaro,
alexandre.torgue
In-Reply-To: <cover.1526474646.git.joabreu@synopsys.com>
From: Jose Abreu <Jose.Abreu@synopsys.com>
Date: Wed, 16 May 2018 13:50:42 +0100
> David raised some rightfull constrains about the use of indirect callbacks in
> the code. I did iperf tests with and without patches 3-12 and the performance
> remained equal. I guess for 1Gb/s and because my setup has a powerfull
> processor these patches don't affect the performance.
Does your cpu need Spectre v1 and v2 workarounds which cause indirect calls to
be extremely expensive?
That's the case I'm worried about.
^ permalink raw reply
* Re: [PATCH net-next V2 2/2] cxgb4: Support ethtool private flags
From: David Miller @ 2018-05-16 19:00 UTC (permalink / raw)
To: ganeshgr; +Cc: netdev, nirranjan, indranil, venkatesh, arjun, leedom
In-Reply-To: <1526476284-6496-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Wed, 16 May 2018 18:41:24 +0530
> From: Arjun Vynipadath <arjun@chelsio.com>
>
> This is used to change TX workrequests, which helps in
> host->vf communication.
>
> Signed-off-by: Arjun Vynipadath <arjun@chelsio.com>
> Signed-off-by: Casey Leedom <leedom@chelsio.com>
> Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
> ---
> V2: no v2 changes in this patch
I don't know about this.
Perhaps find some way to high level abstract what this thing is so that
if another chip has the same kind of control they can use the same
generic ethtool flag/setting?
Thank you.
^ permalink raw reply
* Re: [PATCH net-next V2 1/2] cxgb4: Add support for FW_ETH_TX_PKT_VM_WR
From: David Miller @ 2018-05-16 19:00 UTC (permalink / raw)
To: ganeshgr; +Cc: netdev, nirranjan, indranil, venkatesh, arjun, leedom
In-Reply-To: <1526476247-6452-1-git-send-email-ganeshgr@chelsio.com>
From: Ganesh Goudar <ganeshgr@chelsio.com>
Date: Wed, 16 May 2018 18:40:47 +0530
> @@ -554,6 +563,7 @@ struct port_info {
> struct hwtstamp_config tstamp_config;
> bool ptp_enable;
> struct sched_table *sched_tbl;
> + u32 eth_flags;
> };
>
> struct dentry;
> @@ -864,6 +874,7 @@ struct adapter {
> unsigned int flags;
> unsigned int adap_idx;
> enum chip_type chip;
> + unsigned int eth_flags;
Both of these new struct members should use the same type.
But see my feedback for patch #2 first.
^ 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