* [PATCH iproute2] etf: make printing of variable JSON friendly
From: Vedang Patel @ 2019-07-19 21:40 UTC (permalink / raw)
To: netdev
Cc: jhs, xiyou.wangcong, jiri, stephen, vinicius.gomes,
leandro.maciel.dorileo, dsahern, Vedang Patel
In iproute2 txtime-assist series, it was pointed out that print_bool()
should be used to print binary values. This is to make it JSON friendly.
So, make the corresponding changes in ETF.
Fixes: 8ccd49383cdc ("etf: Add skip_sock_check")
Reported-by: Stephen Hemminger <stephen@networkplumber.org>
Signed-off-by: Vedang Patel <vedang.patel@intel.com>
---
tc/q_etf.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tc/q_etf.c b/tc/q_etf.c
index c2090589bc64..307c50eed48b 100644
--- a/tc/q_etf.c
+++ b/tc/q_etf.c
@@ -176,12 +176,12 @@ static int etf_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
get_clock_name(qopt->clockid));
print_uint(PRINT_ANY, "delta", "delta %d ", qopt->delta);
- print_string(PRINT_ANY, "offload", "offload %s ",
- (qopt->flags & TC_ETF_OFFLOAD_ON) ? "on" : "off");
- print_string(PRINT_ANY, "deadline_mode", "deadline_mode %s ",
- (qopt->flags & TC_ETF_DEADLINE_MODE_ON) ? "on" : "off");
- print_string(PRINT_ANY, "skip_sock_check", "skip_sock_check %s",
- (qopt->flags & TC_ETF_SKIP_SOCK_CHECK) ? "on" : "off");
+ if (qopt->flags & TC_ETF_OFFLOAD_ON)
+ print_bool(PRINT_ANY, "offload", "offload ", true);
+ if (qopt->flags & TC_ETF_DEADLINE_MODE_ON)
+ print_bool(PRINT_ANY, "deadline_mode", "deadline_mode ", true);
+ if (qopt->flags & TC_ETF_SKIP_SOCK_CHECK)
+ print_bool(PRINT_ANY, "skip_sock_check", "skip_sock_check", true);
return 0;
}
--
2.7.3
^ permalink raw reply related
* Re: [PATCH] be2net: fix adapter->big_page_size miscaculation
From: Qian Cai @ 2019-07-19 21:47 UTC (permalink / raw)
To: David Miller
Cc: morbo, ndesaulniers, jyknight, sathya.perla, ajit.khaparde,
sriharsha.basavapatna, somnath.kotur, arnd, dhowells, hpa, netdev,
linux-arch, linux-kernel, natechancellor
In-Reply-To: <20190718.162928.124906203979938369.davem@davemloft.net>
On Thu, 2019-07-18 at 16:29 -0700, David Miller wrote:
> From: Qian Cai <cai@lca.pw>
> Date: Thu, 18 Jul 2019 19:26:47 -0400
>
> >
> >
> >> On Jul 18, 2019, at 5:21 PM, Bill Wendling <morbo@google.com> wrote:
> >>
> >> [My previous response was marked as spam...]
> >>
> >> Top-of-tree clang says that it's const:
> >>
> >> $ gcc a.c -O2 && ./a.out
> >> a is a const.
> >>
> >> $ clang a.c -O2 && ./a.out
> >> a is a const.
> >
> >
> > I used clang-7.0.1. So, this is getting worse where both GCC and clang will
> start to suffer the
> > same problem.
>
> Then rewrite the module parameter macros such that the non-constness
> is evident to all compilers regardless of version.
>
> That is the place to fix this, otherwise we will just be adding hacks
> all over the place rather than in just one spot.
The problem is that when the compiler is compiling be_main.o, it has no
knowledge about what is going to happen in load_module(). The compiler can only
see that a "const struct kernel_param_ops" "__param_ops_rx_frag_size" at the
time with
__param_ops_rx_frag_size.arg = &rx_frag_size
but only in load_module()->parse_args()->parse_one()->param_set_ushort(), it
changes "__param_ops_rx_frag_size.arg" which in-turn changes the value
of "rx_frag_size".
^ permalink raw reply
* Re: [PATCH bpf] libbpf: fix missing __WORDSIZE definition
From: Andrii Nakryiko @ 2019-07-19 21:48 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo
Cc: Andrii Nakryiko, bpf, Networking, Daniel Borkmann,
Alexei Starovoitov, Kernel Team, Jiri Olsa, Namhyung Kim
In-Reply-To: <20190719202703.GR3624@kernel.org>
On Fri, Jul 19, 2019 at 1:27 PM Arnaldo Carvalho de Melo
<arnaldo.melo@gmail.com> wrote:
>
> Em Fri, Jul 19, 2019 at 01:04:32PM -0700, Andrii Nakryiko escreveu:
> > On Fri, Jul 19, 2019 at 11:34 AM Arnaldo Carvalho de Melo
> > <arnaldo.melo@gmail.com> wrote:
> > >
> > > Em Fri, Jul 19, 2019 at 11:26:50AM -0700, Andrii Nakryiko escreveu:
> > > > On Fri, Jul 19, 2019 at 11:14 AM Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com> wrote:
> > > > > Em Fri, Jul 19, 2019 at 10:54:44AM -0700, Andrii Nakryiko escreveu:
> > > > > > Ok, did some more googling. This warning (turned error in your setup)
> > > > > > is emitted when -Wshadow option is enabled for GCC/clang. It appears
> > > > > > to be disabled by default, so it must be enabled somewhere for perf
> > > > > > build or something.
> > >
> > > > > Right, I came to the exact same conclusion, doing tests here:
> > >
> > > > > [perfbuilder@3a58896a648d tmp]$ gcc -Wshadow shadow_global_decl.c -o shadow_global_decl
> > > > > shadow_global_decl.c: In function 'main':
> > > > > shadow_global_decl.c:9: warning: declaration of 'link' shadows a global declaration
> > > > > shadow_global_decl.c:4: warning: shadowed declaration is here
> > > > > [perfbuilder@3a58896a648d tmp]$ gcc --version |& head -1
> > > > > gcc (GCC) 4.4.7 20120313 (Red Hat 4.4.7-23)
> > > > > [perfbuilder@3a58896a648d tmp]$ gcc shadow_global_decl.c -o shadow_global_decl
> > > > > [perfbuilder@3a58896a648d tmp]$
> > >
> > > > > So I'm going to remove this warning from the places where it causes
> > > > > problems.
> > >
> > > > > > Would it be possible to disable it at least for libbpf when building
> > > > > > from perf either everywhere or for those systems where you see this
> > > > > > warning? I don't think this warning is useful, to be honest, just
> > > > > > random name conflict between any local and global variables will cause
> > > > > > this.
> > >
> > > > > Yeah, I might end up having this applied.
> > >
> > > > Thanks!
> > >
> > > So, I'm ending up with the patch below, there is some value after all in
> > > Wshadow, that is, from gcc 4.8 onwards :-)
> >
> > I agree with the intent, but see below.
> >
> > >
> > > - Arnaldo
> > >
> > > diff --git a/tools/scripts/Makefile.include b/tools/scripts/Makefile.include
> > > index 495066bafbe3..ded7a950dc40 100644
> > > --- a/tools/scripts/Makefile.include
> > > +++ b/tools/scripts/Makefile.include
> > > @@ -32,7 +32,6 @@ EXTRA_WARNINGS += -Wno-system-headers
> > > EXTRA_WARNINGS += -Wold-style-definition
> > > EXTRA_WARNINGS += -Wpacked
> > > EXTRA_WARNINGS += -Wredundant-decls
> > > -EXTRA_WARNINGS += -Wshadow
> > > EXTRA_WARNINGS += -Wstrict-prototypes
> > > EXTRA_WARNINGS += -Wswitch-default
> > > EXTRA_WARNINGS += -Wswitch-enum
> > > @@ -69,8 +68,16 @@ endif
> > > # will do for now and keep the above -Wstrict-aliasing=3 in place
> > > # in newer systems.
> > > # Needed for the __raw_cmpxchg in tools/arch/x86/include/asm/cmpxchg.h
> > > +#
> > > +# See https://lkml.org/lkml/2006/11/28/253 and https://gcc.gnu.org/gcc-4.8/changes.html,
> > > +# that takes into account Linus's comments (search for Wshadow) for the reasoning about
> > > +# -Wshadow not being interesting before gcc 4.8.
> > > +
> > > ifneq ($(filter 3.%,$(MAKE_VERSION)),) # make-3
> >
> > This is checking make version, not GCC version. So code comment and
> > configurations are not in sync?
>
> Ah, I should have added a few lines back:
>
> # Hack to avoid type-punned warnings on old systems such as RHEL5:
> # We should be changing CFLAGS and checking gcc version, but this
> # will do for now and keep the above -Wstrict-aliasing=3 in place
> # in newer systems.
> # Needed for the __raw_cmpxchg in tools/arch/x86/include/asm/cmpxchg.h
> #
> # See https://lkml.org/lkml/2006/11/28/253 and https://gcc.gnu.org/gcc-4.8/changes.html,
> # that takes into account Linus's comments (search for Wshadow) for the reasoning about
> # -Wshadow not being interesting before gcc 4.8.
>
>
> In time I'll try and get it to use the gcc version to be strict.
Oh well, if it's how it's done right now :)
Acked-by: Andrii Nakryiko <andriin@fb.com>
>
> - Arnaldo
^ permalink raw reply
* Re: [PATCH iproute2] etf: make printing of variable JSON friendly
From: Stephen Hemminger @ 2019-07-19 22:26 UTC (permalink / raw)
To: Vedang Patel
Cc: netdev, jhs, xiyou.wangcong, jiri, vinicius.gomes,
leandro.maciel.dorileo, dsahern
In-Reply-To: <1563572443-10879-1-git-send-email-vedang.patel@intel.com>
On Fri, 19 Jul 2019 14:40:43 -0700
Vedang Patel <vedang.patel@intel.com> wrote:
> In iproute2 txtime-assist series, it was pointed out that print_bool()
> should be used to print binary values. This is to make it JSON friendly.
>
> So, make the corresponding changes in ETF.
>
> Fixes: 8ccd49383cdc ("etf: Add skip_sock_check")
> Reported-by: Stephen Hemminger <stephen@networkplumber.org>
> Signed-off-by: Vedang Patel <vedang.patel@intel.com>
> ---
> tc/q_etf.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/tc/q_etf.c b/tc/q_etf.c
> index c2090589bc64..307c50eed48b 100644
> --- a/tc/q_etf.c
> +++ b/tc/q_etf.c
> @@ -176,12 +176,12 @@ static int etf_print_opt(struct qdisc_util *qu, FILE *f, struct rtattr *opt)
> get_clock_name(qopt->clockid));
>
> print_uint(PRINT_ANY, "delta", "delta %d ", qopt->delta);
> - print_string(PRINT_ANY, "offload", "offload %s ",
> - (qopt->flags & TC_ETF_OFFLOAD_ON) ? "on" : "off");
> - print_string(PRINT_ANY, "deadline_mode", "deadline_mode %s ",
> - (qopt->flags & TC_ETF_DEADLINE_MODE_ON) ? "on" : "off");
> - print_string(PRINT_ANY, "skip_sock_check", "skip_sock_check %s",
> - (qopt->flags & TC_ETF_SKIP_SOCK_CHECK) ? "on" : "off");
> + if (qopt->flags & TC_ETF_OFFLOAD_ON)
> + print_bool(PRINT_ANY, "offload", "offload ", true);
> + if (qopt->flags & TC_ETF_DEADLINE_MODE_ON)
> + print_bool(PRINT_ANY, "deadline_mode", "deadline_mode ", true);
> + if (qopt->flags & TC_ETF_SKIP_SOCK_CHECK)
> + print_bool(PRINT_ANY, "skip_sock_check", "skip_sock_check", true);
>
> return 0;
> }
Thanks, but if you are only going to print json boolean if true, then a common
way to indicate that something is enabled is to use print_null().
Could you do that instead?
^ permalink raw reply
* Re: [PATCH 2/2] mwifiex: Make use of the new sdio_trigger_replug() API to reset
From: Brian Norris @ 2019-07-19 23:36 UTC (permalink / raw)
To: Douglas Anderson
Cc: Ulf Hansson, Kalle Valo, Adrian Hunter, Ganapathi Bhat,
linux-wireless, Amitkumar Karwar, linux-rockchip, Wolfram Sang,
Nishant Sarmukadam, netdev, Avri Altman, linux-mmc, davem,
Xinming Hu, linux-kernel, Andreas Fenkart
In-Reply-To: <20190716164209.62320-3-dianders@chromium.org>
Hi Doug,
On Tue, Jul 16, 2019 at 09:42:09AM -0700, Doug Anderson wrote:
> As described in the patch ("mmc: core: Add sdio_trigger_replug()
> API"), the current mwifiex_sdio_card_reset() is broken in the cases
> where we're running Bluetooth on a second SDIO func on the same card
> as WiFi. The problem goes away if we just use the
> sdio_trigger_replug() API call.
I'm unfortunately not a good evaluator of SDIO/MMC stuff, so I'll mostly
leave that to others and assume that the "replug" description is pretty
much all I need to know.
> NOTE: Even though with this new solution there is less of a reason to
> do our work from a workqueue (the unplug / plug mechanism we're using
> is possible for a human to perform at any time so the stack is
> supposed to handle it without it needing to be called from a special
> context), we still need a workqueue because the Marvell reset function
> could called from a context where sleeping is invalid and thus we
> can't claim the host. One example is Marvell's wakeup_timer_fn().
>
> Signed-off-by: Douglas Anderson <dianders@chromium.org>
> ---
>
> drivers/net/wireless/marvell/mwifiex/sdio.c | 14 +++-----------
> 1 file changed, 3 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/sdio.c b/drivers/net/wireless/marvell/mwifiex/sdio.c
> index 24c041dad9f6..f77ad2615f08 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> @@ -2218,14 +2218,6 @@ static void mwifiex_sdio_card_reset_work(struct mwifiex_adapter *adapter)
> {
> struct sdio_mmc_card *card = adapter->card;
> struct sdio_func *func = card->func;
> - int ret;
> -
> - mwifiex_shutdown_sw(adapter);
I'm very mildly unhappy to see this driver diverge from the PCIe one
again, but the only way it makes sense to do things the same is if there
is such thing as a "function level reset" for SDIO (i.e., doesn't also
kill the Bluetooth function). But it appears we don't really have such a
thing.
> -
> - /* power cycle the adapter */
> - sdio_claim_host(func);
> - mmc_hw_reset(func->card->host);
> - sdio_release_host(func);
>
> /* Previous save_adapter won't be valid after this. We will cancel
^^^ FTR, the "save_adapter" note was already obsolete as of
cc75c577806a mwifiex: get rid of global save_adapter and sdio_work
but the clear_bit() calls were (before this patch) still useful for
other reasons.
> * pending work requests.
> @@ -2233,9 +2225,9 @@ static void mwifiex_sdio_card_reset_work(struct mwifiex_adapter *adapter)
> clear_bit(MWIFIEX_IFACE_WORK_DEVICE_DUMP, &card->work_flags);
> clear_bit(MWIFIEX_IFACE_WORK_CARD_RESET, &card->work_flags);
But now, I don't think you need these clear_bit() calls any more --
you're totally destroying the card and its workqueue on remove(). (And
anyway, MWIFIEX_IFACE_WORK_CARD_RESET was just cleared by your caller.)
>
> - ret = mwifiex_reinit_sw(adapter);
> - if (ret)
> - dev_err(&func->dev, "reinit failed: %d\n", ret);
> + sdio_claim_host(func);
> + sdio_trigger_replug(func);
> + sdio_release_host(func);
And...we're approximately back to where we were 4 years ago :)
commit b4336a282db86b298b70563f8ed51782b36b772c
Author: Andreas Fenkart <afenkart@gmail.com>
Date: Thu Jul 16 18:50:01 2015 +0200
mwifiex: sdio: reset adapter using mmc_hw_reset
Anyway, assuming the "function reset" thing isn't workable, and you drop
the clear_bit() stuff, I think this is fine:
Reviewed-by: Brian Norris <briannorris@chromium.org>
> }
>
> /* This function read/write firmware */
> --
> 2.22.0.510.g264f2c817a-goog
>
^ permalink raw reply
* Re: [PATCH ghak90 V6 02/10] audit: add container id
From: James Bottomley @ 2019-07-20 2:19 UTC (permalink / raw)
To: Eric W. Biederman, Paul Moore
Cc: nhorman, linux-api, containers, LKML, omosnace, dhowells,
Linux-Audit Mailing List, netfilter-devel, simo, netdev,
linux-fsdevel, Eric Paris, sgrubb
In-Reply-To: <87muhadnfr.fsf@xmission.com>
On Fri, 2019-07-19 at 11:00 -0500, Eric W. Biederman wrote:
> Paul Moore <paul@paul-moore.com> writes:
>
> > On Wed, Jul 17, 2019 at 8:52 PM Richard Guy Briggs <rgb@redhat.com>
> > wrote:
> > > On 2019-07-16 19:30, Paul Moore wrote:
> >
> > ...
> >
> > > > We can trust capable(CAP_AUDIT_CONTROL) for enforcing audit
> > > > container ID policy, we can not trust
> > > > ns_capable(CAP_AUDIT_CONTROL).
> > >
> > > Ok. So does a process in a non-init user namespace have two (or
> > > more) sets of capabilities stored in creds, one in the
> > > init_user_ns, and one in current_user_ns? Or does it get
> > > stripped of all its capabilities in init_user_ns once it has its
> > > own set in current_user_ns? If the former, then we can use
> > > capable(). If the latter, we need another mechanism, as
> > > you have suggested might be needed.
> >
> > Unfortunately I think the problem is that ultimately we need to
> > allow any container orchestrator that has been given privileges to
> > manage the audit container ID to also grant that privilege to any
> > of the child process/containers it manages. I don't believe we can
> > do that with capabilities based on the code I've looked at, and the
> > discussions I've had, but if you find a way I would leave to hear
> > it.
> > > If some random unprivileged user wants to fire up a container
> > > orchestrator/engine in his own user namespace, then audit needs
> > > to be namespaced. Can we safely discard this scenario for now?
> >
> > I think the only time we want to allow a container orchestrator to
> > manage the audit container ID is if it has been granted that
> > privilege by someone who has that privilege already. In the zero-
> > container, or single-level of containers, case this is relatively
> > easy, and we can accomplish it using CAP_AUDIT_CONTROL as the
> > privilege. If we start nesting container orchestrators it becomes
> > more complicated as we need to be able to support granting and
> > inheriting this privilege in a manner; this is why I suggested a
> > new mechanism *may* be necessary.
>
>
> Let me segway a bit and see if I can get this conversation out of the
> rut it seems to have drifted into.
>
> Unprivileged containers and nested containers exist today and are
> going to become increasingly common. Let that be a given.
Agree fully.
> As I recall the interesting thing for audit to log is actions by
> privileged processes. Audit can log more but generally configuring
> logging by of the actions of unprivileged users is effectively a self
> DOS.
>
> So I think the initial implementation can safely ignore actions of
> nested containers and unprivileged containers because you don't care
> about their actions.
I don't entirely agree here: remember there might be two consumers for
the audit data: the physical system owner (checking up on the tenants)
and the tenant themselves who might be watching either their sub
tenants or their users (and who, obviously, won't get the full audit
stream). In either case, the tenant may or may not be privileged, and
if they're privileged, it might be through the user_ns in which case
the physical system owner and the kernel would see them as "not
privileged". So I think we are ultimately going to need the ability to
audit unprivileged containers.
I also think audit has a role to play in intrusion detection and
forensic analysis for fully unprivileged containers running external
services, but I don't think we have to solve that case immediately.
> If we start allow running audit in a container then we need to deal
> with all of the nesting issues but until then I don't think you folks
> care.
>
> Or am I wrong. Do the requirements for securely auditing things from
> the kernel care about the actions of unprivileged users?
I think ultimately we have to care, but it could be three phases: first
would be genuinely privileged containers (i.e. with real root inside,
being our most dangerous problem) the second would be user_ns
privileged containers (i.e. with both user_ns and an interior root
mapping) and the third would be unprivileged containers (with or
without user_ns but no interior root).
James
^ permalink raw reply
* Re: [patch net-next rfc 4/7] net: rtnetlink: put alternative names to getlink message
From: Jakub Kicinski @ 2019-07-20 3:59 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, sthemmin, dsahern, dcbw, mkubecek, andrew, parav,
saeedm, mlxsw
In-Reply-To: <20190719110029.29466-5-jiri@resnulli.us>
On Fri, 19 Jul 2019 13:00:26 +0200, Jiri Pirko wrote:
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 7a2010b16e10..f11a2367037d 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -980,6 +980,18 @@ static size_t rtnl_xdp_size(void)
> return xdp_size;
> }
>
> +static size_t rtnl_alt_ifname_list_size(const struct net_device *dev)
> +{
> + struct netdev_name_node *name_node;
> + size_t size = nla_total_size(0);
> +
> + if (list_empty(&dev->name_node->list))
> + return 0;
Nit: it would make the intent a tiny bit clearer if
size = nla_total_size(0);
was after this early return.
> + list_for_each_entry(name_node, &dev->name_node->list, list)
> + size += nla_total_size(ALTIFNAMSIZ);
Since we have the structure I wonder if it would be worthwhile to store
the exact size in it?
> + return size;
> +}
> +
^ permalink raw reply
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
From: Jakub Kicinski @ 2019-07-20 3:58 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, sthemmin, dsahern, dcbw, mkubecek, andrew, parav,
saeedm, mlxsw
In-Reply-To: <20190719110029.29466-4-jiri@resnulli.us>
On Fri, 19 Jul 2019 13:00:25 +0200, Jiri Pirko wrote:
> +int netdev_name_node_alt_destroy(struct net_device *dev, char *name)
> +{
> + struct netdev_name_node *name_node;
> + struct net *net = dev_net(dev);
> +
> + name_node = netdev_name_node_lookup(net, name);
> + if (!name_node)
> + return -ENOENT;
> + __netdev_name_node_alt_destroy(name_node);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(netdev_name_node_alt_destroy);
I was surprised to see the exports are they strictly necessary?
Just wondering..
> @@ -8258,6 +8313,7 @@ static void rollback_registered_many(struct list_head *head)
> dev_uc_flush(dev);
> dev_mc_flush(dev);
>
> + netdev_name_node_alt_flush(dev);
> netdev_name_node_free(dev->name_node);
>
> if (dev->netdev_ops->ndo_uninit)
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 1ee6460f8275..7a2010b16e10 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1750,6 +1750,8 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
> [IFLA_CARRIER_DOWN_COUNT] = { .type = NLA_U32 },
> [IFLA_MIN_MTU] = { .type = NLA_U32 },
> [IFLA_MAX_MTU] = { .type = NLA_U32 },
> + [IFLA_ALT_IFNAME_MOD] = { .type = NLA_STRING,
> + .len = ALTIFNAMSIZ - 1 },
Should we set:
.strict_start_type = IFLA_ALT_IFNAME_MOD
?
> };
>
> static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
^ permalink raw reply
* Re: [patch net-next rfc 7/7] net: rtnetlink: add possibility to use alternative names as message handle
From: Jakub Kicinski @ 2019-07-20 3:59 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, sthemmin, dsahern, dcbw, mkubecek, andrew, parav,
saeedm, mlxsw
In-Reply-To: <20190719110029.29466-8-jiri@resnulli.us>
On Fri, 19 Jul 2019 13:00:29 +0200, Jiri Pirko wrote:
> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
> index 1fa30d514e3f..68ad12a7fc4d 100644
> --- a/net/core/rtnetlink.c
> +++ b/net/core/rtnetlink.c
> @@ -1793,6 +1793,8 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
> [IFLA_MAX_MTU] = { .type = NLA_U32 },
> [IFLA_ALT_IFNAME_MOD] = { .type = NLA_STRING,
> .len = ALTIFNAMSIZ - 1 },
> + [IFLA_ALT_IFNAME] = { .type = NLA_STRING,
> + .len = ALTIFNAMSIZ - 1 },
What's the disadvantage of just letting IFLA_IFNAME to get longer
on input? Is it just that the handling would be asymmetrical?
> };
>
> static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
^ permalink raw reply
* Re: [PATCH 00/14] Netfilter fixes for net
From: David Miller @ 2019-07-20 4:25 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <20190719164517.29496-1-pablo@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Fri, 19 Jul 2019 18:45:03 +0200
> The following patchset contains Netfilter fixes for net:
...
> You can pull these changes from:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/pablo/nf.git
Pulled.
^ permalink raw reply
* Re: [PATCH nf,v5 0/4] flow_offload fixes
From: David Miller @ 2019-07-20 4:28 UTC (permalink / raw)
To: pablo; +Cc: netfilter-devel, netdev, jiri, jakub.kicinski, pshelar
In-Reply-To: <20190719162016.10243-1-pablo@netfilter.org>
From: Pablo Neira Ayuso <pablo@netfilter.org>
Date: Fri, 19 Jul 2019 18:20:12 +0200
> The following patchset contains fixes for the flow_offload infrastructure:
Series applied, please fix the build failure reported by Jakub.
^ permalink raw reply
* Re: [patch net-next rfc 2/7] net: introduce name_node struct to be used in hashlist
From: Jiri Pirko @ 2019-07-20 7:15 UTC (permalink / raw)
To: Stephen Hemminger
Cc: netdev, davem, jakub.kicinski, sthemmin, dsahern, dcbw, mkubecek,
andrew, parav, saeedm, mlxsw
In-Reply-To: <20190719132649.700e6a5c@hermes.lan>
Fri, Jul 19, 2019 at 10:26:49PM CEST, stephen@networkplumber.org wrote:
>On Fri, 19 Jul 2019 21:17:40 +0200
>Jiri Pirko <jiri@resnulli.us> wrote:
>
>> Fri, Jul 19, 2019 at 06:29:36PM CEST, stephen@networkplumber.org wrote:
>> >On Fri, 19 Jul 2019 13:00:24 +0200
>> >Jiri Pirko <jiri@resnulli.us> wrote:
>> >
>> >> From: Jiri Pirko <jiri@mellanox.com>
>> >>
>> >> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
>> >> ---
>> >> include/linux/netdevice.h | 10 +++-
>> >> net/core/dev.c | 96 +++++++++++++++++++++++++++++++--------
>> >> 2 files changed, 86 insertions(+), 20 deletions(-)
>> >>
>> >> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> >> index 88292953aa6f..74f99f127b0e 100644
>> >> --- a/include/linux/netdevice.h
>> >> +++ b/include/linux/netdevice.h
>> >> @@ -918,6 +918,12 @@ struct dev_ifalias {
>> >> struct devlink;
>> >> struct tlsdev_ops;
>> >>
>> >> +struct netdev_name_node {
>> >> + struct hlist_node hlist;
>> >> + struct net_device *dev;
>> >> + char *name
>> >
>> >You probably can make this const char *
>
>Don't bother, it looks ok as is. the problem is you would have
>to cast it when calling free.
>
>> >Do you want to add __rcu to this list?
>>
>> Which list?
>>
>
> struct netdev_name_node __rcu *name_node;
>
>You might also want to explictly init the hlist node rather
>than relying on the fact that zero is an empty node ptr.
Okay. Will process this in. Thanks!
>
>
> static struct netdev_name_node *netdev_name_node_alloc(struct net_device *dev,
>- char *name)
>+ const char *name)
> {
> struct netdev_name_node *name_node;
>
>- name_node = kzalloc(sizeof(*name_node), GFP_KERNEL);
>+ name_node = kmalloc(sizeof(*name_node));
> if (!name_node)
> return NULL;
>+
>+ INIT_HLIST_NODE(&name_node->hlist);
> name_node->dev = dev;
> name_node->name = name;
> return name_node;
>
^ permalink raw reply
* Re: [patch net-next rfc 4/7] net: rtnetlink: put alternative names to getlink message
From: Jiri Pirko @ 2019-07-20 7:17 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, davem, sthemmin, dsahern, dcbw, mkubecek, andrew, parav,
saeedm, mlxsw
In-Reply-To: <20190719205914.3fc786f6@cakuba>
Sat, Jul 20, 2019 at 05:59:14AM CEST, jakub.kicinski@netronome.com wrote:
>On Fri, 19 Jul 2019 13:00:26 +0200, Jiri Pirko wrote:
>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index 7a2010b16e10..f11a2367037d 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -980,6 +980,18 @@ static size_t rtnl_xdp_size(void)
>> return xdp_size;
>> }
>>
>> +static size_t rtnl_alt_ifname_list_size(const struct net_device *dev)
>> +{
>> + struct netdev_name_node *name_node;
>> + size_t size = nla_total_size(0);
>> +
>> + if (list_empty(&dev->name_node->list))
>> + return 0;
>
>Nit: it would make the intent a tiny bit clearer if
>
> size = nla_total_size(0);
>
>was after this early return.
Sure.
>
>> + list_for_each_entry(name_node, &dev->name_node->list, list)
>> + size += nla_total_size(ALTIFNAMSIZ);
>
>Since we have the structure I wonder if it would be worthwhile to store
Which structure?
>the exact size in it?
>
>> + return size;
>> +}
>> +
^ permalink raw reply
* Re: [patch net-next rfc 3/7] net: rtnetlink: add commands to add and delete alternative ifnames
From: Jiri Pirko @ 2019-07-20 7:20 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, davem, sthemmin, dsahern, dcbw, mkubecek, andrew, parav,
saeedm, mlxsw
In-Reply-To: <20190719205849.11d17192@cakuba>
Sat, Jul 20, 2019 at 05:58:49AM CEST, jakub.kicinski@netronome.com wrote:
>On Fri, 19 Jul 2019 13:00:25 +0200, Jiri Pirko wrote:
>> +int netdev_name_node_alt_destroy(struct net_device *dev, char *name)
>> +{
>> + struct netdev_name_node *name_node;
>> + struct net *net = dev_net(dev);
>> +
>> + name_node = netdev_name_node_lookup(net, name);
>> + if (!name_node)
>> + return -ENOENT;
>> + __netdev_name_node_alt_destroy(name_node);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(netdev_name_node_alt_destroy);
>
>I was surprised to see the exports are they strictly necessary?
Well I call them from net/core/rtnetlink.c, so yes. Maybe I'm missing
your point...
>Just wondering..
>
>> @@ -8258,6 +8313,7 @@ static void rollback_registered_many(struct list_head *head)
>> dev_uc_flush(dev);
>> dev_mc_flush(dev);
>>
>> + netdev_name_node_alt_flush(dev);
>> netdev_name_node_free(dev->name_node);
>>
>> if (dev->netdev_ops->ndo_uninit)
>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index 1ee6460f8275..7a2010b16e10 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -1750,6 +1750,8 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
>> [IFLA_CARRIER_DOWN_COUNT] = { .type = NLA_U32 },
>> [IFLA_MIN_MTU] = { .type = NLA_U32 },
>> [IFLA_MAX_MTU] = { .type = NLA_U32 },
>> + [IFLA_ALT_IFNAME_MOD] = { .type = NLA_STRING,
>> + .len = ALTIFNAMSIZ - 1 },
>
>Should we set:
>
> .strict_start_type = IFLA_ALT_IFNAME_MOD
Probably yes. Will add it.
>
>?
>
>> };
>>
>> static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
^ permalink raw reply
* Re: [patch net-next rfc 7/7] net: rtnetlink: add possibility to use alternative names as message handle
From: Jiri Pirko @ 2019-07-20 7:22 UTC (permalink / raw)
To: Jakub Kicinski
Cc: netdev, davem, sthemmin, dsahern, dcbw, mkubecek, andrew, parav,
saeedm, mlxsw
In-Reply-To: <20190719205927.6638187f@cakuba>
Sat, Jul 20, 2019 at 05:59:27AM CEST, jakub.kicinski@netronome.com wrote:
>On Fri, 19 Jul 2019 13:00:29 +0200, Jiri Pirko wrote:
>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index 1fa30d514e3f..68ad12a7fc4d 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -1793,6 +1793,8 @@ static const struct nla_policy ifla_policy[IFLA_MAX+1] = {
>> [IFLA_MAX_MTU] = { .type = NLA_U32 },
>> [IFLA_ALT_IFNAME_MOD] = { .type = NLA_STRING,
>> .len = ALTIFNAMSIZ - 1 },
>> + [IFLA_ALT_IFNAME] = { .type = NLA_STRING,
>> + .len = ALTIFNAMSIZ - 1 },
>
>What's the disadvantage of just letting IFLA_IFNAME to get longer
>on input? Is it just that the handling would be asymmetrical?
Hmm, that might work. But the problem is that sometimes the IFLA_IFNAME
is used as a handle, but sometimes it is used to carry the ifname
(newlink, changename). That might be a bit confusing and the code would
be harder to follow. I don't know...
>
>> };
>>
>> static const struct nla_policy ifla_info_policy[IFLA_INFO_MAX+1] = {
>
^ permalink raw reply
* Re: ENOBUILD in nf_tables
From: Pablo Neira Ayuso @ 2019-07-20 7:44 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: netdev@vger.kernel.org
In-Reply-To: <20190719100743.2ea14575@cakuba.netronome.com>
Hi Jakub,
On Fri, Jul 19, 2019 at 10:07:43AM -0700, Jakub Kicinski wrote:
> Hi Pablo!
>
> I hit the following build breakage on net with the config attached.
>
> GCC 9, doesn't seem like your just-posted series fixes this?
$ gcc -v
...
gcc version 9.1.0
$ make
...
CC include/net/netfilter/nf_tables_offload.h.s
...
$
Works for me here.
^ permalink raw reply
* Re: network problems with r8169
From: Heiner Kallweit @ 2019-07-20 9:11 UTC (permalink / raw)
To: Thomas Voegtle; +Cc: linux-kernel, netdev@vger.kernel.org
In-Reply-To: <alpine.LSU.2.21.1907192310140.11569@er-systems.de>
On 19.07.2019 23:12, Thomas Voegtle wrote:
> On Fri, 19 Jul 2019, Heiner Kallweit wrote:
>
>> On 18.07.2019 20:50, Thomas Voegtle wrote:
>>>
>>> Hello,
>>>
>>> I'm having network problems with the commits on r8169 since v5.2. There are ping packet loss, sometimes 100%, sometimes 50%. In the end network is unusable.
>>>
>>> v5.2 is fine, I bisected it down to:
>>>
>>> a2928d28643e3c064ff41397281d20c445525032 is the first bad commit
>>> commit a2928d28643e3c064ff41397281d20c445525032
>>> Author: Heiner Kallweit <hkallweit1@gmail.com>
>>> Date: Sun Jun 2 10:53:49 2019 +0200
>>>
>>> r8169: use paged versions of phylib MDIO access functions
>>>
>>> Use paged versions of phylib MDIO access functions to simplify
>>> the code.
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>> Signed-off-by: David S. Miller <davem@davemloft.net>
>>>
>>>
>>> Reverting that commit on top of v5.2-11564-g22051d9c4a57 fixes the problem
>>> for me (had to adjust the renaming to r8169_main.c).
>>>
>>> I have a:
>>> 04:00.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd.
>>> RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller [10ec:8168] (rev
>>> 0c)
>>> Subsystem: Biostar Microtech Int'l Corp Device [1565:2400]
>>> Kernel driver in use: r8169
>>>
>>> on a BIOSTAR H81MG motherboard.
>>>
>> Interesting. I have the same chip version (RTL8168g) and can't reproduce
>> the issue. Can you provide a full dmesg output and test the patch below
>> on top of linux-next? I'd be interested in the WARN_ON stack traces
>> (if any) and would like to know whether the experimental change to
>> __phy_modify_changed helps.
>>
>>>
>>> greetings,
>>>
>>> Thomas
>>>
>>>
>> Heiner
>>
>>
>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
>> index 8d7dd4c5f..26be73000 100644
>> --- a/drivers/net/ethernet/realtek/r8169_main.c
>> +++ b/drivers/net/ethernet/realtek/r8169_main.c
>> @@ -1934,6 +1934,8 @@ static int rtl_get_eee_supp(struct rtl8169_private *tp)
>> struct phy_device *phydev = tp->phydev;
>> int ret;
>>
>> + WARN_ON(phy_read(phydev, 0x1f));
>> +
>> switch (tp->mac_version) {
>> case RTL_GIGA_MAC_VER_34:
>> case RTL_GIGA_MAC_VER_35:
>> @@ -1957,6 +1959,8 @@ static int rtl_get_eee_lpadv(struct rtl8169_private *tp)
>> struct phy_device *phydev = tp->phydev;
>> int ret;
>>
>> + WARN_ON(phy_read(phydev, 0x1f));
>> +
>> switch (tp->mac_version) {
>> case RTL_GIGA_MAC_VER_34:
>> case RTL_GIGA_MAC_VER_35:
>> @@ -1980,6 +1984,8 @@ static int rtl_get_eee_adv(struct rtl8169_private *tp)
>> struct phy_device *phydev = tp->phydev;
>> int ret;
>>
>> + WARN_ON(phy_read(phydev, 0x1f));
>> +
>> switch (tp->mac_version) {
>> case RTL_GIGA_MAC_VER_34:
>> case RTL_GIGA_MAC_VER_35:
>> @@ -2003,6 +2009,8 @@ static int rtl_set_eee_adv(struct rtl8169_private *tp, int val)
>> struct phy_device *phydev = tp->phydev;
>> int ret = 0;
>>
>> + WARN_ON(phy_read(phydev, 0x1f));
>> +
>> switch (tp->mac_version) {
>> case RTL_GIGA_MAC_VER_34:
>> case RTL_GIGA_MAC_VER_35:
>> diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
>> index 16667fbac..1aa1142b8 100644
>> --- a/drivers/net/phy/phy-core.c
>> +++ b/drivers/net/phy/phy-core.c
>> @@ -463,12 +463,10 @@ int __phy_modify_changed(struct phy_device *phydev, u32 regnum, u16 mask,
>> return ret;
>>
>> new = (ret & ~mask) | set;
>> - if (new == ret)
>> - return 0;
>>
>> - ret = __phy_write(phydev, regnum, new);
>> + __phy_write(phydev, regnum, new);
>>
>> - return ret < 0 ? ret : 1;
>> + return new != ret;
>> }
>> EXPORT_SYMBOL_GPL(__phy_modify_changed);
>>
>>
>
> Took your patch on top of next-20190719.
> See attached dmesg.
> It didn't work. Same thing, lots of ping drops, no usable network.
>
> like that:
> 44 packets transmitted, 2 received, 95% packet loss, time 44005ms
>
>
> Maybe important:
> I build a kernel with no modules.
>
> I have to power off when I booted a kernel which doesn't work, a (soft) reboot into a older kernel (e.g. 4.9.y) doesn't
> fix the problem. Powering off and on does.
>
Then, what you could do is reversing the hunks of the patch step by step.
Or make them separate patches and bisect.
Relevant are the hunks from point 1 and 2.
1. first 5 hunks (I don't think you have to reverse them individually)
EEE-related
2. rtl8168g_disable_aldps, rtl8168g_phy_adjust_10m_aldps, rtl8168g_1_hw_phy_config
all of these hunks are in the path for RTL8168g
3. rtl8168h_1_hw_phy_config, rtl8168h_2_hw_phy_config, rtl8168ep_1_hw_phy_config,
rtl8168ep_2_hw_phy_config
not in the path for RTL8168g
>
> greetings,
>
> Thomas
Heiner
^ permalink raw reply
* Re: network problems with r8169
From: Heiner Kallweit @ 2019-07-20 9:13 UTC (permalink / raw)
To: Thomas Voegtle; +Cc: linux-kernel, netdev@vger.kernel.org
In-Reply-To: <alpine.LSU.2.21.1907192310140.11569@er-systems.de>
On 19.07.2019 23:12, Thomas Voegtle wrote:
> On Fri, 19 Jul 2019, Heiner Kallweit wrote:
>
>> On 18.07.2019 20:50, Thomas Voegtle wrote:
>>>
>>> Hello,
>>>
>>> I'm having network problems with the commits on r8169 since v5.2. There are ping packet loss, sometimes 100%, sometimes 50%. In the end network is unusable.
>>>
>>> v5.2 is fine, I bisected it down to:
>>>
>>> a2928d28643e3c064ff41397281d20c445525032 is the first bad commit
>>> commit a2928d28643e3c064ff41397281d20c445525032
>>> Author: Heiner Kallweit <hkallweit1@gmail.com>
>>> Date: Sun Jun 2 10:53:49 2019 +0200
>>>
>>> r8169: use paged versions of phylib MDIO access functions
>>>
>>> Use paged versions of phylib MDIO access functions to simplify
>>> the code.
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>> Signed-off-by: David S. Miller <davem@davemloft.net>
>>>
>>>
>>> Reverting that commit on top of v5.2-11564-g22051d9c4a57 fixes the problem
>>> for me (had to adjust the renaming to r8169_main.c).
>>>
>>> I have a:
>>> 04:00.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd.
>>> RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller [10ec:8168] (rev
>>> 0c)
>>> Subsystem: Biostar Microtech Int'l Corp Device [1565:2400]
>>> Kernel driver in use: r8169
>>>
>>> on a BIOSTAR H81MG motherboard.
>>>
>> Interesting. I have the same chip version (RTL8168g) and can't reproduce
>> the issue. Can you provide a full dmesg output and test the patch below
>> on top of linux-next? I'd be interested in the WARN_ON stack traces
>> (if any) and would like to know whether the experimental change to
>> __phy_modify_changed helps.
>>
>>>
>>> greetings,
>>>
>>> Thomas
>>>
>>>
>> Heiner
>>
>>
>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
>> index 8d7dd4c5f..26be73000 100644
>> --- a/drivers/net/ethernet/realtek/r8169_main.c
>> +++ b/drivers/net/ethernet/realtek/r8169_main.c
>> @@ -1934,6 +1934,8 @@ static int rtl_get_eee_supp(struct rtl8169_private *tp)
>> struct phy_device *phydev = tp->phydev;
>> int ret;
>>
>> + WARN_ON(phy_read(phydev, 0x1f));
>> +
>> switch (tp->mac_version) {
>> case RTL_GIGA_MAC_VER_34:
>> case RTL_GIGA_MAC_VER_35:
>> @@ -1957,6 +1959,8 @@ static int rtl_get_eee_lpadv(struct rtl8169_private *tp)
>> struct phy_device *phydev = tp->phydev;
>> int ret;
>>
>> + WARN_ON(phy_read(phydev, 0x1f));
>> +
>> switch (tp->mac_version) {
>> case RTL_GIGA_MAC_VER_34:
>> case RTL_GIGA_MAC_VER_35:
>> @@ -1980,6 +1984,8 @@ static int rtl_get_eee_adv(struct rtl8169_private *tp)
>> struct phy_device *phydev = tp->phydev;
>> int ret;
>>
>> + WARN_ON(phy_read(phydev, 0x1f));
>> +
>> switch (tp->mac_version) {
>> case RTL_GIGA_MAC_VER_34:
>> case RTL_GIGA_MAC_VER_35:
>> @@ -2003,6 +2009,8 @@ static int rtl_set_eee_adv(struct rtl8169_private *tp, int val)
>> struct phy_device *phydev = tp->phydev;
>> int ret = 0;
>>
>> + WARN_ON(phy_read(phydev, 0x1f));
>> +
>> switch (tp->mac_version) {
>> case RTL_GIGA_MAC_VER_34:
>> case RTL_GIGA_MAC_VER_35:
>> diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
>> index 16667fbac..1aa1142b8 100644
>> --- a/drivers/net/phy/phy-core.c
>> +++ b/drivers/net/phy/phy-core.c
>> @@ -463,12 +463,10 @@ int __phy_modify_changed(struct phy_device *phydev, u32 regnum, u16 mask,
>> return ret;
>>
>> new = (ret & ~mask) | set;
>> - if (new == ret)
>> - return 0;
>>
>> - ret = __phy_write(phydev, regnum, new);
>> + __phy_write(phydev, regnum, new);
>>
>> - return ret < 0 ? ret : 1;
>> + return new != ret;
>> }
>> EXPORT_SYMBOL_GPL(__phy_modify_changed);
>>
>>
>
> Took your patch on top of next-20190719.
> See attached dmesg.
> It didn't work. Same thing, lots of ping drops, no usable network.
>
> like that:
> 44 packets transmitted, 2 received, 95% packet loss, time 44005ms
>
I remember that I once had problems with this chip version and 100Mbps.
Could you check whether you face the same issues with 1Gbps?
>
> Maybe important:
> I build a kernel with no modules.
>
> I have to power off when I booted a kernel which doesn't work, a (soft) reboot into a older kernel (e.g. 4.9.y) doesn't
> fix the problem. Powering off and on does.
>
>
> greetings,
>
> Thomas
^ permalink raw reply
* [PATCH] netfilter: ebtables: compat: fix a memory leak bug
From: Wenwen Wang @ 2019-07-20 12:22 UTC (permalink / raw)
To: Wenwen Wang
Cc: Pablo Neira Ayuso, Jozsef Kadlecsik, Florian Westphal,
Roopa Prabhu, Nikolay Aleksandrov, David S. Miller,
open list:NETFILTER, open list:NETFILTER,
moderated list:ETHERNET BRIDGE, open list:ETHERNET BRIDGE,
open list
From: Wenwen Wang <wenwen@cs.uga.edu>
In compat_do_replace(), a temporary buffer is allocated through vmalloc()
to hold entries copied from the user space. The buffer address is firstly
saved to 'newinfo->entries', and later on assigned to 'entries_tmp'. Then
the entries in this temporary buffer is copied to the internal kernel
structure through compat_copy_entries(). If this copy process fails,
compat_do_replace() should be terminated. However, the allocated temporary
buffer is not freed on this path, leading to a memory leak.
To fix the bug, free the buffer before returning from compat_do_replace().
Signed-off-by: Wenwen Wang <wenwen@cs.uga.edu>
---
net/bridge/netfilter/ebtables.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/bridge/netfilter/ebtables.c b/net/bridge/netfilter/ebtables.c
index 963dfdc..fd84b48e 100644
--- a/net/bridge/netfilter/ebtables.c
+++ b/net/bridge/netfilter/ebtables.c
@@ -2261,8 +2261,10 @@ static int compat_do_replace(struct net *net, void __user *user,
state.buf_kern_len = size64;
ret = compat_copy_entries(entries_tmp, tmp.entries_size, &state);
- if (WARN_ON(ret < 0))
+ if (WARN_ON(ret < 0)) {
+ vfree(entries_tmp);
goto out_unlock;
+ }
vfree(entries_tmp);
tmp.entries_size = size64;
--
2.7.4
^ permalink raw reply related
* Re: network problems with r8169
From: Thomas Voegtle @ 2019-07-20 14:22 UTC (permalink / raw)
To: Heiner Kallweit; +Cc: linux-kernel, netdev@vger.kernel.org
In-Reply-To: <9cab7996-d801-0ae5-9e82-6d24eeb8d7c7@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 6602 bytes --]
On Sat, 20 Jul 2019, Heiner Kallweit wrote:
> On 19.07.2019 23:12, Thomas Voegtle wrote:
>> On Fri, 19 Jul 2019, Heiner Kallweit wrote:
>>
>>> On 18.07.2019 20:50, Thomas Voegtle wrote:
>>>>
>>>> Hello,
>>>>
>>>> I'm having network problems with the commits on r8169 since v5.2. There are ping packet loss, sometimes 100%, sometimes 50%. In the end network is unusable.
>>>>
>>>> v5.2 is fine, I bisected it down to:
>>>>
>>>> a2928d28643e3c064ff41397281d20c445525032 is the first bad commit
>>>> commit a2928d28643e3c064ff41397281d20c445525032
>>>> Author: Heiner Kallweit <hkallweit1@gmail.com>
>>>> Date: Sun Jun 2 10:53:49 2019 +0200
>>>>
>>>> r8169: use paged versions of phylib MDIO access functions
>>>>
>>>> Use paged versions of phylib MDIO access functions to simplify
>>>> the code.
>>>>
>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>> Signed-off-by: David S. Miller <davem@davemloft.net>
>>>>
>>>>
>>>> Reverting that commit on top of v5.2-11564-g22051d9c4a57 fixes the problem
>>>> for me (had to adjust the renaming to r8169_main.c).
>>>>
>>>> I have a:
>>>> 04:00.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd.
>>>> RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller [10ec:8168] (rev
>>>> 0c)
>>>> Subsystem: Biostar Microtech Int'l Corp Device [1565:2400]
>>>> Kernel driver in use: r8169
>>>>
>>>> on a BIOSTAR H81MG motherboard.
>>>>
>>> Interesting. I have the same chip version (RTL8168g) and can't reproduce
>>> the issue. Can you provide a full dmesg output and test the patch below
>>> on top of linux-next? I'd be interested in the WARN_ON stack traces
>>> (if any) and would like to know whether the experimental change to
>>> __phy_modify_changed helps.
>>>
>>>>
>>>> greetings,
>>>>
>>>> Thomas
>>>>
>>>>
>>> Heiner
>>>
>>>
>>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
>>> index 8d7dd4c5f..26be73000 100644
>>> --- a/drivers/net/ethernet/realtek/r8169_main.c
>>> +++ b/drivers/net/ethernet/realtek/r8169_main.c
>>> @@ -1934,6 +1934,8 @@ static int rtl_get_eee_supp(struct rtl8169_private *tp)
>>> struct phy_device *phydev = tp->phydev;
>>> int ret;
>>>
>>> + WARN_ON(phy_read(phydev, 0x1f));
>>> +
>>> switch (tp->mac_version) {
>>> case RTL_GIGA_MAC_VER_34:
>>> case RTL_GIGA_MAC_VER_35:
>>> @@ -1957,6 +1959,8 @@ static int rtl_get_eee_lpadv(struct rtl8169_private *tp)
>>> struct phy_device *phydev = tp->phydev;
>>> int ret;
>>>
>>> + WARN_ON(phy_read(phydev, 0x1f));
>>> +
>>> switch (tp->mac_version) {
>>> case RTL_GIGA_MAC_VER_34:
>>> case RTL_GIGA_MAC_VER_35:
>>> @@ -1980,6 +1984,8 @@ static int rtl_get_eee_adv(struct rtl8169_private *tp)
>>> struct phy_device *phydev = tp->phydev;
>>> int ret;
>>>
>>> + WARN_ON(phy_read(phydev, 0x1f));
>>> +
>>> switch (tp->mac_version) {
>>> case RTL_GIGA_MAC_VER_34:
>>> case RTL_GIGA_MAC_VER_35:
>>> @@ -2003,6 +2009,8 @@ static int rtl_set_eee_adv(struct rtl8169_private *tp, int val)
>>> struct phy_device *phydev = tp->phydev;
>>> int ret = 0;
>>>
>>> + WARN_ON(phy_read(phydev, 0x1f));
>>> +
>>> switch (tp->mac_version) {
>>> case RTL_GIGA_MAC_VER_34:
>>> case RTL_GIGA_MAC_VER_35:
>>> diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
>>> index 16667fbac..1aa1142b8 100644
>>> --- a/drivers/net/phy/phy-core.c
>>> +++ b/drivers/net/phy/phy-core.c
>>> @@ -463,12 +463,10 @@ int __phy_modify_changed(struct phy_device *phydev, u32 regnum, u16 mask,
>>> return ret;
>>>
>>> new = (ret & ~mask) | set;
>>> - if (new == ret)
>>> - return 0;
>>>
>>> - ret = __phy_write(phydev, regnum, new);
>>> + __phy_write(phydev, regnum, new);
>>>
>>> - return ret < 0 ? ret : 1;
>>> + return new != ret;
>>> }
>>> EXPORT_SYMBOL_GPL(__phy_modify_changed);
>>>
>>>
>>
>> Took your patch on top of next-20190719.
>> See attached dmesg.
>> It didn't work. Same thing, lots of ping drops, no usable network.
>>
>> like that:
>> 44 packets transmitted, 2 received, 95% packet loss, time 44005ms
>>
>>
>> Maybe important:
>> I build a kernel with no modules.
>>
>> I have to power off when I booted a kernel which doesn't work, a (soft) reboot into a older kernel (e.g. 4.9.y) doesn't
>> fix the problem. Powering off and on does.
>>
>
> Then, what you could do is reversing the hunks of the patch step by step.
> Or make them separate patches and bisect.
> Relevant are the hunks from point 1 and 2.
>
> 1. first 5 hunks (I don't think you have to reverse them individually)
> EEE-related
>
> 2. rtl8168g_disable_aldps, rtl8168g_phy_adjust_10m_aldps, rtl8168g_1_hw_phy_config
> all of these hunks are in the path for RTL8168g
>
> 3. rtl8168h_1_hw_phy_config, rtl8168h_2_hw_phy_config, rtl8168ep_1_hw_phy_config,
> rtl8168ep_2_hw_phy_config
> not in the path for RTL8168g
>
this is the minimal revert:
diff --git a/drivers/net/ethernet/realtek/r8169_main.c
b/drivers/net/ethernet/realtek/r8169_main.c
index efef5453b94f..267995a614b5 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -3249,12 +3249,14 @@ static void rtl8168g_1_hw_phy_config(struct
rtl8169_private *tp)
else
phy_modify_paged(tp->phydev, 0x0bcc, 0x12, 0, BIT(15));
- ret = phy_read_paged(tp->phydev, 0x0a46, 0x13);
- if (ret & BIT(8))
- phy_modify_paged(tp->phydev, 0x0c41, 0x12, 0, BIT(1));
- else
- phy_modify_paged(tp->phydev, 0x0c41, 0x12, BIT(1), 0);
-
+ rtl_writephy(tp, 0x1f, 0x0a46);
+ if (rtl_readphy(tp, 0x13) & 0x0100) {
+ rtl_writephy(tp, 0x1f, 0x0c41);
+ rtl_w0w1_phy(tp, 0x15, 0x0002, 0x0000);
+ } else {
+ rtl_writephy(tp, 0x1f, 0x0c41);
+ rtl_w0w1_phy(tp, 0x15, 0x0000, 0x0002);
+ }
/* Enable PHY auto speed down */
phy_modify_paged(tp->phydev, 0x0a44, 0x11, 0, BIT(3) | BIT(2));
Could it be, that there is just a typo?
if (ret & BIT(8))
- phy_modify_paged(tp->phydev, 0x0c41, 0x12, 0, BIT(1));
+ phy_modify_paged(tp->phydev, 0x0c41, 0x15, 0, BIT(1));
else
- phy_modify_paged(tp->phydev, 0x0c41, 0x12, BIT(1), 0);
+ phy_modify_paged(tp->phydev, 0x0c41, 0x15, BIT(1), 0);
greetings,
Thomas
^ permalink raw reply related
* Re: [PATCH net-next v2 8/8] net: mscc: PTP Hardware Clock (PHC) support
From: kbuild test robot @ 2019-07-20 15:23 UTC (permalink / raw)
To: Antoine Tenart
Cc: kbuild-all, davem, richardcochran, alexandre.belloni,
UNGLinuxDriver, ralf, paul.burton, jhogan, Antoine Tenart, netdev,
linux-mips, thomas.petazzoni, allan.nielsen
In-Reply-To: <20190705195213.22041-9-antoine.tenart@bootlin.com>
[-- Attachment #1: Type: text/plain, Size: 1178 bytes --]
Hi Antoine,
I love your patch! Yet something to improve:
[auto build test ERROR on net-next/master]
url: https://github.com/0day-ci/linux/commits/Antoine-Tenart/net-mscc-PTP-Hardware-Clock-PHC-support/20190707-075931
config: ia64-allmodconfig (attached as .config)
compiler: ia64-linux-gcc (GCC) 7.4.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=7.4.0 make.cross ARCH=ia64
If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
All errors (new ones prefixed by >>):
ERROR: "ia64_delay_loop" [drivers/spi/spi-thunderx.ko] undefined!
ERROR: "ia64_delay_loop" [drivers/net/phy/mdio-cavium.ko] undefined!
>> ERROR: "ocelot_ptp_gettime64" [drivers/net/ethernet/mscc/ocelot_board.ko] undefined!
>> ERROR: "ocelot_get_hwtimestamp" [drivers/net/ethernet/mscc/ocelot_board.ko] undefined!
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 54158 bytes --]
^ permalink raw reply
* Re: network problems with r8169
From: Heiner Kallweit @ 2019-07-20 16:44 UTC (permalink / raw)
To: Thomas Voegtle; +Cc: linux-kernel, netdev@vger.kernel.org
In-Reply-To: <alpine.LSU.2.21.1907201620070.2099@er-systems.de>
On 20.07.2019 16:22, Thomas Voegtle wrote:
> On Sat, 20 Jul 2019, Heiner Kallweit wrote:
>
>> On 19.07.2019 23:12, Thomas Voegtle wrote:
>>> On Fri, 19 Jul 2019, Heiner Kallweit wrote:
>>>
>>>> On 18.07.2019 20:50, Thomas Voegtle wrote:
>>>>>
>>>>> Hello,
>>>>>
>>>>> I'm having network problems with the commits on r8169 since v5.2. There are ping packet loss, sometimes 100%, sometimes 50%. In the end network is unusable.
>>>>>
>>>>> v5.2 is fine, I bisected it down to:
>>>>>
>>>>> a2928d28643e3c064ff41397281d20c445525032 is the first bad commit
>>>>> commit a2928d28643e3c064ff41397281d20c445525032
>>>>> Author: Heiner Kallweit <hkallweit1@gmail.com>
>>>>> Date: Sun Jun 2 10:53:49 2019 +0200
>>>>>
>>>>> r8169: use paged versions of phylib MDIO access functions
>>>>>
>>>>> Use paged versions of phylib MDIO access functions to simplify
>>>>> the code.
>>>>>
>>>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>>>> Signed-off-by: David S. Miller <davem@davemloft.net>
>>>>>
>>>>>
>>>>> Reverting that commit on top of v5.2-11564-g22051d9c4a57 fixes the problem
>>>>> for me (had to adjust the renaming to r8169_main.c).
>>>>>
>>>>> I have a:
>>>>> 04:00.0 Ethernet controller [0200]: Realtek Semiconductor Co., Ltd.
>>>>> RTL8111/8168/8411 PCI Express Gigabit Ethernet Controller [10ec:8168] (rev
>>>>> 0c)
>>>>> Subsystem: Biostar Microtech Int'l Corp Device [1565:2400]
>>>>> Kernel driver in use: r8169
>>>>>
>>>>> on a BIOSTAR H81MG motherboard.
>>>>>
>>>> Interesting. I have the same chip version (RTL8168g) and can't reproduce
>>>> the issue. Can you provide a full dmesg output and test the patch below
>>>> on top of linux-next? I'd be interested in the WARN_ON stack traces
>>>> (if any) and would like to know whether the experimental change to
>>>> __phy_modify_changed helps.
>>>>
>>>>>
>>>>> greetings,
>>>>>
>>>>> Thomas
>>>>>
>>>>>
>>>> Heiner
>>>>
>>>>
>>>> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
>>>> index 8d7dd4c5f..26be73000 100644
>>>> --- a/drivers/net/ethernet/realtek/r8169_main.c
>>>> +++ b/drivers/net/ethernet/realtek/r8169_main.c
>>>> @@ -1934,6 +1934,8 @@ static int rtl_get_eee_supp(struct rtl8169_private *tp)
>>>> struct phy_device *phydev = tp->phydev;
>>>> int ret;
>>>>
>>>> + WARN_ON(phy_read(phydev, 0x1f));
>>>> +
>>>> switch (tp->mac_version) {
>>>> case RTL_GIGA_MAC_VER_34:
>>>> case RTL_GIGA_MAC_VER_35:
>>>> @@ -1957,6 +1959,8 @@ static int rtl_get_eee_lpadv(struct rtl8169_private *tp)
>>>> struct phy_device *phydev = tp->phydev;
>>>> int ret;
>>>>
>>>> + WARN_ON(phy_read(phydev, 0x1f));
>>>> +
>>>> switch (tp->mac_version) {
>>>> case RTL_GIGA_MAC_VER_34:
>>>> case RTL_GIGA_MAC_VER_35:
>>>> @@ -1980,6 +1984,8 @@ static int rtl_get_eee_adv(struct rtl8169_private *tp)
>>>> struct phy_device *phydev = tp->phydev;
>>>> int ret;
>>>>
>>>> + WARN_ON(phy_read(phydev, 0x1f));
>>>> +
>>>> switch (tp->mac_version) {
>>>> case RTL_GIGA_MAC_VER_34:
>>>> case RTL_GIGA_MAC_VER_35:
>>>> @@ -2003,6 +2009,8 @@ static int rtl_set_eee_adv(struct rtl8169_private *tp, int val)
>>>> struct phy_device *phydev = tp->phydev;
>>>> int ret = 0;
>>>>
>>>> + WARN_ON(phy_read(phydev, 0x1f));
>>>> +
>>>> switch (tp->mac_version) {
>>>> case RTL_GIGA_MAC_VER_34:
>>>> case RTL_GIGA_MAC_VER_35:
>>>> diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
>>>> index 16667fbac..1aa1142b8 100644
>>>> --- a/drivers/net/phy/phy-core.c
>>>> +++ b/drivers/net/phy/phy-core.c
>>>> @@ -463,12 +463,10 @@ int __phy_modify_changed(struct phy_device *phydev, u32 regnum, u16 mask,
>>>> return ret;
>>>>
>>>> new = (ret & ~mask) | set;
>>>> - if (new == ret)
>>>> - return 0;
>>>>
>>>> - ret = __phy_write(phydev, regnum, new);
>>>> + __phy_write(phydev, regnum, new);
>>>>
>>>> - return ret < 0 ? ret : 1;
>>>> + return new != ret;
>>>> }
>>>> EXPORT_SYMBOL_GPL(__phy_modify_changed);
>>>>
>>>>
>>>
>>> Took your patch on top of next-20190719.
>>> See attached dmesg.
>>> It didn't work. Same thing, lots of ping drops, no usable network.
>>>
>>> like that:
>>> 44 packets transmitted, 2 received, 95% packet loss, time 44005ms
>>>
>>>
>>> Maybe important:
>>> I build a kernel with no modules.
>>>
>>> I have to power off when I booted a kernel which doesn't work, a (soft) reboot into a older kernel (e.g. 4.9.y) doesn't
>>> fix the problem. Powering off and on does.
>>>
>>
>> Then, what you could do is reversing the hunks of the patch step by step.
>> Or make them separate patches and bisect.
>> Relevant are the hunks from point 1 and 2.
>>
>> 1. first 5 hunks (I don't think you have to reverse them individually)
>> EEE-related
>>
>> 2. rtl8168g_disable_aldps, rtl8168g_phy_adjust_10m_aldps, rtl8168g_1_hw_phy_config
>> all of these hunks are in the path for RTL8168g
>>
>> 3. rtl8168h_1_hw_phy_config, rtl8168h_2_hw_phy_config, rtl8168ep_1_hw_phy_config,
>> rtl8168ep_2_hw_phy_config
>> not in the path for RTL8168g
>>
>
> this is the minimal revert:
>
> diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
> index efef5453b94f..267995a614b5 100644
> --- a/drivers/net/ethernet/realtek/r8169_main.c
> +++ b/drivers/net/ethernet/realtek/r8169_main.c
> @@ -3249,12 +3249,14 @@ static void rtl8168g_1_hw_phy_config(struct rtl8169_private *tp)
> else
> phy_modify_paged(tp->phydev, 0x0bcc, 0x12, 0, BIT(15));
>
> - ret = phy_read_paged(tp->phydev, 0x0a46, 0x13);
> - if (ret & BIT(8))
> - phy_modify_paged(tp->phydev, 0x0c41, 0x12, 0, BIT(1));
> - else
> - phy_modify_paged(tp->phydev, 0x0c41, 0x12, BIT(1), 0);
> -
> + rtl_writephy(tp, 0x1f, 0x0a46);
> + if (rtl_readphy(tp, 0x13) & 0x0100) {
> + rtl_writephy(tp, 0x1f, 0x0c41);
> + rtl_w0w1_phy(tp, 0x15, 0x0002, 0x0000);
> + } else {
> + rtl_writephy(tp, 0x1f, 0x0c41);
> + rtl_w0w1_phy(tp, 0x15, 0x0000, 0x0002);
> + }
> /* Enable PHY auto speed down */
> phy_modify_paged(tp->phydev, 0x0a44, 0x11, 0, BIT(3) | BIT(2));
>
>
>
> Could it be, that there is just a typo?
>
I looked a hundred times over this piece of code ..
Yes, it's simply a typo. I'll submit a patch for it.
Thanks a lot for your testing efforts!
> if (ret & BIT(8))
> - phy_modify_paged(tp->phydev, 0x0c41, 0x12, 0, BIT(1));
> + phy_modify_paged(tp->phydev, 0x0c41, 0x15, 0, BIT(1));
> else
> - phy_modify_paged(tp->phydev, 0x0c41, 0x12, BIT(1), 0);
> + phy_modify_paged(tp->phydev, 0x0c41, 0x15, BIT(1), 0);
>
>
>
>
> greetings,
>
> Thomas
Heiner
^ permalink raw reply
* linux-headers-5.2 and proper use of SIOCGSTAMP
From: Sergei Trofimovich @ 2019-07-20 16:48 UTC (permalink / raw)
To: netdev, linux-kernel, libc-alpha
Cc: Arnd Bergmann, David S. Miller, mtk.manpages, linux-man
Commit https://github.com/torvalds/linux/commit/0768e17073dc527ccd18ed5f96ce85f9985e9115
("net: socket: implement 64-bit timestamps") caused a bit of userspace breakage
for existing programs:
- firefox: https://bugs.gentoo.org/689808
- qemu: https://lists.sr.ht/~philmd/qemu/%3C20190604071915.288045-1-borntraeger%40de.ibm.com%3E
- linux-atm: https://gitweb.gentoo.org/repo/gentoo.git/tree/net-dialup/linux-atm/files/linux-atm-2.5.2-linux-5.2-SIOCGSTAMP.patch?id=408621819a85bf67a73efd33a06ea371c20ea5a2
I have a question: how a well-behaved app should include 'SIOCGSTAMP'
definition to keep being buildable against old and new linux-headers?
'man 7 socket' explicitly mentions SIOCGSTAMP and mentions only
#include <sys/socket.h>
as needed header.
Should #include <linux/sockios.h> always be included by user app?
Or should glibc tweak it's definition of '#include <sys/socket.h>'
to make it available on both old and new version of linux headers?
CCing both kernel and glibc folk as I don't understand on which
side issue should be fixed.
Thanks!
--
Sergei
^ permalink raw reply
* [PATCH net] r8169: fix RTL8168g PHY init
From: Heiner Kallweit @ 2019-07-20 17:01 UTC (permalink / raw)
To: David Miller, Realtek linux nic maintainers
Cc: netdev@vger.kernel.org, Thomas Voegtle
From: Thomas Voegtle <tv@lio96.de>
This fixes a copy&paste error in the original patch. Setting the wrong
register resulted in massive packet loss on some systems.
Fixes: a2928d28643e ("r8169: use paged versions of phylib MDIO access functions")
Tested-by: Thomas Voegtle <tv@lio96.de>
Signed-off-by: Thomas Voegtle <tv@lio96.de>
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/ethernet/realtek/r8169_main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 0637c6752..6272115b2 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -3251,9 +3251,9 @@ static void rtl8168g_1_hw_phy_config(struct rtl8169_private *tp)
ret = phy_read_paged(tp->phydev, 0x0a46, 0x13);
if (ret & BIT(8))
- phy_modify_paged(tp->phydev, 0x0c41, 0x12, 0, BIT(1));
+ phy_modify_paged(tp->phydev, 0x0c41, 0x15, 0, BIT(1));
else
- phy_modify_paged(tp->phydev, 0x0c41, 0x12, BIT(1), 0);
+ phy_modify_paged(tp->phydev, 0x0c41, 0x15, BIT(1), 0);
/* Enable PHY auto speed down */
phy_modify_paged(tp->phydev, 0x0a44, 0x11, 0, BIT(3) | BIT(2));
--
2.22.0
^ permalink raw reply related
* [PATCH] rat_cs: Remove duplicate code
From: Hariprasad Kelam @ 2019-07-20 17:46 UTC (permalink / raw)
To: Kalle Valo, David S. Miller, linux-wireless, netdev, linux-kernel
Code is same if translate is true/false in case invalid packet is
received.So remove else part.
Issue identified with coccicheck
Signed-off-by: Hariprasad Kelam <hariprasad.kelam@gmail.com>
---
drivers/net/wireless/ray_cs.c | 29 ++++++++---------------------
1 file changed, 8 insertions(+), 21 deletions(-)
diff --git a/drivers/net/wireless/ray_cs.c b/drivers/net/wireless/ray_cs.c
index cf37268..a51bbe7 100644
--- a/drivers/net/wireless/ray_cs.c
+++ b/drivers/net/wireless/ray_cs.c
@@ -2108,29 +2108,16 @@ static void rx_data(struct net_device *dev, struct rcs __iomem *prcs,
#endif
if (!sniffer) {
- if (translate) {
/* TBD length needs fixing for translated header */
- if (rx_len < (ETH_HLEN + RX_MAC_HEADER_LENGTH) ||
- rx_len >
- (dev->mtu + RX_MAC_HEADER_LENGTH + ETH_HLEN +
- FCS_LEN)) {
- pr_debug(
- "ray_cs invalid packet length %d received\n",
- rx_len);
- return;
- }
- } else { /* encapsulated ethernet */
-
- if (rx_len < (ETH_HLEN + RX_MAC_HEADER_LENGTH) ||
- rx_len >
- (dev->mtu + RX_MAC_HEADER_LENGTH + ETH_HLEN +
- FCS_LEN)) {
- pr_debug(
- "ray_cs invalid packet length %d received\n",
- rx_len);
- return;
+ if (rx_len < (ETH_HLEN + RX_MAC_HEADER_LENGTH) ||
+ rx_len >
+ (dev->mtu + RX_MAC_HEADER_LENGTH + ETH_HLEN +
+ FCS_LEN)) {
+ pr_debug(
+ "ray_cs invalid packet length %d received\n",
+ rx_len);
+ return;
}
- }
}
pr_debug("ray_cs rx_data packet\n");
/* If fragmented packet, verify sizes of fragments add up */
--
2.7.4
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox