* [PATCH net-next] net: mscc: ocelot: remove set but not used variable 'phy_mode'
From: YueHaibing @ 2018-10-08 14:07 UTC (permalink / raw)
To: Alexandre Belloni, David S. Miller; +Cc: YueHaibing, netdev, kernel-janitors
Fixes gcc '-Wunused-but-set-variable' warning:
drivers/net/ethernet/mscc/ocelot_board.c: In function 'mscc_ocelot_probe':
drivers/net/ethernet/mscc/ocelot_board.c:262:17: warning:
variable 'phy_mode' set but not used [-Wunused-but-set-variable]
enum phy_mode phy_mode;
It never used since introduction in
commit 71e32a20cfbf ("net: mscc: ocelot: make use of SerDes PHYs for handling their configuration")
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/mscc/ocelot_board.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/ethernet/mscc/ocelot_board.c b/drivers/net/ethernet/mscc/ocelot_board.c
index 953b326..0cf0b09 100644
--- a/drivers/net/ethernet/mscc/ocelot_board.c
+++ b/drivers/net/ethernet/mscc/ocelot_board.c
@@ -259,7 +259,6 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
struct phy_device *phy;
struct resource *res;
struct phy *serdes;
- enum phy_mode phy_mode;
void __iomem *regs;
char res_name[8];
u32 port;
@@ -297,10 +296,8 @@ static int mscc_ocelot_probe(struct platform_device *pdev)
case PHY_INTERFACE_MODE_NA:
continue;
case PHY_INTERFACE_MODE_SGMII:
- phy_mode = PHY_MODE_SGMII;
break;
case PHY_INTERFACE_MODE_QSGMII:
- phy_mode = PHY_MODE_QSGMII;
break;
default:
dev_err(ocelot->dev,
^ permalink raw reply related
* Re: [PATCH net-next 04/19] net: usb: aqc111: Various callbacks implementation
From: Oliver Neukum @ 2018-10-08 13:47 UTC (permalink / raw)
To: Igor Russkikh, David S . Miller
Cc: Dmitry Bezrukov, linux-usb@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <ee389e525cfde23ab7dcdb0bd748915dd387d552.1538734658.git.igor.russkikh@aquantia.com>
On Fr, 2018-10-05 at 10:24 +0000, Igor Russkikh wrote:
> From: Dmitry Bezrukov <dmitry.bezrukov@aquantia.com>
>
> Reset, stop callbacks, driver unbind callback.
> More register defines required for these callbacks.
>
> Signed-off-by: Dmitry Bezrukov <dmitry.bezrukov@aquantia.com>
> Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
> ---
> drivers/net/usb/aqc111.c | 48 ++++++++++++++++++++++
> drivers/net/usb/aqc111.h | 101 +++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 149 insertions(+)
>
> diff --git a/drivers/net/usb/aqc111.c b/drivers/net/usb/aqc111.c
> index 7f3e5a615750..22bb259d71fb 100644
> --- a/drivers/net/usb/aqc111.c
> +++ b/drivers/net/usb/aqc111.c
> @@ -169,12 +169,60 @@ static int aqc111_bind(struct usbnet *dev, struct usb_interface *intf)
>
> static void aqc111_unbind(struct usbnet *dev, struct usb_interface *intf)
> {
> + u8 reg8;
> + u16 reg16;
> +
> + /* Force bz */
> + reg16 = SFR_PHYPWR_RSTCTL_BZ;
> + aqc111_write_cmd_nopm(dev, AQ_ACCESS_MAC, SFR_PHYPWR_RSTCTL,
> + 2, 2, ®16);
No, I am sorry, you are doing DMA on the kernel stack. That is not
allowed. These functions will all have to be fixed.
Regards
Oliver
^ permalink raw reply
* Re: [PATCH net-next 03/19] net: usb: aqc111: Add implementation of read and write commands
From: Oliver Neukum @ 2018-10-08 13:44 UTC (permalink / raw)
To: Igor Russkikh, David S . Miller
Cc: Dmitry Bezrukov, linux-usb@vger.kernel.org,
netdev@vger.kernel.org
In-Reply-To: <ebf0867900ae849581fbd20b52ee9e855e6345c8.1538734658.git.igor.russkikh@aquantia.com>
On Fr, 2018-10-05 at 10:24 +0000, Igor Russkikh wrote:
> From: Dmitry Bezrukov <dmitry.bezrukov@aquantia.com>
>
> Read/write command register defines and functions
>
> Signed-off-by: Dmitry Bezrukov <dmitry.bezrukov@aquantia.com>
> Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
> ---
> drivers/net/usb/aqc111.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++
> drivers/net/usb/aqc111.h | 19 ++++++++
> 2 files changed, 143 insertions(+)
> create mode 100644 drivers/net/usb/aqc111.h
>
> diff --git a/drivers/net/usb/aqc111.c b/drivers/net/usb/aqc111.c
> index c914e19387f2..7f3e5a615750 100644
> --- a/drivers/net/usb/aqc111.c
> +++ b/drivers/net/usb/aqc111.c
> @@ -14,6 +14,130 @@
> #include <linux/usb/cdc.h>
> #include <linux/usb/usbnet.h>
>
> +#include "aqc111.h"
> +
> +static int __aqc111_read_cmd(struct usbnet *dev, u8 cmd, u16 value,
> + u16 index, u16 size, void *data, int nopm)
> +{
> + int ret;
> + int (*fn)(struct usbnet *dev, u8 cmd, u8 reqtype, u16 value,
> + u16 index, void *data, u16 size);
> +
> + if (nopm)
> + fn = usbnet_read_cmd_nopm;
> + else
> + fn = usbnet_read_cmd;
If you really want to do this, pass the function.
> +
> + ret = fn(dev, cmd, USB_DIR_IN | USB_TYPE_VENDOR | USB_RECIP_DEVICE,
> + value, index, data, size);
> + if (size == 2)
> + le16_to_cpus(data);
That is incredibly dirty
> +
> + if (unlikely(ret < 0))
> + netdev_warn(dev->net,
> + "Failed to read(0x%x) reg index 0x%04x: %d\n",
> + cmd, index, ret);
> + return ret;
> +}
> +
> +static int aqc111_read_cmd_nopm(struct usbnet *dev, u8 cmd, u16 value,
> + u16 index, u16 size, void *data)
> +{
> + return __aqc111_read_cmd(dev, cmd, value, index, size, data, 1);
> +}
> +
> +static int aqc111_read_cmd(struct usbnet *dev, u8 cmd, u16 value,
> + u16 index, u16 size, void *data)
> +{
> + return __aqc111_read_cmd(dev, cmd, value, index, size, data, 0);
> +}
> +
> +static int __aq_write_cmd(struct usbnet *dev, u8 cmd, u8 reqtype,
> + u16 value, u16 index, const void *data, u16 size)
> +{
> + void *buf = NULL;
> + int err = -ENOMEM;
> +
> + netdev_dbg(dev->net,
> + "%s cmd=%#x reqtype=%#x value=%#x index=%#x size=%d\n",
> + __func__, cmd, reqtype, value, index, size);
> +
> + if (data) {
> + buf = kmemdup(data, size, GFP_KERNEL);
Under which contexts is this used?
Regards
Oliver
^ permalink raw reply
* Re: [PATCH 2/2] bpftool: Allow add linker flags via EXTRA_LDFLAGS variable
From: Song Liu @ 2018-10-08 21:02 UTC (permalink / raw)
To: Jiri Olsa; +Cc: Alexei Starovoitov, Daniel Borkmann, open list, Networking
In-Reply-To: <20181008082259.26742-2-jolsa@kernel.org>
On Mon, Oct 8, 2018 at 1:23 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding EXTRA_LDFLAGS allowing user to specify extra flags
> for LD_FLAGS variable. Also adding LDFLAGS to build command
> line.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> tools/bpf/bpftool/Makefile | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 5bfa07e3f2f7..dac7eff4c7e5 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -49,6 +49,9 @@ CFLAGS += -DBPFTOOL_VERSION='"$(BPFTOOL_VERSION)"'
> ifneq ($(EXTRA_CFLAGS),)
> CFLAGS += $(EXTRA_CFLAGS)
> endif
> +ifneq ($(EXTRA_LDFLAGS),)
> +LDFLAGS += $(EXTRA_LDFLAGS)
> +endif
>
> LIBS = -lelf -lbfd -lopcodes $(LIBBPF)
>
> @@ -94,7 +97,7 @@ $(OUTPUT)disasm.o: $(srctree)/kernel/bpf/disasm.c
> $(QUIET_CC)$(COMPILE.c) -MMD -o $@ $<
>
> $(OUTPUT)bpftool: $(OBJS) $(LIBBPF)
> - $(QUIET_LINK)$(CC) $(CFLAGS) -o $@ $^ $(LIBS)
> + $(QUIET_LINK)$(CC) $(CFLAGS) $(LDFLAGS) -o $@ $^ $(LIBS)
>
> $(OUTPUT)%.o: %.c
> $(QUIET_CC)$(COMPILE.c) -MMD -o $@ $<
> --
> 2.17.2
>
^ permalink raw reply
* Re: [PATCH v8 01/15] octeontx2-af: Add Marvell OcteonTX2 RVU AF driver
From: Sunil Kovvuri @ 2018-10-08 13:50 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Linux Netdev List, David S. Miller, linux-soc, Sunil Goutham
In-Reply-To: <CAK8P3a2vgBVon8JU=y-4upF0ggLYt7WL-DJxby-6MqaM0Uvrqg@mail.gmail.com>
On Mon, Oct 8, 2018 at 5:52 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Sun, Oct 7, 2018 at 4:59 PM <sunil.kovvuri@gmail.com> wrote:
>
> > --- /dev/null
> > +++ b/drivers/net/ethernet/marvell/octeontx2/Kconfig
> > @@ -0,0 +1,12 @@
> > +#
> > +# Marvell OcteonTX2 drivers configuration
> > +#
> > +
> > +config OCTEONTX2_AF
> > + tristate "Marvell OcteonTX2 RVU Admin Function driver"
> > + depends on ARM64 && PCI
>
> You should try to allow building it on x86 and other architectures, even though
> the driver won't be used there this helps get reports from static
> build infrastructure.
> You could use e.g.
>
> depends on (64BIT && COMPILE_TEST) || ARM64
> depends on PCI
>
> Arnd
Thanks for the suggestion.
But going forward we will have few arm64 assembly instructions used in
the driver.
So a x86 build will fail. And i have built code using sparse and
coverity as well before
submitting patches upstream.
Sunil.
^ permalink raw reply
* Re: [PATCH 1/2] bpftool: Allow to add compiler flags via EXTRA_CFLAGS variable
From: Song Liu @ 2018-10-08 21:02 UTC (permalink / raw)
To: Jiri Olsa; +Cc: Alexei Starovoitov, Daniel Borkmann, open list, Networking
In-Reply-To: <20181008082259.26742-1-jolsa@kernel.org>
On Mon, Oct 8, 2018 at 1:24 AM Jiri Olsa <jolsa@kernel.org> wrote:
>
> Adding EXTRA_CFLAGS allowing user to specify extra flags
> for CFLAGS variable.
>
> Signed-off-by: Jiri Olsa <jolsa@kernel.org>
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> tools/bpf/bpftool/Makefile | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/tools/bpf/bpftool/Makefile b/tools/bpf/bpftool/Makefile
> index 74288a2197ab..5bfa07e3f2f7 100644
> --- a/tools/bpf/bpftool/Makefile
> +++ b/tools/bpf/bpftool/Makefile
> @@ -46,6 +46,10 @@ CFLAGS += -DPACKAGE='"bpftool"' -D__EXPORTED_HEADERS__ \
> -I$(srctree)/tools/lib/bpf \
> -I$(srctree)/tools/perf
> CFLAGS += -DBPFTOOL_VERSION='"$(BPFTOOL_VERSION)"'
> +ifneq ($(EXTRA_CFLAGS),)
> +CFLAGS += $(EXTRA_CFLAGS)
> +endif
> +
> LIBS = -lelf -lbfd -lopcodes $(LIBBPF)
>
> INSTALL ?= install
> --
> 2.17.2
>
^ permalink raw reply
* Re: [PATCH net-next 14/19] net: usb: aqc111: Implement set_rx_mode callback
From: Igor Russkikh @ 2018-10-08 13:49 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S . Miller, linux-usb@vger.kernel.org,
netdev@vger.kernel.org, Dmitry Bezrukov
In-Reply-To: <20181006170343.GB6990@lunn.ch>
Hi Andrew,
>> +{
>> + struct usbnet *dev = netdev_priv(net);
>> + struct aqc111_data *aqc111_data = (struct aqc111_data *)dev->data[0];
>
>> + u8 *m_filter = ((u8 *)dev->data) + 12;
>
> Please could you explain is.
Oh, that was a legacy code, the idea is it used spare area in dev->data array to
keep permanently 8 bytes for mcast filter request.
But the truth is usbnet_write anyway preallocates the data buffer and copies the data in there.
Therefore its really better to just allocate m_filter on stack to make the code clean.
Regards,
Igor
^ permalink raw reply
* Re: [PATCH net-next 08/19] net: usb: aqc111: Implement TX data path
From: Igor Russkikh @ 2018-10-08 13:43 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S . Miller, linux-usb@vger.kernel.org,
netdev@vger.kernel.org, Dmitry Bezrukov
In-Reply-To: <20181006011325.GB32455@lunn.ch>
>> + skb_push(skb, AQ_TX_HEADER_SIZE);
>> + cpu_to_le64s(&tx_hdr);
>
> Is that portable? tx_hdr is a structure of 2x u32 bitfields. What
> endian have you tested that one?
>
You are right, this is wrong for BE hardware.
We don't have such a hardware to check unfortunately.
Think its better to drop endianess conversions and declare
the driver as little endian only.
Do you think that'll be acceptable?
Regards,
Igor
^ permalink raw reply
* Re: [PATCHv2 bpf-next] bpf: fix building without CONFIG_INET
From: Song Liu @ 2018-10-08 20:52 UTC (permalink / raw)
To: joe
Cc: Daniel Borkmann, Arnd Bergmann, Alexei Starovoitov,
David S . Miller, John Fastabend, Martin KaFai Lau,
makita.toshiaki, Lawrence Brakmo, Andrey Ignatov,
Jesper Dangaard Brouer, Jakub Kicinski, Mathieu Xhonneux,
David Ahern, Networking, open list
In-Reply-To: <20181008183001.10406-1-joe@wand.net.nz>
On Mon, Oct 8, 2018 at 11:30 AM Joe Stringer <joe@wand.net.nz> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> The newly added TCP and UDP handling fails to link when CONFIG_INET
> is disabled:
>
> net/core/filter.o: In function `sk_lookup':
> filter.c:(.text+0x7ff8): undefined reference to `tcp_hashinfo'
> filter.c:(.text+0x7ffc): undefined reference to `tcp_hashinfo'
> filter.c:(.text+0x8020): undefined reference to `__inet_lookup_established'
> filter.c:(.text+0x8058): undefined reference to `__inet_lookup_listener'
> filter.c:(.text+0x8068): undefined reference to `udp_table'
> filter.c:(.text+0x8070): undefined reference to `udp_table'
> filter.c:(.text+0x808c): undefined reference to `__udp4_lib_lookup'
> net/core/filter.o: In function `bpf_sk_release':
> filter.c:(.text+0x82e8): undefined reference to `sock_gen_put'
>
> Wrap the related sections of code in #ifdefs for the config option.
>
> Furthermore, sk_lookup() should always have been marked 'static', this
> also avoids a warning about a missing prototype when building with
> 'make W=1'.
>
> Fixes: 6acc9b432e67 ("bpf: Add helper to retrieve socket in BPF")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Joe Stringer <joe@wand.net.nz>
Acked-by: Song Liu <songliubraving@fb.com>
> ---
> v2: Modify the patch to remove the related code in preprocessor rather
> than relying on compiler optimizations to remove the code.
> ---
> net/core/filter.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 30c6b2d3ef16..4bbc6567fcb8 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -4817,8 +4817,9 @@ static const struct bpf_func_proto bpf_lwt_seg6_adjust_srh_proto = {
> };
> #endif /* CONFIG_IPV6_SEG6_BPF */
>
> -struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
> - struct sk_buff *skb, u8 family, u8 proto)
> +#ifdef CONFIG_INET
> +static struct sock *sk_lookup(struct net *net, struct bpf_sock_tuple *tuple,
> + struct sk_buff *skb, u8 family, u8 proto)
> {
> int dif = skb->dev->ifindex;
> bool refcounted = false;
> @@ -4951,6 +4952,7 @@ static const struct bpf_func_proto bpf_sk_release_proto = {
> .ret_type = RET_INTEGER,
> .arg1_type = ARG_PTR_TO_SOCKET,
> };
> +#endif /* CONFIG_INET */
>
> bool bpf_helper_changes_pkt_data(void *func)
> {
> @@ -5158,12 +5160,14 @@ tc_cls_act_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> case BPF_FUNC_skb_ancestor_cgroup_id:
> return &bpf_skb_ancestor_cgroup_id_proto;
> #endif
> +#ifdef CONFIG_INET
> case BPF_FUNC_sk_lookup_tcp:
> return &bpf_sk_lookup_tcp_proto;
> case BPF_FUNC_sk_lookup_udp:
> return &bpf_sk_lookup_udp_proto;
> case BPF_FUNC_sk_release:
> return &bpf_sk_release_proto;
> +#endif
> default:
> return bpf_base_func_proto(func_id);
> }
> @@ -5264,12 +5268,14 @@ sk_skb_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
> return &bpf_sk_redirect_hash_proto;
> case BPF_FUNC_get_local_storage:
> return &bpf_get_local_storage_proto;
> +#ifdef CONFIG_INET
> case BPF_FUNC_sk_lookup_tcp:
> return &bpf_sk_lookup_tcp_proto;
> case BPF_FUNC_sk_lookup_udp:
> return &bpf_sk_lookup_udp_proto;
> case BPF_FUNC_sk_release:
> return &bpf_sk_release_proto;
> +#endif
> default:
> return bpf_base_func_proto(func_id);
> }
> --
> 2.17.1
>
^ permalink raw reply
* Re: [PATCH] bpf: btf: Fix a missing check bug
From: Song Liu @ 2018-10-08 20:51 UTC (permalink / raw)
To: wang6495; +Cc: kjlu, Alexei Starovoitov, Daniel Borkmann, Networking, open list
In-Reply-To: <1538943795-30895-1-git-send-email-wang6495@umn.edu>
On Sun, Oct 7, 2018 at 1:26 PM Wenwen Wang <wang6495@umn.edu> wrote:
>
> In btf_parse_hdr(), the length of the btf data header is firstly copied
> from the user space to 'hdr_len' and checked to see whether it is larger
> than 'btf_data_size'. If yes, an error code EINVAL is returned. Otherwise,
> the whole header is copied again from the user space to 'btf->hdr'.
> However, after the second copy, there is no check between
> 'btf->hdr->hdr_len' and 'hdr_len' to confirm that the two copies get the
> same value. Given that the btf data is in the user space, a malicious user
> can race to change the data between the two copies. By doing so, the user
> can provide malicious data to the kernel and cause undefined behavior.
>
> This patch adds a necessary check after the second copy, to make sure
> 'btf->hdr->hdr_len' has the same value as 'hdr_len'. Otherwise, an error
> code EINVAL will be returned.
These two numbers are copied from same memory location, right? So I
think this check is not necessary?
Thank,
Song
>
> Signed-off-by: Wenwen Wang <wang6495@umn.edu>
> ---
> kernel/bpf/btf.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 2590700..7cce7db 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -2114,6 +2114,9 @@ static int btf_parse_hdr(struct btf_verifier_env *env, void __user *btf_data,
>
> hdr = &btf->hdr;
>
> + if (hdr->hdr_len != hdr_len)
> + return -EINVAL;
> +
> btf_verifier_log_hdr(env, btf_data_size);
>
> if (hdr->magic != BTF_MAGIC) {
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH v2 net-next 17/23] net/namespace: Update rtnl_net_dumpid for strict data checking
From: Christian Brauner @ 2018-10-08 13:37 UTC (permalink / raw)
To: David Ahern; +Cc: David Ahern, netdev, davem, jbenc, stephen
In-Reply-To: <a6fa1883-e4b0-79c9-2e1e-ebe331f3d80e@gmail.com>
On Mon, Oct 08, 2018 at 07:28:33AM -0600, David Ahern wrote:
> On 10/8/18 4:54 AM, Christian Brauner wrote:
> > On Sun, Oct 07, 2018 at 08:16:38PM -0700, David Ahern wrote:
> >> From: David Ahern <dsahern@gmail.com>
> >>
> >> Update rtnl_net_dumpid for strict data checking. If the flag is set,
> >> the dump request is expected to have an rtgenmsg struct as the header
> >> which has the family as the only element. No data may be appended.
> >>
> >> Signed-off-by: David Ahern <dsahern@gmail.com>
> >> ---
> >> net/core/net_namespace.c | 6 ++++++
> >> 1 file changed, 6 insertions(+)
> >>
> >> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
> >> index 670c84b1bfc2..fefe72774aeb 100644
> >> --- a/net/core/net_namespace.c
> >> +++ b/net/core/net_namespace.c
> >> @@ -853,6 +853,12 @@ static int rtnl_net_dumpid(struct sk_buff *skb, struct netlink_callback *cb)
> >> .s_idx = cb->args[0],
> >> };
> >>
> >> + if (cb->strict_check &&
> >
> > Hm, shouldn't this also verify that the passed header is indeed struct
> > rtgenmsg before checking whether there are any attributes?
Thanks!
>
> rtgenmsg is only a struct with only the family as an element.
> rtnetlink_rcv_msg has already verified that the nl msg header contains
> at least the rtgenmsg struct.
>
>
> >
> >> + nlmsg_attrlen(cb->nlh, sizeof(struct rtgenmsg))) {
> >> + NL_SET_ERR_MSG(cb->extack, "Unknown data in network namespace id dump request");
> >> + return -EINVAL;
> >> + }
> >> +
> >> spin_lock_bh(&net->nsid_lock);
> >> idr_for_each(&net->netns_ids, rtnl_net_dumpid_one, &net_cb);
> >> spin_unlock_bh(&net->nsid_lock);
> >> --
> >> 2.11.0
> >>
>
^ permalink raw reply
* Re: selftests/bpf: test_kmod.sh hangs on all devices
From: Daniel Borkmann @ 2018-10-08 13:28 UTC (permalink / raw)
To: Naresh Kamboju, open list:KERNEL SELFTEST FRAMEWORK
Cc: Shuah Khan, Anders Roxell, Rafael Tinoco, ast, netdev,
songliubraving, willemb
In-Reply-To: <CA+G9fYumFxZghmxndq7STURbDbM4jgUzESr88yCgWvv=S+hbRA@mail.gmail.com>
On 10/08/2018 03:13 PM, Naresh Kamboju wrote:
> BPF test case test_kmod.sh hangs on all devices running linux next.
>
> + cd /opt/kselftests/default-in-kernel/bpf
> + ./test_kmod.sh
> sysctl: cannot stat /proc/sys/net/core/bpf_jit_enable: No such file or directory
> sysctl: cannot stat /proc/sys/net/core/bpf_jit_harden: No such file or directory
> sysctl: cannot stat /proc/sys/net/core/bpf_jit_enable: No such file or directory
> sysctl: cannot stat /proc/sys/net/core/bpf_jit_harden: No such file or directory
> [ JIT enabled:0 hardened:0 ]
>
> https://lkft.validation.linaro.org/scheduler/job/429726
>
> Test hangs started from 4.19.0-rc4-next-20180918.
> Linux version 4.19.0-rc4-next-20180918 (oe-user@oe-host) (gcc version
> 7.1.1 20170707 (Linaro GCC 7.1-2017.08)) #1 SMP Tue Sep 18 05:26:00
> UTC 2018
>
> History can be compared from this page.
> https://qa-reports.linaro.org/lkft/linux-next-oe/tests/kselftest/bpf_test_kmod.sh
>
> OTOH,
> There is a kernel BUG,
This is quite an old linux-next kernel, should be fixed by 100811936f89 ("bpf: test_bpf:
add init_net to dev for flow_dissector"). Please make sure you have that commit included
in your testing:
https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/commit/?id=100811936f89fd455eda1984810c09003550555b
Thanks,
Daniel
> [ 41.003698] BUG: unable to handle kernel paging request at 0000000000001460
> [ 41.016603] PGD 800000045d648067 P4D 800000045d648067 PUD 4575c8067 PMD 0
> [ 41.023475] Oops: 0000 [#1] SMP PTI
> [ 41.026959] CPU: 3 PID: 2790 Comm: modprobe Not tainted
> 4.19.0-rc4-next-20180920 #1
> [ 41.034621] Hardware name: Supermicro SYS-5019S-ML/X11SSH-F, BIOS
> 2.0b 07/27/2017
> [ 41.042094] RIP: 0010:__skb_flow_dissect+0xd9/0x1740
> [ 41.047057] Code: 7b 79 ff 85 c0 41 5a 74 0d 80 3d 06 cd 00 01 00
> 0f 84 4f 05 00 00 4d 85 ff 0f 84 4a 02 00 00 49 8b 47 10 48 8b 80 40
> 05 00 00 <4c> 8b 80 60 14 00 00 4c 89 85 28 ff ff ff e8 54 7b 79 ff 85
> c0 4c
> [ 41.065795] RSP: 0018:ffff9accc1cf79c0 EFLAGS: 00010286
> [ 41.071019] RAX: 0000000000000000 RBX: 000000000000000e RCX: 000000002c29b266
> [ 41.078143] RDX: 0000000097969774 RSI: 0000000000000000 RDI: 0000000000000001
> [ 41.085268] RBP: ffff9accc1cf7ac0 R08: ffffffff9b7517e9 R09: 0000000000000002
> [ 41.092392] R10: ffffffff9b7517e9 R11: ffffffff9c662080 R12: ffffffff9c78b620
> [ 41.099517] R13: 0000000000000008 R14: ffff9accc1cf7aec R15: ffff882f575e6800
> [ 41.106640] FS: 00007fcb533fe740(0000) GS:ffff882f5fb80000(0000)
> knlGS:0000000000000000
> [ 41.114716] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 41.120454] CR2: 0000000000001460 CR3: 000000045b606002 CR4: 00000000003606e0
> [ 41.127590] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 41.134737] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 41.141860] Call Trace:
> [ 41.144305] ? find_held_lock+0x35/0xa0
> [ 41.148136] ? __lock_acquire+0x2b5/0x1340
> [ 41.152227] ? find_held_lock+0x35/0xa0
> [ 41.156057] skb_get_poff+0x4b/0x90
> [ 41.159542] ? skb_get_poff+0x4b/0x90
> [ 41.163207] bpf_skb_get_pay_offset+0xe/0x20
> [ 41.167471] ___bpf_prog_run+0x45f/0x10d0
> [ 41.171475] __bpf_prog_run32+0x39/0x50
> [ 41.175305] ? lockdep_hardirqs_on+0xef/0x180
> [ 41.179656] ? ktime_get+0x6b/0x110
> [ 41.183143] test_bpf_init+0x5ab/0x1000 [test_bpf]
> [ 41.187933] ? 0xffffffffc02e2000
> [ 41.191244] do_one_initcall+0x5f/0x2b7
> [ 41.195074] ? rcu_read_lock_sched_held+0x81/0x90
> [ 41.199770] ? kmem_cache_alloc_trace+0x1de/0x210
> [ 41.204469] ? do_init_module+0x27/0x212
> [ 41.208387] do_init_module+0x5f/0x212
> [ 41.212138] load_module+0x20f5/0x2640
> [ 41.215886] __do_sys_finit_module+0xd1/0xf0
> [ 41.220156] ? __do_sys_finit_module+0xd1/0xf0
> [ 41.224626] __x64_sys_finit_module+0x1a/0x20
> [ 41.228984] do_syscall_64+0x4f/0x190
> [ 41.232645] entry_SYSCALL_64_after_hwframe+0x49/0xbe
> [ 41.237697] RIP: 0033:0x7fcb52d087f9
> [ 41.241266] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00
> 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 73 01 c3 48 8b 0d 4f e6 2b 00 f7 d8 64 89
> 01 48
> [ 41.260004] RSP: 002b:00007ffe780dd828 EFLAGS: 00000206 ORIG_RAX:
> 0000000000000139
> [ 41.267589] RAX: ffffffffffffffda RBX: 00000000006aba70 RCX: 00007fcb52d087f9
> [ 41.274737] RDX: 0000000000000000 RSI: 0000000000418424 RDI: 0000000000000003
> [ 41.281861] RBP: 0000000000418424 R08: 0000000000000000 R09: 0000000000000000
> [ 41.288985] R10: 0000000000000003 R11: 0000000000000206 R12: 0000000000000000
> [ 41.296108] R13: 0000000000040000 R14: 0000000000000000 R15: 0000000000000000
> [ 41.303234] Modules linked in: test_bpf(+) x86_pkg_temp_thermal fuse
> [ 41.309590] CR2: 0000000000001460
> [ 41.312930] ---[ end trace a7d5229a26c41aad ]---
> [ 41.317541] RIP: 0010:__skb_flow_dissect+0xd9/0x1740
> [ 41.322497] Code: 7b 79 ff 85 c0 41 5a 74 0d 80 3d 06 cd 00 01 00
> 0f 84 4f 05 00 00 4d 85 ff 0f 84 4a 02 00 00 49 8b 47 10 48 8b 80 40
> 05 00 00 <4c> 8b 80 60 14 00 00 4c 89 85 28 ff ff ff e8 54 7b 79 ff 85
> c0 4c
> [ 41.341235] RSP: 0018:ffff9accc1cf79c0 EFLAGS: 00010286
> [ 41.346452] RAX: 0000000000000000 RBX: 000000000000000e RCX: 000000002c29b266
> [ 41.353588] RDX: 0000000097969774 RSI: 0000000000000000 RDI: 0000000000000001
> [ 41.360716] RBP: ffff9accc1cf7ac0 R08: ffffffff9b7517e9 R09: 0000000000000002
> [ 41.367841] R10: ffffffff9b7517e9 R11: ffffffff9c662080 R12: ffffffff9c78b620
> [ 41.374964] R13: 0000000000000008 R14: ffff9accc1cf7aec R15: ffff882f575e6800
> [ 41.382089] FS: 00007fcb533fe740(0000) GS:ffff882f5fb80000(0000)
> knlGS:0000000000000000
> [ 41.390165] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 41.395903] CR2: 0000000000001460 CR3: 000000045b606002 CR4: 00000000003606e0
> [ 41.403026] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 41.410153] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> [ 41.417275] BUG: sleeping function called from invalid context at
> /srv/oe/build/tmp-rpb-glibc/work-shared/intel-corei7-64/kernel-source/include/linux/percpu-rwsem.h:34
> [ 41.432199] in_atomic(): 1, irqs_disabled(): 1, pid: 2790, name: modprobe
> [ 41.438977] INFO: lockdep is turned off.
> [ 41.442893] irq event stamp: 13766
> [ 41.446292] hardirqs last enabled at (13765): [<ffffffff9af037eb>]
> ktime_get+0x6b/0x110
> [ 41.454369] hardirqs last disabled at (13766): [<ffffffff9ae01c6b>]
> trace_hardirqs_off_thunk+0x1a/0x1c
> [ 41.463658] softirqs last enabled at (13728): [<ffffffff9be0032d>]
> __do_softirq+0x32d/0x42f
> [ 41.472084] softirqs last disabled at (13711): [<ffffffff9ae70679>]
> irq_exit+0xc9/0xd0
> [ 41.479996] CPU: 3 PID: 2790 Comm: modprobe Tainted: G D
> 4.19.0-rc4-next-20180920 #1
> [ 41.489026] Hardware name: Supermicro SYS-5019S-ML/X11SSH-F, BIOS
> 2.0b 07/27/2017
> [ 41.496496] Call Trace:
> [ 41.498943] dump_stack+0x70/0xa5
> [ 41.502261] ___might_sleep+0x152/0x240
> [ 41.506091] __might_sleep+0x4a/0x80
> [ 41.509679] exit_signals+0x33/0x240
> [ 41.513249] do_exit+0xb1/0xc60
> [ 41.516387] rewind_stack_do_exit+0x17/0x20
> [ 41.520587] RIP: 0033:0x7fcb52d087f9
> [ 41.524177] Code: 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00
> 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 73 01 c3 48 8b 0d 4f e6 2b 00 f7 d8 64 89
> 01 48
> [ 41.542914] RSP: 002b:00007ffe780dd828 EFLAGS: 00000206 ORIG_RAX:
> 0000000000000139
> [ 41.550471] RAX: ffffffffffffffda RBX: 00000000006aba70 RCX: 00007fcb52d087f9
> [ 41.557608] RDX: 0000000000000000 RSI: 0000000000418424 RDI: 0000000000000003
> [ 41.564735] RBP: 0000000000418424 R08: 0000000000000000 R09: 0000000000000000
> [ 41.571860] R10: 0000000000000003 R11: 0000000000000206 R12: 0000000000000000
> [ 41.578985] R13: 0000000000040000 R14: 0000000000000000 R15: 0000000000000000
> e)
>
> Bug reported,
> https://bugs.linaro.org/show_bug.cgi?id=4006
>
^ permalink raw reply
* Re: [PATCH v2 net-next 17/23] net/namespace: Update rtnl_net_dumpid for strict data checking
From: David Ahern @ 2018-10-08 13:28 UTC (permalink / raw)
To: Christian Brauner, David Ahern; +Cc: netdev, davem, jbenc, stephen
In-Reply-To: <20181008105430.cnpflmfwhkn3u2lq@brauner.io>
On 10/8/18 4:54 AM, Christian Brauner wrote:
> On Sun, Oct 07, 2018 at 08:16:38PM -0700, David Ahern wrote:
>> From: David Ahern <dsahern@gmail.com>
>>
>> Update rtnl_net_dumpid for strict data checking. If the flag is set,
>> the dump request is expected to have an rtgenmsg struct as the header
>> which has the family as the only element. No data may be appended.
>>
>> Signed-off-by: David Ahern <dsahern@gmail.com>
>> ---
>> net/core/net_namespace.c | 6 ++++++
>> 1 file changed, 6 insertions(+)
>>
>> diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c
>> index 670c84b1bfc2..fefe72774aeb 100644
>> --- a/net/core/net_namespace.c
>> +++ b/net/core/net_namespace.c
>> @@ -853,6 +853,12 @@ static int rtnl_net_dumpid(struct sk_buff *skb, struct netlink_callback *cb)
>> .s_idx = cb->args[0],
>> };
>>
>> + if (cb->strict_check &&
>
> Hm, shouldn't this also verify that the passed header is indeed struct
> rtgenmsg before checking whether there are any attributes?
rtgenmsg is only a struct with only the family as an element.
rtnetlink_rcv_msg has already verified that the nl msg header contains
at least the rtgenmsg struct.
>
>> + nlmsg_attrlen(cb->nlh, sizeof(struct rtgenmsg))) {
>> + NL_SET_ERR_MSG(cb->extack, "Unknown data in network namespace id dump request");
>> + return -EINVAL;
>> + }
>> +
>> spin_lock_bh(&net->nsid_lock);
>> idr_for_each(&net->netns_ids, rtnl_net_dumpid_one, &net_cb);
>> spin_unlock_bh(&net->nsid_lock);
>> --
>> 2.11.0
>>
^ permalink raw reply
* Re: [PATCH v2 net-next 11/23] rtnetlink: Update rtnl_stats_dump for strict data checking
From: David Ahern @ 2018-10-08 13:25 UTC (permalink / raw)
To: Christian Brauner, David Ahern; +Cc: netdev, davem, jbenc, stephen
In-Reply-To: <20181008101725.zjdsnwcyww7tmwnr@brauner.io>
On 10/8/18 4:17 AM, Christian Brauner wrote:
>> @@ -4696,13 +4697,32 @@ static int rtnl_stats_dump(struct sk_buff *skb, struct netlink_callback *cb)
>>
>> cb->seq = net->dev_base_seq;
>>
>> - if (nlmsg_len(cb->nlh) < sizeof(*ifsm))
>> + if (nlmsg_len(cb->nlh) < sizeof(*ifsm)) {
>> + NL_SET_ERR_MSG(extack, "Invalid header for stats dump");
>> return -EINVAL;
>> + }
>>
>> ifsm = nlmsg_data(cb->nlh);
>> +
>> + /* only requests using NLM_F_DUMP_PROPER_HDR can pass data to
>
> That looks like an accidental leftover before we changed this to a
> socket option. :)
>
ugh. thanks for noticing.
^ permalink raw reply
* Re: [PATCH v3] rtw88: Add firmware file for driver rtw88
From: Josh Boyer @ 2018-10-08 13:23 UTC (permalink / raw)
To: Larry Finger; +Cc: Linux Firmware, Linux Wireless, netdev, yhchuang
In-Reply-To: <20181004193155.22669-1-Larry.Finger@lwfinger.net>
On Thu, Oct 4, 2018 at 3:32 PM Larry Finger <Larry.Finger@lwfinger.net> wrote:
>
> This file contains the firmware for the new driver rtw88 for Realtek
> RTL8822BE and RTL8822CE devices. It was provided by the Realtek
> developer.
>
> Signed-off-by: Yan-Hsuan Chuang <yhchuang@realtek.com>
> Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
> ---
> v2 - removed redundant information from subject
> v3 - drivers are for 802.11ac, not 802.11n
> added firmware for RL8822CE
> ---
> WHENCE | 10 ++++++++++
> rtw88/rtw8822b_fw.bin | Bin 0 -> 150984 bytes
> rtw88/rtw8822c_fw.bin | Bin 0 -> 162064 bytes
> 3 files changed, 10 insertions(+)
> create mode 100644 rtw88/rtw8822b_fw.bin
> create mode 100644 rtw88/rtw8822c_fw.bin
Applied and pushed out.
josh
^ permalink raw reply
* Re: [PATCH net-next 06/19] net: usb: aqc111: Introduce link management
From: Igor Russkikh @ 2018-10-08 13:22 UTC (permalink / raw)
To: Andrew Lunn
Cc: David S . Miller, linux-usb@vger.kernel.org,
netdev@vger.kernel.org, Dmitry Bezrukov
In-Reply-To: <20181005171158.GL4730@lunn.ch>
Hi Andrew,
> Hi Igor, Dmitry
>
> Please could you explain why you decided to not use drivers/net/phy?
> The previous patch introduced basically what you need to export a
> standard Linux MDIO bus. From that you can use a standard Linux PHY
> driver.
Thats again because of this product has tightly integrated MAC+Phy.
MAC FW controls system interface and reports/alters link state
as a joint state on copper and SIF (even in dpa direct phy mode).
We can't extract phy api into a standalone fully functional phylib therefore.
Also as far as I know this particular phy is not available in the wild.
Regards,
Igor
^ permalink raw reply
* Re: general protection fault in __handle_mm_fault
From: Willem de Bruijn @ 2018-10-08 20:33 UTC (permalink / raw)
To: syzbot+1577fbe983d20fe2e88f
Cc: David Miller, Eric Dumazet, Alexey Kuznetsov, LKML,
Network Development, syzkaller-bugs, Hideaki YOSHIFUJI,
Andrew Morton, kirill.shutemov, aneesh.kumar
In-Reply-To: <CAF=yD-+7NxN37Ez=3WXG0+2i6j30uSV8u9D7oJXEhp+SxzYCpA@mail.gmail.com>
On Mon, Oct 8, 2018 at 12:10 PM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> On Fri, Oct 5, 2018 at 6:27 PM syzbot
> <syzbot+1577fbe983d20fe2e88f@syzkaller.appspotmail.com> wrote:
> >
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit: 25bcda3e8b9f Add linux-next specific files for 20181004
> > git tree: linux-next
> > console output: https://syzkaller.appspot.com/x/log.txt?x=130e3bf1400000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=603d7f9140c3368a
> > dashboard link: https://syzkaller.appspot.com/bug?extid=1577fbe983d20fe2e88f
> > compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=127e88d6400000
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=13cdb67e400000
>
> > RIP: 0010:copy_user_enhanced_fast_string+0xe/0x20
> > arch/x86/lib/copy_user_64.S:180
> > Code: 89 d1 c1 e9 03 83 e2 07 f3 48 a5 89 d1 f3 a4 31 c0 0f 1f 00 c3 0f 1f
> > 80 00 00 00 00 0f 1f 00 83 fa 40 0f 82 70 ff ff ff 89 d1 <f3> a4 31 c0 0f
> > 1f 00 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 83
> > RSP: 0018:ffff8801bbe675b8 EFLAGS: 00010202
> > RAX: 0000000000000000 RBX: 0000000000007a50 RCX: 0000000000001b40
> > RDX: 0000000000007a50 RSI: 0000000020077000 RDI: ffff8801ce615f10
> > RBP: ffff8801bbe675f0 R08: ffffed0039cc2f4a R09: ffffed0039cc2f4a
> > R10: ffffed0039cc2f49 R11: ffff8801ce617a4f R12: 0000000020078b40
> > R13: 00000000200710f0 R14: ffff8801ce610000 R15: 00007ffffffff000
> > _copy_from_iter_full+0x263/0xc20 lib/iov_iter.c:724
> > copy_from_iter_full include/linux/uio.h:124 [inline]
> > skb_do_copy_data_nocache include/net/sock.h:1951 [inline]
> > skb_copy_to_page_nocache include/net/sock.h:1977 [inline]
> > tcp_sendmsg_locked+0x159e/0x3f90 net/ipv4/tcp.c:1338
>
> This started on next-20181004. It still happens as of next-20181008.
>
> It does not trigger on next 20181003. It does not occur if
> CONFIG_DEBUG_KOBJECT is disabled.
Bisected to commit e4d0c281a4c9 ("mm/memory.c: recheck page table
entry with page table lock held").
Verified to not trigger on next-20181008 after reverting that commit.
^ permalink raw reply
* Re: [PATCH V1 net 0/5] minor bug fixes for ENA Ethernet driver
From: Bshara, Nafea @ 2018-10-08 12:47 UTC (permalink / raw)
To: Kiyanovski, Arthur, davem@davemloft.net, netdev@vger.kernel.org
Cc: Woodhouse, David, Machulsky, Zorik, Matushevsky, Alexander,
Bshara, Saeed, Wilson, Matt, Liguori, Anthony, Tzalik, Guy,
Belgazal, Netanel, Saidi, Ali
In-Reply-To: <1539001724-25980-1-git-send-email-akiyano@amazon.com>
Ship it
On 10/8/18, 5:28 AM, "akiyano@amazon.com" <akiyano@amazon.com> wrote:
From: Arthur Kiyanovski <akiyano@amazon.com>
Arthur Kiyanovski (5):
net: ena: fix indentations in ena_defs for better readability
net: ena: fix warning in rmmod caused by double iounmap
net: ena: fix rare bug when failed restart/resume is followed by
driver removal
net: ena: fix NULL dereference due to untimely napi initialization
net: ena: fix auto casting to boolean
drivers/net/ethernet/amazon/ena/ena_admin_defs.h | 308 +++++++++-------------
drivers/net/ethernet/amazon/ena/ena_eth_com.c | 8 +-
drivers/net/ethernet/amazon/ena/ena_eth_io_defs.h | 219 ++++++++-------
drivers/net/ethernet/amazon/ena/ena_netdev.c | 23 +-
drivers/net/ethernet/amazon/ena/ena_regs_defs.h | 206 +++++++--------
5 files changed, 341 insertions(+), 423 deletions(-)
--
2.7.4
^ permalink raw reply
* Re: re iproute2 - don't return error on success fix
From: Phil Sutter @ 2018-10-08 12:41 UTC (permalink / raw)
To: Or Gerlitz; +Cc: Stephen Hemminger, David Ahern, Linux Netdev List, Roi Dayan
In-Reply-To: <CAJ3xEMj0wQZiCE3+qcpOq25oNZKqP81f5i0Xt0ctcUt-yvVAkQ@mail.gmail.com>
Hi Or,
On Tue, Oct 02, 2018 at 03:21:18PM +0300, Or Gerlitz wrote:
> On Thu, Sep 27, 2018 at 3:53 PM Phil Sutter <phil@nwl.cc> wrote:
[...]
> > Hmm, I can't reproduce this. My HEAD is at the commit you mentioned:
> >
> > | % sudo ./tc/tc filter add dev d0 protocol ip parent ffff: flower skip_sw ip_flags nofirstfrag action drop
> > | RTNETLINK answers: Operation not supported
> > | We have an error talking to the kernel, -1
> > | % echo $?
> > | 2
> >
> > Are you sure you tested the right binary?
>
> double checked now, seems broken on my end:
>
> $ git log --oneline -3
> b45e300 libnetlink: don't return error on success
> 5dc2204 testsuite: add libmnl
> 8804a8c Makefile: Add check target
>
> $./tc/tc filter add dev enp33s0f0 protocol ip parent ffff: flower
> skip_sw ip_flags frag action drop && echo "success" || echo "failed"
> success
>
> $ ./tc/tc filter add dev enp33s0f0 protocol ip parent ffff: flower
> skip_sw ip_flags firstfrag action drop && echo "success" || echo
> "failed"
> RTNETLINK answers: Operation not supported
> success
Interestingly, your output lacks the "We have an error" message mine
shows. So in your case, the call to rtnl_talk_iov() from
tc_filter_modify() does not return < 0.
In my case, commands always fail with 'skip_sw' since I only test on a
dummy interface. So maybe we hit different error paths? Could you please
check what happens for you in __rtnl_talk_iov()? I guess the "RTNETLINK
answers:" message should come from rtnl_talk_error(). In my case it does
at least, and I haven't found an alternative place where that message
could come from.
Cheers, Phil
^ permalink raw reply
* [PATCH net-next 3/3] selftests: pmtu: add basic IPv4 and IPv6 PMTU tests
From: Sabrina Dubroca @ 2018-10-08 12:37 UTC (permalink / raw)
To: netdev; +Cc: Stefano Brivio, Sabrina Dubroca
In-Reply-To: <cover.1539001627.git.sd@queasysnail.net>
Commit d1f1b9cbf34c ("selftests: net: Introduce first PMTU test") and
follow-ups introduced some PMTU tests, but they all rely on tunneling,
and, particularly, on VTI.
These new tests use simple routing to exercise the generation and
update of PMTU exceptions in IPv4 and IPv6.
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
---
tools/testing/selftests/net/pmtu.sh | 207 +++++++++++++++++++++++++++-
1 file changed, 203 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
index 03e56a27f69c..b9cdb68df4c5 100755
--- a/tools/testing/selftests/net/pmtu.sh
+++ b/tools/testing/selftests/net/pmtu.sh
@@ -6,6 +6,26 @@
#
# Tests currently implemented:
#
+# - pmtu_ipv4
+# Set up two namespaces, A and B, with two paths between them over routers
+# R1 and R2 (also implemented with namespaces), with different MTUs:
+#
+# segment a_r1 segment b_r1 a_r1: 2000
+# .--------------R1--------------. a_r2: 1500
+# A B a_r3: 2000
+# '--------------R2--------------' a_r4: 1400
+# segment a_r2 segment b_r2
+#
+# Check that PMTU exceptions with the correct PMTU are created. Then
+# decrease and increase the MTU of the local link for one of the paths,
+# A to R1, checking that route exception PMTU changes accordingly over
+# this path. Also check that locked exceptions are created when an ICMP
+# message advertising a PMTU smaller than net.ipv4.route.min_pmtu is
+# received
+#
+# - pmtu_ipv6
+# Same as pmtu_ipv4, except for locked PMTU tests, using IPv6
+#
# - pmtu_vti4_exception
# Set up vti tunnel on top of veth, with xfrm states and policies, in two
# namespaces with matching endpoints. Check that route exception is not
@@ -50,6 +70,8 @@ ksft_skip=4
which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
tests="
+ pmtu_ipv4_exception ipv4: PMTU exceptions
+ pmtu_ipv6_exception ipv6: PMTU exceptions
pmtu_vti6_exception vti6: PMTU exceptions
pmtu_vti4_exception vti4: PMTU exceptions
pmtu_vti4_default_mtu vti4: default MTU assignment
@@ -60,8 +82,45 @@ tests="
NS_A="ns-$(mktemp -u XXXXXX)"
NS_B="ns-$(mktemp -u XXXXXX)"
+NS_R1="ns-$(mktemp -u XXXXXX)"
+NS_R2="ns-$(mktemp -u XXXXXX)"
ns_a="ip netns exec ${NS_A}"
ns_b="ip netns exec ${NS_B}"
+ns_r1="ip netns exec ${NS_R1}"
+ns_r2="ip netns exec ${NS_R2}"
+
+# Addressing and routing for tests with routers: four network segments, with
+# index SEGMENT between 1 and 4, a common prefix (PREFIX4 or PREFIX6) and an
+# identifier ID, which is 1 for hosts (A and B), 2 for routers (R1 and R2).
+# Addresses are:
+# - IPv4: PREFIX4.SEGMENT.ID (/24)
+# - IPv6: PREFIX6:SEGMENT::ID (/64)
+prefix4="192.168"
+prefix6="fd00"
+a_r1=1
+a_r2=2
+b_r1=3
+b_r2=4
+# ns peer segment
+routing_addrs="
+ A R1 ${a_r1}
+ A R2 ${a_r2}
+ B R1 ${b_r1}
+ B R2 ${b_r2}
+"
+# Traffic from A to B goes through R1 by default, and through R2, if destined to
+# B's address on the b_r2 segment.
+# Traffic from B to A goes through R1.
+# ns destination gateway
+routes="
+ A default ${prefix4}.${a_r1}.2
+ A ${prefix4}.${b_r2}.1 ${prefix4}.${a_r2}.2
+ B default ${prefix4}.${b_r1}.2
+
+ A default ${prefix6}:${a_r1}::2
+ A ${prefix6}:${b_r2}::1 ${prefix6}:${a_r2}::2
+ B default ${prefix6}:${b_r1}::2
+"
veth4_a_addr="192.168.1.1"
veth4_b_addr="192.168.1.2"
@@ -94,9 +153,15 @@ err_flush() {
err_buf=
}
+# Find the auto-generated name for this namespace
+nsname() {
+ eval echo \$NS_$1
+}
+
setup_namespaces() {
- ip netns add ${NS_A} || return 1
- ip netns add ${NS_B}
+ for n in ${NS_A} ${NS_B} ${NS_R1} ${NS_R2}; do
+ ip netns add ${n} || return 1
+ done
}
setup_veth() {
@@ -167,6 +232,49 @@ setup_xfrm6() {
setup_xfrm 6 ${veth6_a_addr} ${veth6_b_addr}
}
+setup_routing() {
+ for i in ${NS_R1} ${NS_R2}; do
+ ip netns exec ${i} sysctl -q net/ipv4/ip_forward=1
+ ip netns exec ${i} sysctl -q net/ipv6/conf/all/forwarding=1
+ done
+
+ for i in ${routing_addrs}; do
+ [ "${ns}" = "" ] && ns="${i}" && continue
+ [ "${peer}" = "" ] && peer="${i}" && continue
+ [ "${segment}" = "" ] && segment="${i}"
+
+ ns_name="$(nsname ${ns})"
+ peer_name="$(nsname ${peer})"
+ if="veth_${ns}-${peer}"
+ ifpeer="veth_${peer}-${ns}"
+
+ # Create veth links
+ ip link add ${if} up netns ${ns_name} type veth peer name ${ifpeer} netns ${peer_name} || return 1
+ ip -n ${peer_name} link set dev ${ifpeer} up
+
+ # Add addresses
+ ip -n ${ns_name} addr add ${prefix4}.${segment}.1/24 dev ${if}
+ ip -n ${ns_name} addr add ${prefix6}:${segment}::1/64 dev ${if}
+
+ ip -n ${peer_name} addr add ${prefix4}.${segment}.2/24 dev ${ifpeer}
+ ip -n ${peer_name} addr add ${prefix6}:${segment}::2/64 dev ${ifpeer}
+
+ ns=""; peer=""; segment=""
+ done
+
+ for i in ${routes}; do
+ [ "${ns}" = "" ] && ns="${i}" && continue
+ [ "${addr}" = "" ] && addr="${i}" && continue
+ [ "${gw}" = "" ] && gw="${i}"
+
+ ns_name="$(nsname ${ns})"
+
+ ip -n ${ns_name} route add ${addr} via ${gw}
+
+ ns=""; addr=""; gw=""
+ done
+}
+
setup() {
[ "$(id -u)" -ne 0 ] && echo " need to run as root" && return $ksft_skip
@@ -178,8 +286,9 @@ setup() {
cleanup() {
[ ${cleanup_done} -eq 1 ] && return
- ip netns del ${NS_A} 2> /dev/null
- ip netns del ${NS_B} 2> /dev/null
+ for n in ${NS_A} ${NS_B} ${NS_R1} ${NS_R2}; do
+ ip netns del ${n} 2> /dev/null
+ done
cleanup_done=1
}
@@ -244,6 +353,96 @@ check_pmtu_value() {
return 1
}
+test_pmtu_ipvX() {
+ family=${1}
+
+ setup namespaces routing || return 2
+
+ if [ ${family} -eq 4 ]; then
+ ping=ping
+ dst1="${prefix4}.${b_r1}.1"
+ dst2="${prefix4}.${b_r2}.1"
+ else
+ ping=${ping6}
+ dst1="${prefix6}:${b_r1}::1"
+ dst2="${prefix6}:${b_r2}::1"
+ fi
+
+ # Set up initial MTU values
+ mtu "${ns_a}" veth_A-R1 2000
+ mtu "${ns_r1}" veth_R1-A 2000
+ mtu "${ns_r1}" veth_R1-B 1400
+ mtu "${ns_b}" veth_B-R1 1400
+
+ mtu "${ns_a}" veth_A-R2 2000
+ mtu "${ns_r2}" veth_R2-A 2000
+ mtu "${ns_r2}" veth_R2-B 1500
+ mtu "${ns_b}" veth_B-R2 1500
+
+ # Create route exceptions
+ ${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s 1800 ${dst1} > /dev/null
+ ${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s 1800 ${dst2} > /dev/null
+
+ # Check that exceptions have been created with the correct PMTU
+ pmtu_1="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst1})"
+ check_pmtu_value "1400" "${pmtu_1}" "exceeding MTU" || return 1
+ pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})"
+ check_pmtu_value "1500" "${pmtu_2}" "exceeding MTU" || return 1
+
+ # Decrease local MTU below PMTU, check for PMTU decrease in route exception
+ mtu "${ns_a}" veth_A-R1 1300
+ mtu "${ns_r1}" veth_R1-A 1300
+ pmtu_1="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst1})"
+ check_pmtu_value "1300" "${pmtu_1}" "decreasing local MTU" || return 1
+ # Second exception shouldn't be modified
+ pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})"
+ check_pmtu_value "1500" "${pmtu_2}" "changing local MTU on a link not on this path" || return 1
+
+ # Increase MTU, check for PMTU increase in route exception
+ mtu "${ns_a}" veth_A-R1 1700
+ mtu "${ns_r1}" veth_R1-A 1700
+ pmtu_1="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst1})"
+ check_pmtu_value "1700" "${pmtu_1}" "increasing local MTU" || return 1
+ # Second exception shouldn't be modified
+ pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})"
+ check_pmtu_value "1500" "${pmtu_2}" "changing local MTU on a link not on this path" || return 1
+
+ # Skip PMTU locking tests for IPv6
+ [ $family -eq 6 ] && return 0
+
+ # Decrease remote MTU on path via R2, get new exception
+ mtu "${ns_r2}" veth_R2-B 400
+ mtu "${ns_b}" veth_B-R2 400
+ ${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s 1400 ${dst2} > /dev/null
+ pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})"
+ check_pmtu_value "lock 552" "${pmtu_2}" "exceeding MTU, with MTU < min_pmtu" || return 1
+
+ # Decrease local MTU below PMTU
+ mtu "${ns_a}" veth_A-R2 500
+ mtu "${ns_r2}" veth_R2-A 500
+ pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})"
+ check_pmtu_value "500" "${pmtu_2}" "decreasing local MTU" || return 1
+
+ # Increase local MTU
+ mtu "${ns_a}" veth_A-R2 1500
+ mtu "${ns_r2}" veth_R2-A 1500
+ pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})"
+ check_pmtu_value "1500" "${pmtu_2}" "increasing local MTU" || return 1
+
+ # Get new exception
+ ${ns_a} ${ping} -q -M want -i 0.1 -w 2 -s 1400 ${dst2} > /dev/null
+ pmtu_2="$(route_get_dst_pmtu_from_exception "${ns_a}" ${dst2})"
+ check_pmtu_value "lock 552" "${pmtu_2}" "exceeding MTU, with MTU < min_pmtu" || return 1
+}
+
+test_pmtu_ipv4_exception() {
+ test_pmtu_ipvX 4
+}
+
+test_pmtu_ipv6_exception() {
+ test_pmtu_ipvX 6
+}
+
test_pmtu_vti4_exception() {
setup namespaces veth vti4 xfrm4 || return 2
--
2.19.0
^ permalink raw reply related
* [PATCH net-next 2/3] selftests: pmtu: extend MTU parsing helper to locked MTU
From: Sabrina Dubroca @ 2018-10-08 12:37 UTC (permalink / raw)
To: netdev; +Cc: Stefano Brivio, Sabrina Dubroca
In-Reply-To: <cover.1539001627.git.sd@queasysnail.net>
The mtu_parse helper introduced in commit f2c929feeccd ("selftests:
pmtu: Factor out MTU parsing helper") can only handle "mtu 1234", but
not "mtu lock 1234". Extend it, so that we can do IPv4 tests with PMTU
smaller than net.ipv4.route.min_pmtu
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
tools/testing/selftests/net/pmtu.sh | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
index 6c38a7637744..03e56a27f69c 100755
--- a/tools/testing/selftests/net/pmtu.sh
+++ b/tools/testing/selftests/net/pmtu.sh
@@ -196,7 +196,9 @@ mtu_parse() {
next=0
for i in ${input}; do
+ [ ${next} -eq 1 -a "${i}" = "lock" ] && next=2 && continue
[ ${next} -eq 1 ] && echo "${i}" && return
+ [ ${next} -eq 2 ] && echo "lock ${i}" && return
[ "${i}" = "mtu" ] && next=1
done
}
--
2.19.0
^ permalink raw reply related
* [PATCH net-next 1/3] selftests: pmtu: Introduce check_pmtu_value()
From: Sabrina Dubroca @ 2018-10-08 12:37 UTC (permalink / raw)
To: netdev; +Cc: Stefano Brivio, Sabrina Dubroca
In-Reply-To: <cover.1539001627.git.sd@queasysnail.net>
From: Stefano Brivio <sbrivio@redhat.com>
Introduce and use a function that checks PMTU values against
expected values and logs error messages, to remove some clutter.
Signed-off-by: Stefano Brivio <sbrivio@redhat.com>
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
---
tools/testing/selftests/net/pmtu.sh | 49 +++++++++++++----------------
1 file changed, 22 insertions(+), 27 deletions(-)
diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
index 0ab9423d009f..6c38a7637744 100755
--- a/tools/testing/selftests/net/pmtu.sh
+++ b/tools/testing/selftests/net/pmtu.sh
@@ -229,6 +229,19 @@ route_get_dst_pmtu_from_exception() {
mtu_parse "$(route_get_dst_exception "${ns_cmd}" ${dst})"
}
+check_pmtu_value() {
+ expected="${1}"
+ value="${2}"
+ event="${3}"
+
+ [ "${expected}" = "any" ] && [ -n "${value}" ] && return 0
+ [ "${value}" = "${expected}" ] && return 0
+ [ -z "${value}" ] && err " PMTU exception wasn't created after ${event}" && return 1
+ [ -z "${expected}" ] && err " PMTU exception shouldn't exist after ${event}" && return 1
+ err " found PMTU exception with incorrect MTU ${value}, expected ${expected}, after ${event}"
+ return 1
+}
+
test_pmtu_vti4_exception() {
setup namespaces veth vti4 xfrm4 || return 2
@@ -248,24 +261,13 @@ test_pmtu_vti4_exception() {
# exception is created
${ns_a} ping -q -M want -i 0.1 -w 2 -s ${ping_payload} ${vti4_b_addr} > /dev/null
pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti4_b_addr})"
- if [ "${pmtu}" != "" ]; then
- err " unexpected exception created with PMTU ${pmtu} for IP payload length ${esp_payload_rfc4106}"
- return 1
- fi
+ check_pmtu_value "" "${pmtu}" "sending packet smaller than PMTU (IP payload length ${esp_payload_rfc4106})" || return 1
# Now exceed link layer MTU by one byte, check that exception is created
+ # with the right PMTU value
${ns_a} ping -q -M want -i 0.1 -w 2 -s $((ping_payload + 1)) ${vti4_b_addr} > /dev/null
pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti4_b_addr})"
- if [ "${pmtu}" = "" ]; then
- err " exception not created for IP payload length $((esp_payload_rfc4106 + 1))"
- return 1
- fi
-
- # ...with the right PMTU value
- if [ ${pmtu} -ne ${esp_payload_rfc4106} ]; then
- err " wrong PMTU ${pmtu} in exception, expected: ${esp_payload_rfc4106}"
- return 1
- fi
+ check_pmtu_value "${esp_payload_rfc4106}" "${pmtu}" "exceeding PMTU (IP payload length $((esp_payload_rfc4106 + 1)))"
}
test_pmtu_vti6_exception() {
@@ -280,25 +282,18 @@ test_pmtu_vti6_exception() {
${ns_a} ${ping6} -q -i 0.1 -w 2 -s 60000 ${vti6_b_addr} > /dev/null
# Check that exception was created
- if [ "$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti6_b_addr})" = "" ]; then
- err " tunnel exceeding link layer MTU didn't create route exception"
- return 1
- fi
+ pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti6_b_addr})"
+ check_pmtu_value any "${pmtu}" "creating tunnel exceeding link layer MTU" || return 1
# Decrease tunnel MTU, check for PMTU decrease in route exception
mtu "${ns_a}" vti6_a 3000
-
- if [ "$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti6_b_addr})" -ne 3000 ]; then
- err " decreasing tunnel MTU didn't decrease route exception PMTU"
- fail=1
- fi
+ pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti6_b_addr})"
+ check_pmtu_value "3000" "${pmtu}" "decreasing tunnel MTU" || fail=1
# Increase tunnel MTU, check for PMTU increase in route exception
mtu "${ns_a}" vti6_a 9000
- if [ "$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti6_b_addr})" -ne 9000 ]; then
- err " increasing tunnel MTU didn't increase route exception PMTU"
- fail=1
- fi
+ pmtu="$(route_get_dst_pmtu_from_exception "${ns_a}" ${vti6_b_addr})"
+ check_pmtu_value "9000" "${pmtu}" "increasing tunnel MTU" || fail=1
return ${fail}
}
--
2.19.0
^ permalink raw reply related
* [PATCH net-next 0/3] selftests: add more PMTU tests
From: Sabrina Dubroca @ 2018-10-08 12:37 UTC (permalink / raw)
To: netdev; +Cc: Stefano Brivio, Sabrina Dubroca
The current selftests for PMTU cover VTI tunnels, but there's nothing
about the generation and handling of PMTU exceptions by intermediate
routers. This series adds and improves existing helpers, then adds
IPv4 and IPv6 selftests with a setup involving an intermediate router.
Joint work with Stefano Brivio.
Sabrina Dubroca (2):
selftests: pmtu: extend MTU parsing helper to locked MTU
selftests: pmtu: add basic IPv4 and IPv6 PMTU tests
Stefano Brivio (1):
selftests: pmtu: Introduce check_pmtu_value()
tools/testing/selftests/net/pmtu.sh | 258 ++++++++++++++++++++++++----
1 file changed, 227 insertions(+), 31 deletions(-)
--
2.19.0
^ permalink raw reply
* [PATCH net 1/2] net: ipv4: update fnhe_pmtu when first hop's MTU changes
From: Sabrina Dubroca @ 2018-10-08 12:36 UTC (permalink / raw)
To: netdev; +Cc: Stefano Brivio, Sabrina Dubroca
Since commit 5aad1de5ea2c ("ipv4: use separate genid for next hop
exceptions"), exceptions get deprecated separately from cached
routes. In particular, administrative changes don't clear PMTU anymore.
As Stefano described in commit e9fa1495d738 ("ipv6: Reflect MTU changes
on PMTU of exceptions for MTU-less routes"), the PMTU discovered before
the local MTU change can become stale:
- if the local MTU is now lower than the PMTU, that PMTU is now
incorrect
- if the local MTU was the lowest value in the path, and is increased,
we might discover a higher PMTU
Similarly to what commit e9fa1495d738 did for IPv6, update PMTU in those
cases.
If the exception was locked, discovered the PMTU was smaller than the
minimal accepted PMTU. In that case, if the new local MTU is smaller
than the current PMTU, let PMTU discovery figure out if locking of the
exception is still needed.
To do this, we need to know the old link MTU in the NETDEV_CHANGEMTU
notifier. By the time the notifier is called, dev->mtu has been
changed. This patch adds the old MTU as additional information in the
notifier structure, and a new call_netdevice_notifiers_u32() function.
Fixes: 5aad1de5ea2c ("ipv4: use separate genid for next hop exceptions")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
---
include/linux/netdevice.h | 7 ++++++
include/net/ip_fib.h | 1 +
net/core/dev.c | 29 +++++++++++++++++++++--
net/ipv4/fib_frontend.c | 12 ++++++----
net/ipv4/fib_semantics.c | 50 +++++++++++++++++++++++++++++++++++++++
5 files changed, 93 insertions(+), 6 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index c7861e4b402c..dc9d2668d9bb 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2458,6 +2458,13 @@ struct netdev_notifier_info {
struct netlink_ext_ack *extack;
};
+struct netdev_notifier_info_ext {
+ struct netdev_notifier_info info; /* must be first */
+ union {
+ u32 u32;
+ } ext;
+};
+
struct netdev_notifier_change_info {
struct netdev_notifier_info info; /* must be first */
unsigned int flags_changed;
diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
index 69c91d1934c1..c9b7b136939d 100644
--- a/include/net/ip_fib.h
+++ b/include/net/ip_fib.h
@@ -394,6 +394,7 @@ int ip_fib_check_default(__be32 gw, struct net_device *dev);
int fib_sync_down_dev(struct net_device *dev, unsigned long event, bool force);
int fib_sync_down_addr(struct net_device *dev, __be32 local);
int fib_sync_up(struct net_device *dev, unsigned int nh_flags);
+void fib_sync_mtu(struct net_device *dev, u32 orig_mtu);
#ifdef CONFIG_IP_ROUTE_MULTIPATH
int fib_multipath_hash(const struct net *net, const struct flowi4 *fl4,
diff --git a/net/core/dev.c b/net/core/dev.c
index 82114e1111e6..f4a505fb5cb2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1752,6 +1752,29 @@ int call_netdevice_notifiers(unsigned long val, struct net_device *dev)
}
EXPORT_SYMBOL(call_netdevice_notifiers);
+/**
+ * call_netdevice_notifiers_u32 - call all network notifier blocks
+ * @val: value passed unmodified to notifier function
+ * @dev: net_device pointer passed unmodified to notifier function
+ * @u32: additional u32 argument passed to the notifier function
+ *
+ * Call all network notifier blocks. Parameters and return value
+ * are as for raw_notifier_call_chain().
+ */
+int call_netdevice_notifiers_u32(unsigned long val, struct net_device *dev,
+ u32 arg)
+{
+ struct netdev_notifier_info_ext info = {
+ .info.dev = dev,
+ .ext.u32 = arg,
+ };
+
+ BUILD_BUG_ON(offsetof(struct netdev_notifier_info_ext, info) != 0);
+
+ return call_netdevice_notifiers_info(val, &info.info);
+}
+EXPORT_SYMBOL(call_netdevice_notifiers_u32);
+
#ifdef CONFIG_NET_INGRESS
static DEFINE_STATIC_KEY_FALSE(ingress_needed_key);
@@ -7574,14 +7597,16 @@ int dev_set_mtu_ext(struct net_device *dev, int new_mtu,
err = __dev_set_mtu(dev, new_mtu);
if (!err) {
- err = call_netdevice_notifiers(NETDEV_CHANGEMTU, dev);
+ err = call_netdevice_notifiers_u32(NETDEV_CHANGEMTU, dev,
+ orig_mtu);
err = notifier_to_errno(err);
if (err) {
/* setting mtu back and notifying everyone again,
* so that they have a chance to revert changes.
*/
__dev_set_mtu(dev, orig_mtu);
- call_netdevice_notifiers(NETDEV_CHANGEMTU, dev);
+ call_netdevice_notifiers_u32(NETDEV_CHANGEMTU, dev,
+ new_mtu);
}
}
return err;
diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 2998b0e47d4b..849198dc79a5 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -1243,7 +1243,8 @@ static int fib_inetaddr_event(struct notifier_block *this, unsigned long event,
static int fib_netdev_event(struct notifier_block *this, unsigned long event, void *ptr)
{
struct net_device *dev = netdev_notifier_info_to_dev(ptr);
- struct netdev_notifier_changeupper_info *info;
+ struct netdev_notifier_changeupper_info *upper_info = ptr;
+ struct netdev_notifier_info_ext *info_ext = ptr;
struct in_device *in_dev;
struct net *net = dev_net(dev);
unsigned int flags;
@@ -1278,16 +1279,19 @@ static int fib_netdev_event(struct notifier_block *this, unsigned long event, vo
fib_sync_up(dev, RTNH_F_LINKDOWN);
else
fib_sync_down_dev(dev, event, false);
- /* fall through */
+ rt_cache_flush(net);
+ break;
case NETDEV_CHANGEMTU:
+ fib_sync_mtu(dev, info_ext->ext.u32);
rt_cache_flush(net);
break;
case NETDEV_CHANGEUPPER:
- info = ptr;
+ upper_info = ptr;
/* flush all routes if dev is linked to or unlinked from
* an L3 master device (e.g., VRF)
*/
- if (info->upper_dev && netif_is_l3_master(info->upper_dev))
+ if (upper_info->upper_dev &&
+ netif_is_l3_master(upper_info->upper_dev))
fib_disable_ip(dev, NETDEV_DOWN, true);
break;
}
diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
index f3c89ccf14c5..446204ca7406 100644
--- a/net/ipv4/fib_semantics.c
+++ b/net/ipv4/fib_semantics.c
@@ -1470,6 +1470,56 @@ static int call_fib_nh_notifiers(struct fib_nh *fib_nh,
return NOTIFY_DONE;
}
+/* Update the PMTU of exceptions when:
+ * - the new MTU of the first hop becomes smaller than the PMTU
+ * - the old MTU was the same as the PMTU, and it limited discovery of
+ * larger MTUs on the path. With that limit raised, we can now
+ * discover larger MTUs
+ * A special case is locked exceptions, for which the PMTU is smaller
+ * than the minimal accepted PMTU:
+ * - if the new MTU is greater than the PMTU, don't make any change
+ * - otherwise, unlock and set PMTU
+ */
+static void nh_update_mtu(struct fib_nh *nh, u32 new, u32 orig)
+{
+ struct fnhe_hash_bucket *bucket;
+ int i;
+
+ bucket = rcu_dereference_protected(nh->nh_exceptions, 1);
+ if (!bucket)
+ return;
+
+ for (i = 0; i < FNHE_HASH_SIZE; i++) {
+ struct fib_nh_exception *fnhe;
+
+ for (fnhe = rcu_dereference_protected(bucket[i].chain, 1);
+ fnhe;
+ fnhe = rcu_dereference_protected(fnhe->fnhe_next, 1)) {
+ if (fnhe->fnhe_mtu_locked) {
+ if (new <= fnhe->fnhe_pmtu) {
+ fnhe->fnhe_pmtu = new;
+ fnhe->fnhe_mtu_locked = false;
+ }
+ } else if (new < fnhe->fnhe_pmtu ||
+ orig == fnhe->fnhe_pmtu) {
+ fnhe->fnhe_pmtu = new;
+ }
+ }
+ }
+}
+
+void fib_sync_mtu(struct net_device *dev, u32 orig_mtu)
+{
+ unsigned int hash = fib_devindex_hashfn(dev->ifindex);
+ struct hlist_head *head = &fib_info_devhash[hash];
+ struct fib_nh *nh;
+
+ hlist_for_each_entry(nh, head, nh_hash) {
+ if (nh->nh_dev == dev)
+ nh_update_mtu(nh, dev->mtu, orig_mtu);
+ }
+}
+
/* Event force Flags Description
* NETDEV_CHANGE 0 LINKDOWN Carrier OFF, not for scope host
* NETDEV_DOWN 0 LINKDOWN|DEAD Link down, not for scope host
--
2.19.0
^ permalink raw reply related
* [PATCH net 2/2] ipv4: don't let PMTU updates increase route MTU
From: Sabrina Dubroca @ 2018-10-08 12:36 UTC (permalink / raw)
To: netdev; +Cc: Stefano Brivio, Sabrina Dubroca
In-Reply-To: <bdb0235df165b1a9684670be3839962c80c9b45a.1539000663.git.sd@queasysnail.net>
When an MTU update with PMTU smaller than net.ipv4.route.min_pmtu is
received, we must clamp its value. However, we can receive a PMTU
exception with PMTU < old_mtu < ip_rt_min_pmtu, which would lead to an
increase in PMTU.
To fix this, take the smallest of the old MTU and ip_rt_min_pmtu.
Before this patch, in case of an update, the exception's MTU would
always change. Now, an exception can have only its lock flag updated,
but not the MTU, so we need to add a check on locking to the following
"is this exception getting updated, or close to expiring?" test.
Fixes: d52e5a7e7ca4 ("ipv4: lock mtu in fnhe when received PMTU < net.ipv4.route.min_pmtu")
Signed-off-by: Sabrina Dubroca <sd@queasysnail.net>
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
---
net/ipv4/route.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index b678466da451..8501554e96a4 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1001,21 +1001,22 @@ out: kfree_skb(skb);
static void __ip_rt_update_pmtu(struct rtable *rt, struct flowi4 *fl4, u32 mtu)
{
struct dst_entry *dst = &rt->dst;
+ u32 old_mtu = ipv4_mtu(dst);
struct fib_result res;
bool lock = false;
if (ip_mtu_locked(dst))
return;
- if (ipv4_mtu(dst) < mtu)
+ if (old_mtu < mtu)
return;
if (mtu < ip_rt_min_pmtu) {
lock = true;
- mtu = ip_rt_min_pmtu;
+ mtu = min(old_mtu, ip_rt_min_pmtu);
}
- if (rt->rt_pmtu == mtu &&
+ if (rt->rt_pmtu == mtu && !lock &&
time_before(jiffies, dst->expires - ip_rt_mtu_expires / 2))
return;
--
2.19.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox