* [PATCH net-next 5/5] net: stmmac: Limit max speeds of XGMAC if asked to
From: Jose Abreu @ 2019-09-06 7:41 UTC (permalink / raw)
To: netdev
Cc: Joao Pinto, Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue,
David S. Miller, Maxime Coquelin, linux-arm-kernel, linux-kernel
In-Reply-To: <cover.1567755423.git.joabreu@synopsys.com>
We may have some SoCs that can't achieve XGMAC max speed. Limit it if
asked to.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
---
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: netdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 25 +++++++++++++++--------
1 file changed, 16 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index c3baca9f587b..686b82068142 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -831,15 +831,22 @@ static void stmmac_validate(struct phylink_config *config,
phylink_set(mask, 1000baseT_Full);
phylink_set(mask, 1000baseX_Full);
} else if (priv->plat->has_xgmac) {
- phylink_set(mac_supported, 2500baseT_Full);
- phylink_set(mac_supported, 5000baseT_Full);
- phylink_set(mac_supported, 10000baseSR_Full);
- phylink_set(mac_supported, 10000baseLR_Full);
- phylink_set(mac_supported, 10000baseER_Full);
- phylink_set(mac_supported, 10000baseLRM_Full);
- phylink_set(mac_supported, 10000baseT_Full);
- phylink_set(mac_supported, 10000baseKX4_Full);
- phylink_set(mac_supported, 10000baseKR_Full);
+ if (!max_speed || (max_speed >= 2500)) {
+ phylink_set(mac_supported, 2500baseT_Full);
+ phylink_set(mac_supported, 2500baseX_Full);
+ }
+ if (!max_speed || (max_speed >= 5000)) {
+ phylink_set(mac_supported, 5000baseT_Full);
+ }
+ if (!max_speed || (max_speed >= 10000)) {
+ phylink_set(mac_supported, 10000baseSR_Full);
+ phylink_set(mac_supported, 10000baseLR_Full);
+ phylink_set(mac_supported, 10000baseER_Full);
+ phylink_set(mac_supported, 10000baseLRM_Full);
+ phylink_set(mac_supported, 10000baseT_Full);
+ phylink_set(mac_supported, 10000baseKX4_Full);
+ phylink_set(mac_supported, 10000baseKR_Full);
+ }
}
/* Half-Duplex can only work with single queue */
--
2.7.4
^ permalink raw reply related
* [PATCH net-next 3/5] net: stmmac: dwmac4: Enable RX Jumbo frame support
From: Jose Abreu @ 2019-09-06 7:41 UTC (permalink / raw)
To: netdev
Cc: Joao Pinto, Jose Abreu, Giuseppe Cavallaro, Alexandre Torgue,
David S. Miller, Maxime Coquelin, linux-arm-kernel, linux-kernel
In-Reply-To: <cover.1567755423.git.joabreu@synopsys.com>
We are already doing it by default in the TX path so we can also enable
Jumbo Frame support in the RX path independently of MTU value.
Signed-off-by: Jose Abreu <joabreu@synopsys.com>
---
Cc: Giuseppe Cavallaro <peppe.cavallaro@st.com>
Cc: Alexandre Torgue <alexandre.torgue@st.com>
Cc: Jose Abreu <joabreu@synopsys.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Maxime Coquelin <mcoquelin.stm32@gmail.com>
Cc: netdev@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
drivers/net/ethernet/stmicro/stmmac/dwmac4.h | 3 ++-
drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c | 6 ------
2 files changed, 2 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
index 2ed11a581d80..03301ffc0391 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4.h
@@ -352,7 +352,8 @@ enum power_event {
/* Default operating mode of the MAC */
#define GMAC_CORE_INIT (GMAC_CONFIG_JD | GMAC_CONFIG_PS | \
- GMAC_CONFIG_BE | GMAC_CONFIG_DCRS)
+ GMAC_CONFIG_BE | GMAC_CONFIG_DCRS | \
+ GMAC_CONFIG_JE)
/* To dump the core regs excluding the Address Registers */
#define GMAC_REG_NUM 132
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
index fc9954e4a772..596311a80d1c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_core.c
@@ -25,15 +25,9 @@ static void dwmac4_core_init(struct mac_device_info *hw,
{
void __iomem *ioaddr = hw->pcsr;
u32 value = readl(ioaddr + GMAC_CONFIG);
- int mtu = dev->mtu;
value |= GMAC_CORE_INIT;
- if (mtu > 1500)
- value |= GMAC_CONFIG_2K;
- if (mtu > 2000)
- value |= GMAC_CONFIG_JE;
-
if (hw->ps) {
value |= GMAC_CONFIG_TE;
--
2.7.4
^ permalink raw reply related
* Re: linux-next: build failure after merge of the net-next tree
From: Andrii Nakryiko @ 2019-09-06 9:02 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Stephen Rothwell, David Miller, Networking,
Linux Next Mailing List, Linux Kernel Mailing List,
Andrii Nakryiko, Daniel Borkmann, Alexei Starovoitov
In-Reply-To: <CAK7LNATkk3VfzgynBEyOinKo3yBEDgNHLgO3bftLAPbDVVWx=A@mail.gmail.com>
On Thu, Sep 5, 2019 at 7:53 PM Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
>
> On Fri, Sep 6, 2019 at 4:26 AM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Tue, Sep 3, 2019 at 11:20 PM Masahiro Yamada
> > <yamada.masahiro@socionext.com> wrote:
> > >
> > > On Wed, Sep 4, 2019 at 3:00 PM Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> > > >
> > > > Hi all,
> > > >
> > > > After merging the net-next tree, today's linux-next build (arm
> > > > multi_v7_defconfig) failed like this:
> > > >
> > > > scripts/link-vmlinux.sh: 74: Bad substitution
> > > >
> > > > Caused by commit
> > > >
> > > > 341dfcf8d78e ("btf: expose BTF info through sysfs")
> > > >
> > > > interacting with commit
> > > >
> > > > 1267f9d3047d ("kbuild: add $(BASH) to run scripts with bash-extension")
> > > >
> > > > from the kbuild tree.
> > >
> > >
> > > I knew that they were using bash-extension
> > > in the #!/bin/sh script. :-D
> > >
> > > In fact, I wrote my patch in order to break their code
> > > and make btf people realize that they were doing wrong.
> >
> > Was there a specific reason to wait until this would break during
> > Stephen's merge, instead of giving me a heads up (or just replying on
> > original patch) and letting me fix it and save everyone's time and
> > efforts?
> >
> > Either way, I've fixed the issue in
> > https://patchwork.ozlabs.org/patch/1158620/ and will pay way more
> > attention to BASH-specific features going forward (I found it pretty
> > hard to verify stuff like this, unfortunately). But again, code review
> > process is the best place to catch this and I really hope in the
> > future we can keep this process productive. Thanks!
>
> I could have pointed it out if I had noticed
> it in the review process.
>
> I actually noticed your patch by Stephen's
> former email. (i.e. when it appeared in linux-next)
>
> (I try my best to check kbuild ML, and also search for
> my name in LKML in case I am explicitly addressed,
> but a large number of emails fall off my filter)
>
> It was somewhat too late when I noticed it.
> Of course, I still could email you afterward, or even send a patch to btf ML,
> but I did not fix a particular instance of breakage
> because there are already the same type of breakages in code base.
>
> Then, I applied the all-or-nothing checker because I thought it was
> the only way to address the root cause of the problems.
>
> I admit I could have done the process better.
> Sorry if I made people uncomfortable and waste time.
No worries. Thanks for candid answer. I just wanted to make sure there
are no hard feelings and I can engage your expertise effectively in
the future for kbuild stuff to ensure issues like this don't slip
through, if we ever have to do anything like this for BPF-related
things again. I'll keep CC'ing you and will add kbuild ML as well.
Thanks!
>
> Thanks.
>
>
>
>
> > >
> > >
> > >
> > > > The change in the net-next tree turned link-vmlinux.sh into a bash script
> > > > (I think).
> > > >
> > > > I have applied the following patch for today:
> > >
> > >
> > > But, this is a temporary fix only for linux-next.
> > >
> > > scripts/link-vmlinux.sh does not need to use the
> > > bash-extension ${@:2} in the first place.
> > >
> > > I hope btf people will write the correct code.
> >
> > I replaced ${@:2} with shift and ${@}, I hope that's a correct fix,
> > but if you think it's not, please reply on the patch and let me know.
> >
> >
> > >
> > > Thanks.
> > >
> > >
> > >
> > >
> > > > From: Stephen Rothwell <sfr@canb.auug.org.au>
> > > > Date: Wed, 4 Sep 2019 15:43:41 +1000
> > > > Subject: [PATCH] link-vmlinux.sh is now a bash script
> > > >
> > > > Signed-off-by: Stephen Rothwell <sfr@canb.auug.org.au>
> > > > ---
> > > > Makefile | 4 ++--
> > > > scripts/link-vmlinux.sh | 2 +-
> > > > 2 files changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/Makefile b/Makefile
> > > > index ac97fb282d99..523d12c5cebe 100644
> > > > --- a/Makefile
> > > > +++ b/Makefile
> > > > @@ -1087,7 +1087,7 @@ ARCH_POSTLINK := $(wildcard $(srctree)/arch/$(SRCARCH)/Makefile.postlink)
> > > >
> > > > # Final link of vmlinux with optional arch pass after final link
> > > > cmd_link-vmlinux = \
> > > > - $(CONFIG_SHELL) $< $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_vmlinux) ; \
> > > > + $(BASH) $< $(LD) $(KBUILD_LDFLAGS) $(LDFLAGS_vmlinux) ; \
> > > > $(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) $@, true)
> > > >
> > > > vmlinux: scripts/link-vmlinux.sh autoksyms_recursive $(vmlinux-deps) FORCE
> > > > @@ -1403,7 +1403,7 @@ clean: rm-files := $(CLEAN_FILES)
> > > > PHONY += archclean vmlinuxclean
> > > >
> > > > vmlinuxclean:
> > > > - $(Q)$(CONFIG_SHELL) $(srctree)/scripts/link-vmlinux.sh clean
> > > > + $(Q)$(BASH) $(srctree)/scripts/link-vmlinux.sh clean
> > > > $(Q)$(if $(ARCH_POSTLINK), $(MAKE) -f $(ARCH_POSTLINK) clean)
> > > >
> > > > clean: archclean vmlinuxclean
> > > > diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> > > > index f7edb75f9806..ea1f8673869d 100755
> > > > --- a/scripts/link-vmlinux.sh
> > > > +++ b/scripts/link-vmlinux.sh
> > > > @@ -1,4 +1,4 @@
> > > > -#!/bin/sh
> > > > +#!/bin/bash
> > > > # SPDX-License-Identifier: GPL-2.0
> > > > #
> > > > # link vmlinux
> > > > --
> > > > 2.23.0.rc1
> > > >
> > > > --
> > > > Cheers,
> > > > Stephen Rothwell
> > >
> > >
> > >
> > > --
> > > Best Regards
> > > Masahiro Yamada
>
>
>
> --
> Best Regards
> Masahiro Yamada
^ permalink raw reply
* Re: [PATCH 0/7] libbpf: Fix cast away const qualifiers in btf.h
From: Andrii Nakryiko @ 2019-09-06 9:09 UTC (permalink / raw)
To: Jiri Olsa, Alexei Starovoitov, Daniel Borkmann
Cc: netdev@vger.kernel.org, bpf@vger.kernel.org, Yonghong Song,
Martin Lau
In-Reply-To: <20190906073144.31068-1-jolsa@kernel.org>
On 9/6/19 8:31 AM, Jiri Olsa wrote:
> hi,
> when including btf.h in bpftrace, I'm getting -Wcast-qual warnings like:
>
> bpf/btf.h: In function ‘btf_var_secinfo* btf_var_secinfos(const btf_type*)’:
> bpf/btf.h:302:41: warning: cast from type ‘const btf_type*’ to type
> ‘btf_var_secinfo*’ casts away qualifiers [-Wcast-qual]
> 302 | return (struct btf_var_secinfo *)(t + 1);
> | ^
>
> I changed the btf.h header to comply with -Wcast-qual checks
> and used const cast away casting in libbpf objects, where it's
Hey Jiri,
We made all those helper funcs return non-const structs intentionally to
improve their usability and avoid all those casts that you added back.
Also, those helpers are now part of public API, so we can't just change
them to const, as it can break existing users easily.
If there is a need to run with -Wcast-qual, we should probably disable
those checks where appropriate in libbpf code.
So this will be a NACK from me, sorry.
> all related to deduplication code, so I believe loosing const
> is fine there.
>
> thanks,
> jirka
>
>
> ---
> Jiri Olsa (7):
> libbpf: Use const cast for btf_int_* functions
> libbpf: Return const btf_array from btf_array inline function
> libbpf: Return const btf_enum from btf_enum inline function
> libbpf: Return const btf_member from btf_members inline function
> libbpf: Return const btf_param from btf_params inline function
> libbpf: Return const btf_var from btf_var inline function
> libbpf: Return const struct btf_var_secinfo from btf_var_secinfos inline function
>
> tools/lib/bpf/btf.c | 21 +++++++++++----------
> tools/lib/bpf/btf.h | 30 +++++++++++++++---------------
> tools/lib/bpf/libbpf.c | 2 +-
> 3 files changed, 27 insertions(+), 26 deletions(-)
>
^ permalink raw reply
* Re: [PATCH bpf-next] kbuild: replace BASH-specific ${@:2} with shift and ${@}
From: Andrii Nakryiko @ 2019-09-06 9:11 UTC (permalink / raw)
To: Yonghong Song, bpf@vger.kernel.org, netdev@vger.kernel.org,
Alexei Starovoitov, daniel@iogearbox.net
Cc: andrii.nakryiko@gmail.com, Kernel Team, Stephen Rothwell,
Masahiro Yamada
In-Reply-To: <0a408cf0-1d18-6a39-84bd-31898de6c10d@fb.com>
On 9/6/19 12:59 AM, Yonghong Song wrote:
>
>
> On 9/5/19 10:59 AM, Andrii Nakryiko wrote:
>> ${@:2} is BASH-specific extension, which makes link-vmlinux.sh rely on
>> BASH. Use shift and ${@} instead to fix this issue.
>>
>> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
>> Fixes: 341dfcf8d78e ("btf: expose BTF info through sysfs")
>> Cc: Stephen Rothwell <sfr@canb.auug.org.au>
>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
>> Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>
> Tested with bash/sh/csh, all works.
Thanks for testing, Yonghong! In my system sh is an alias to bash, so it
still behaved like bash and didn't fail even with existing code. I
didn't have an opportunity to install csh at that time and try it, so
thanks a lot for confirming. I basically relied on documentation to
verify shift and $@ is not BASH'ism.
> Acked-by: Yonghong Song <yhs@fb.com>
>
>> ---
>> scripts/link-vmlinux.sh | 16 +++++++++++-----
>> 1 file changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
>> index 0d8f41db8cd6..8c59970a09dc 100755
>> --- a/scripts/link-vmlinux.sh
>> +++ b/scripts/link-vmlinux.sh
>> @@ -57,12 +57,16 @@ modpost_link()
>>
>> # Link of vmlinux
>> # ${1} - output file
>> -# ${@:2} - optional extra .o files
>> +# ${2}, ${3}, ... - optional extra .o files
>> vmlinux_link()
>> {
>> local lds="${objtree}/${KBUILD_LDS}"
>> + local output=${1}
>> local objects
>>
>> + # skip output file argument
>> + shift
>> +
>> if [ "${SRCARCH}" != "um" ]; then
>> objects="--whole-archive \
>> ${KBUILD_VMLINUX_OBJS} \
>> @@ -70,9 +74,10 @@ vmlinux_link()
>> --start-group \
>> ${KBUILD_VMLINUX_LIBS} \
>> --end-group \
>> - ${@:2}"
>> + ${@}"
>>
>> - ${LD} ${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux} -o ${1} \
>> + ${LD} ${KBUILD_LDFLAGS} ${LDFLAGS_vmlinux} \
>> + -o ${output} \
>> -T ${lds} ${objects}
>> else
>> objects="-Wl,--whole-archive \
>> @@ -81,9 +86,10 @@ vmlinux_link()
>> -Wl,--start-group \
>> ${KBUILD_VMLINUX_LIBS} \
>> -Wl,--end-group \
>> - ${@:2}"
>> + ${@}"
>>
>> - ${CC} ${CFLAGS_vmlinux} -o ${1} \
>> + ${CC} ${CFLAGS_vmlinux} \
>> + -o ${output} \
>> -Wl,-T,${lds} \
>> ${objects} \
>> -lutil -lrt -lpthread
>>
^ permalink raw reply
* [PATCH v2 net] net: gso: Fix skb_segment splat when splitting gso_size mangled skb having linear-headed frag_list
From: Shmulik Ladkani @ 2019-09-06 9:23 UTC (permalink / raw)
To: Alexander Duyck, Daniel Borkmann, Eric Dumazet, Willem de Bruijn
Cc: netdev, eyal, shmulik, Shmulik Ladkani
Historically, support for frag_list packets entering skb_segment() was
limited to frag_list members terminating on exact same gso_size
boundaries. This is verified with a BUG_ON since commit 89319d3801d1
("net: Add frag_list support to skb_segment"), quote:
As such we require all frag_list members terminate on exact MSS
boundaries. This is checked using BUG_ON.
As there should only be one producer in the kernel of such packets,
namely GRO, this requirement should not be difficult to maintain.
However, since commit 6578171a7ff0 ("bpf: add bpf_skb_change_proto helper"),
the "exact MSS boundaries" assumption no longer holds:
An eBPF program using bpf_skb_change_proto() DOES modify 'gso_size', but
leaves the frag_list members as originally merged by GRO with the
original 'gso_size'. Example of such programs are bpf-based NAT46 or
NAT64.
This lead to a kernel BUG_ON for flows involving:
- GRO generating a frag_list skb
- bpf program performing bpf_skb_change_proto() or bpf_skb_adjust_room()
- skb_segment() of the skb
See example BUG_ON reports in [0].
In commit 13acc94eff12 ("net: permit skb_segment on head_frag frag_list skb"),
skb_segment() was modified to support the "gso_size mangling" case of
a frag_list GRO'ed skb, but *only* for frag_list members having
head_frag==true (having a page-fragment head).
Alas, GRO packets having frag_list members with a linear kmalloced head
(head_frag==false) still hit the BUG_ON.
This commit adds support to skb_segment() for a 'head_skb' packet having
a frag_list whose members are *non* head_frag, with gso_size mangled, by
disabling SG and thus falling-back to copying the data from the given
'head_skb' into the generated segmented skbs - as suggested by Willem de
Bruijn [1].
Since this approach involves the penalty of skb_copy_and_csum_bits()
when building the segments, care was taken in order to enable this
solution only when required:
- untrusted gso_size, by testing SKB_GSO_DODGY is set
(SKB_GSO_DODGY is set by any gso_size mangling functions in
net/core/filter.c)
- the frag_list is non empty, its item is a non head_frag, *and* the
headlen of the given 'head_skb' does not match the gso_size.
[0]
https://lore.kernel.org/netdev/20190826170724.25ff616f@pixies/
https://lore.kernel.org/netdev/9265b93f-253d-6b8c-f2b8-4b54eff1835c@fb.com/
[1]
https://lore.kernel.org/netdev/CA+FuTSfVsgNDi7c=GUU8nMg2hWxF2SjCNLXetHeVPdnxAW5K-w@mail.gmail.com/
Fixes: 6578171a7ff0 ("bpf: add bpf_skb_change_proto helper")
Suggested-by: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Alexander Duyck <alexander.duyck@gmail.com>
Signed-off-by: Shmulik Ladkani <shmulik.ladkani@gmail.com>
---
v2: reorder the test conditions, as suggested by Alexander Duyck
---
net/core/skbuff.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index ea8e8d332d85..d540d00b93a9 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3670,6 +3670,25 @@ struct sk_buff *skb_segment(struct sk_buff *head_skb,
int pos;
int dummy;
+ if (list_skb && !list_skb->head_frag && skb_headlen(list_skb) &&
+ (skb_shinfo(head_skb)->gso_type & SKB_GSO_DODGY)) {
+ /* gso_size is untrusted, and we have a frag_list with a linear
+ * non head_frag head.
+ *
+ * (we assume checking the first list_skb member suffices;
+ * i.e if either of the list_skb members have non head_frag
+ * head, then the first one has too).
+ *
+ * If head_skb's headlen does not fit requested gso_size, it
+ * means that the frag_list members do NOT terminate on exact
+ * gso_size boundaries. Hence we cannot perform skb_frag_t page
+ * sharing. Therefore we must fallback to copying the frag_list
+ * skbs; we do so by disabling SG.
+ */
+ if (mss != GSO_BY_FRAGS && mss != skb_headlen(head_skb))
+ features &= ~NETIF_F_SG;
+ }
+
__skb_push(head_skb, doffset);
proto = skb_network_protocol(head_skb, &dummy);
if (unlikely(!proto))
--
2.19.1
^ permalink raw reply related
* Re: [PATCH bpf-next 0/6] selftests/bpf: move sockopt tests under test_progs
From: Andrii Nakryiko @ 2019-09-06 9:32 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Stanislav Fomichev, Networking, bpf, David S. Miller,
Alexei Starovoitov, Daniel Borkmann
In-Reply-To: <20190904230331.ld4zsn4jgldu7l6q@ast-mbp.dhcp.thefacebook.com>
On Wed, Sep 4, 2019 at 4:03 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Wed, Sep 04, 2019 at 09:25:03AM -0700, Stanislav Fomichev wrote:
> > Now that test_progs is shaping into more generic test framework,
> > let's convert sockopt tests to it. This requires adding
> > a helper to create and join a cgroup first (test__join_cgroup).
> > Since we already hijack stdout/stderr that shouldn't be
> > a problem (cgroup helpers log to stderr).
> >
> > The rest of the patches just move sockopt tests files under prog_tests/
> > and do the required small adjustments.
>
> Looks good. Thank you for working on it.
> Could you de-verbose setsockopt test a bit?
> #23/32 setsockopt: deny write ctx->retval:OK
> #23/33 setsockopt: deny read ctx->retval:OK
> #23/34 setsockopt: deny writing to ctx->optval:OK
> #23/35 setsockopt: deny writing to ctx->optval_end:OK
> #23/36 setsockopt: allow IP_TOS <= 128:OK
> #23/37 setsockopt: deny IP_TOS > 128:OK
> 37 subtests is a bit too much spam.
If we merged test_btf into test_progs, we'd have >150 subtests, which
would be pretty verbose as well. But the benefit of subtest is that
you can run just that sub-test and debug/verify just it, without all
the rest stuff.
So I'm wondering, if too many lines of default output is the only
problem, should we just not output per-subtest line by default?
>
^ permalink raw reply
* [PATCH net-next] ionic: Remove unused including <linux/version.h>
From: YueHaibing @ 2019-09-06 9:54 UTC (permalink / raw)
To: Shannon Nelson, Pensando Drivers, David S . Miller
Cc: YueHaibing, netdev, kernel-janitors
Remove including <linux/version.h> that don't need it.
Signed-off-by: YueHaibing <yuehaibing@huawei.com>
---
drivers/net/ethernet/pensando/ionic/ionic_main.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_main.c b/drivers/net/ethernet/pensando/ionic/ionic_main.c
index 5ec67f3f1853..15e432386b35 100644
--- a/drivers/net/ethernet/pensando/ionic/ionic_main.c
+++ b/drivers/net/ethernet/pensando/ionic/ionic_main.c
@@ -2,7 +2,6 @@
/* Copyright(c) 2017 - 2019 Pensando Systems, Inc */
#include <linux/module.h>
-#include <linux/version.h>
#include <linux/netdevice.h>
#include <linux/utsname.h>
^ permalink raw reply related
* [PATCH] Bluetooth: hidp: Fix error checks in hidp_get/set_raw_report
From: Dan Elkouby @ 2019-09-06 9:41 UTC (permalink / raw)
Cc: Dan Elkouby, Marcel Holtmann, Johan Hedberg, David S. Miller,
Dan Carpenter, Fabian Henneke, Brian Norris, Al Viro,
Andrea Parri, linux-bluetooth, netdev, linux-kernel
Commit 48d9cc9d85dd ("Bluetooth: hidp: Let hidp_send_message return
number of queued bytes") changed hidp_send_message to return non-zero
values on success, which some other bits did not expect. This caused
spurious errors to be propagated through the stack, breaking some (all?)
drivers, such as hid-sony for the Dualshock 4 in Bluetooth mode.
Signed-off-by: Dan Elkouby <streetwalkermc@gmail.com>
---
net/bluetooth/hidp/core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 8d889969ae7e..bef84b95e2c4 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -267,7 +267,7 @@ static int hidp_get_raw_report(struct hid_device *hid,
set_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
data[0] = report_number;
ret = hidp_send_ctrl_message(session, report_type, data, 1);
- if (ret)
+ if (ret < 0)
goto err;
/* Wait for the return of the report. The returned report
@@ -343,7 +343,7 @@ static int hidp_set_raw_report(struct hid_device *hid, unsigned char reportnum,
data[0] = reportnum;
set_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags);
ret = hidp_send_ctrl_message(session, report_type, data, count);
- if (ret)
+ if (ret < 0)
goto err;
/* Wait for the ACK from the device. */
--
2.23.0
^ permalink raw reply related
* [PATCH] tcp: fix tcp_disconnect() not clear tp->fastopen_rsk sometimes
From: chunguo feng @ 2019-09-06 9:34 UTC (permalink / raw)
To: edumazet
Cc: davem, kuznet, yoshfuji, ast, daniel, netdev, kafai,
songliubraving, yhs, linux-kernel, bpf, fengchunguo
From: fengchunguo <chunguo.feng@amlogic.com>
This patch avoids fastopen_rsk not be cleared every times, then occur
the below BUG_ON:
tcp_v4_destroy_sock
->BUG_ON(tp->fastopen_rsk);
When playback some videos from netwrok,used tcp_disconnect continually.
Call trace:
kfree+0x210/0x250
tcp_v4_destroy_sock+0xb8/0x1b0
tcp_v6_destroy_sock+0x20/0x34
inet_csk_destroy_sock+0x58/0x114
tcp_done+0x144/0x148
tcp_rcv_state_process+0x5d4/0xe3c
tcp_v4_do_rcv+0x74/0x240
tcp_v4_rcv+0xaac/0xba0
ip_local_deliver_finish+0xe8/0x25c
ip_local_deliver+0x60/0x118
ip_rcv+0x70/0x108
__netif_receive_skb_core+0x6f8/0xb80
process_backlog+0xe4/0x1f4
napi_poll+0x94/0x1ec
net_rx_action+0xe4/0x224
__do_softirq+0x16c/0x3bc
do_softirq.part.15+0x70/0x74
do_softirq+0x24/0x2c
netif_rx_ni+0x108/0x138
dhd_rxf_thread+0x134/0x1e4
kthread+0x114/0x140
ret_from_fork+0x10/0x18
Signed-off-by: fengchunguo <chunguo.feng@amlogic.com>
---
net/ipv4/tcp.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 61082065b26a..f5c354c0b24c 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2655,6 +2655,7 @@ int tcp_disconnect(struct sock *sk, int flags)
/* Clean up fastopen related fields */
tcp_free_fastopen_req(tp);
inet->defer_connect = 0;
+ tp->fastopen_rsk = 0;
WARN_ON(inet->inet_num && !icsk->icsk_bind_hash);
--
2.22.0
^ permalink raw reply related
* [PATCH net v2] bridge/mdb: remove wrong use of NLM_F_MULTI
From: Nicolas Dichtel @ 2019-09-06 9:47 UTC (permalink / raw)
To: davem; +Cc: roopa, netdev, bridge, Nicolas Dichtel, Nikolay Aleksandrov
NLM_F_MULTI must be used only when a NLMSG_DONE message is sent at the end.
In fact, NLMSG_DONE is sent only at the end of a dump.
Libraries like libnl will wait forever for NLMSG_DONE.
Fixes: 949f1e39a617 ("bridge: mdb: notify on router port add and del")
CC: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
---
v2:
add netdev and bridge ml :D
remove Satish Ashok <sashok@cumulusnetworks.com> (its mail bounces)
net/bridge/br_mdb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
index bf6acd34234d..63f9c08625f0 100644
--- a/net/bridge/br_mdb.c
+++ b/net/bridge/br_mdb.c
@@ -437,7 +437,7 @@ static int nlmsg_populate_rtr_fill(struct sk_buff *skb,
struct nlmsghdr *nlh;
struct nlattr *nest;
- nlh = nlmsg_put(skb, pid, seq, type, sizeof(*bpm), NLM_F_MULTI);
+ nlh = nlmsg_put(skb, pid, seq, type, sizeof(*bpm), 0);
if (!nlh)
return -EMSGSIZE;
--
2.21.0
^ permalink raw reply related
* Re: [PATCH net-next,v3 0/4] flow_offload: update mangle action representation
From: Edward Cree @ 2019-09-06 10:02 UTC (permalink / raw)
To: Pablo Neira Ayuso, netfilter-devel
Cc: davem, netdev, jakub.kicinski, jiri, saeedm, vishal, vladbu
In-Reply-To: <20190906000403.3701-1-pablo@netfilter.org>
On 06/09/2019 01:03, Pablo Neira Ayuso wrote:
> This patch updates the mangle action representation:
>
> Patch 1) Undo bitwise NOT operation on the mangle mask (coming from tc
> pedit userspace).
>
> Patch 2) mangle value &= mask from the front-end side.
>
> Patch 3) adjust offset, length and coalesce consecutive pedit keys into
> one single action.
>
> Patch 4) add support for payload mangling for netfilter.
>
> After this patchset:
>
> * Offset to payload does not need to be on the 32-bits boundaries anymore.
> This patchset adds front-end code to adjust the offset and length coming
> from the tc pedit representation, so drivers get an exact header field
> offset and length.
>
> * This new front-end code coalesces consecutive pedit actions into one
> single action, so drivers can mangle IPv6 and ethernet address fields
> in one go, instead once for each 32-bit word.
>
> On the driver side, diffstat -t shows that drivers code to deal with
> payload mangling gets simplified:
>
> INSERTED,DELETED,MODIFIED,FILENAME
> 46,116,0,drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c (-70 LOC)
> 12,28,0,drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.h (-16 LOC)
> 26,54,0,drivers/net/ethernet/mellanox/mlx5/core/en_tc.c (-27 LOC)
> 89,111,0,drivers/net/ethernet/netronome/nfp/flower/action.c (-17 LOC)
>
> While, on the front-end side the balance is the following:
>
> 123,22,0,net/sched/cls_api.c (+101 LOC)
>
> Changes since v2:
>
> * Fix is_action_keys_supported() breakage in mlx5 reported by Vlad Buslov.
Still NAK for the same reasons as three versions ago (when it was called
"netfilter: payload mangling offload support"), you've never managed to
explain why this extra API complexity is useful. (Reducing LOC does not
mean you've reduced complexity.)
As Jakub said, 'We suffered through enough haphazard "updates"'. Please
can you fix the problems your previous API changes caused (I still haven't
had an answer about the flow block changes since sending you my driver code
two weeks ago) before trying to ram new ones through.
-Ed
^ permalink raw reply
* Re: [PATCH 0/2] Revert and rework on the metadata accelreation
From: Jason Wang @ 2019-09-06 10:02 UTC (permalink / raw)
To: Jason Gunthorpe
Cc: mst@redhat.com, kvm@vger.kernel.org,
virtualization@lists.linux-foundation.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, aarcange@redhat.com,
jglisse@redhat.com, linux-mm@kvack.org
In-Reply-To: <20190905135907.GB6011@mellanox.com>
On 2019/9/5 下午9:59, Jason Gunthorpe wrote:
> On Thu, Sep 05, 2019 at 08:27:34PM +0800, Jason Wang wrote:
>> Hi:
>>
>> Per request from Michael and Jason, the metadata accelreation is
>> reverted in this version and rework in next version.
>>
>> Please review.
>>
>> Thanks
>>
>> Jason Wang (2):
>> Revert "vhost: access vq metadata through kernel virtual address"
>> vhost: re-introducing metadata acceleration through kernel virtual
>> address
> There are a bunch of patches in the queue already that will help
> vhost, and I a working on one for next cycle that will help alot more
> too.
I will check those patches, but if you can give me some pointers or
keywords it would be much appreciated.
>
> I think you should apply the revert this cycle and rebase the other
> patch for next..
>
> Jason
Yes, the plan is to revert in this release cycle.
Thanks
^ permalink raw reply
* Re: [PATCHv3 net-next] ipmr: remove hard code cache_resolve_queue_len limit
From: Nikolay Aleksandrov @ 2019-09-06 10:08 UTC (permalink / raw)
To: Hangbin Liu, netdev
Cc: Phil Karn, Sukumar Gopalakrishnan, David S . Miller,
Alexey Kuznetsov, Hideaki YOSHIFUJI, Eric Dumazet
In-Reply-To: <20190906073601.10525-1-liuhangbin@gmail.com>
On 06/09/2019 10:36, Hangbin Liu wrote:
> This is a re-post of previous patch wrote by David Miller[1].
>
> Phil Karn reported[2] that on busy networks with lots of unresolved
> multicast routing entries, the creation of new multicast group routes
> can be extremely slow and unreliable.
>
> The reason is we hard-coded multicast route entries with unresolved source
> addresses(cache_resolve_queue_len) to 10. If some multicast route never
> resolves and the unresolved source addresses increased, there will
> be no ability to create new multicast route cache.
>
> To resolve this issue, we need either add a sysctl entry to make the
> cache_resolve_queue_len configurable, or just remove cache_resolve_queue_len
> limit directly, as we already have the socket receive queue limits of mrouted
> socket, pointed by David.
>
> From my side, I'd perfer to remove the cache_resolve_queue_len limit instead
> of creating two more(IPv4 and IPv6 version) sysctl entry.
>
> [1] https://lkml.org/lkml/2018/7/22/11
> [2] https://lkml.org/lkml/2018/7/21/343
>
> v3: instead of remove cache_resolve_queue_len totally, let's only remove
> the hard code limit when allocate the unresolved cache, as Eric Dumazet
> suggested, so we don't need to re-count it in other places.
>
> v2: hold the mfc_unres_lock while walking the unresolved list in
> queue_count(), as Nikolay Aleksandrov remind.
>
> Reported-by: Phil Karn <karn@ka9q.net>
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> net/ipv4/ipmr.c | 4 ++--
> net/ipv6/ip6mr.c | 4 ++--
> 2 files changed, 4 insertions(+), 4 deletions(-)
>
Please CC all interested parties who have reviewed/commented on previous versions.
I'd also add a Suggested-by tag such as:
Suggested-by: Eric Dumazet <eric.dumazet@gmail.com>
Looks good to me, thanks!
Reviewed-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
^ permalink raw reply
* Re: [PATCH] Bluetooth: hidp: Fix error checks in hidp_get/set_raw_report
From: Dan Carpenter @ 2019-09-06 10:13 UTC (permalink / raw)
To: Dan Elkouby
Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Fabian Henneke,
Brian Norris, Al Viro, Andrea Parri, linux-bluetooth, netdev,
linux-kernel
In-Reply-To: <20190906094158.8854-1-streetwalkermc@gmail.com>
On Fri, Sep 06, 2019 at 12:41:57PM +0300, Dan Elkouby wrote:
> Commit 48d9cc9d85dd ("Bluetooth: hidp: Let hidp_send_message return
> number of queued bytes") changed hidp_send_message to return non-zero
> values on success, which some other bits did not expect. This caused
> spurious errors to be propagated through the stack, breaking some (all?)
> drivers, such as hid-sony for the Dualshock 4 in Bluetooth mode.
>
> Signed-off-by: Dan Elkouby <streetwalkermc@gmail.com>
I think we also need to update update ms_ff_worker() which assumes that
hid_hw_output_report() returns zero on success. Please use the Fixes
tag for this since a lot of scripts rely on it to decide what to
backport.
Fixes: 48d9cc9d85dd ("Bluetooth: hidp: Let hidp_send_message return number of queued bytes")
Otherwise, it looks good. Thanks for catching this.
regards,
dan carpenter
^ permalink raw reply
* Re: [PATCH iproute2-next] bpf: fix snprintf truncation warning
From: Andrea Claudi @ 2019-09-06 10:19 UTC (permalink / raw)
To: Stephen Hemminger; +Cc: David Ahern, linux-netdev, David Ahern
In-Reply-To: <20190905085147.72772bba@hermes.lan>
On Thu, Sep 5, 2019 at 5:51 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
>
> On Thu, 5 Sep 2019 13:44:55 +0200
> Andrea Claudi <aclaudi@redhat.com> wrote:
>
> > On Thu, Sep 5, 2019 at 12:15 AM David Ahern <dsahern@gmail.com> wrote:
> > >
> > > On 9/4/19 9:50 AM, Andrea Claudi wrote:
> > > > gcc v9.2.1 produces the following warning compiling iproute2:
> > > >
> > > > bpf.c: In function ‘bpf_get_work_dir’:
> > > > bpf.c:784:49: warning: ‘snprintf’ output may be truncated before the last format character [-Wformat-truncation=]
> > > > 784 | snprintf(bpf_wrk_dir, sizeof(bpf_wrk_dir), "%s/", mnt);
> > > > | ^
> > > > bpf.c:784:2: note: ‘snprintf’ output between 2 and 4097 bytes into a destination of size 4096
> > > > 784 | snprintf(bpf_wrk_dir, sizeof(bpf_wrk_dir), "%s/", mnt);
> > > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > > >
> > > > Fix it extending bpf_wrk_dir size by 1 byte for the extra "/" char.
> > > >
> > > > Signed-off-by: Andrea Claudi <aclaudi@redhat.com>
> > > > ---
> > > > lib/bpf.c | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/lib/bpf.c b/lib/bpf.c
> > > > index 7d2a322ffbaec..95de7894a93ce 100644
> > > > --- a/lib/bpf.c
> > > > +++ b/lib/bpf.c
> > > > @@ -742,7 +742,7 @@ static int bpf_gen_hierarchy(const char *base)
> > > > static const char *bpf_get_work_dir(enum bpf_prog_type type)
> > > > {
> > > > static char bpf_tmp[PATH_MAX] = BPF_DIR_MNT;
> > > > - static char bpf_wrk_dir[PATH_MAX];
> > > > + static char bpf_wrk_dir[PATH_MAX + 1];
> > > > static const char *mnt;
> > > > static bool bpf_mnt_cached;
> > > > const char *mnt_env = getenv(BPF_ENV_MNT);
> > > >
> > >
> > > PATH_MAX is meant to be the max length for a filesystem path including
> > > the null terminator, so I think it would be better to change the
> > > snprintf to 'sizeof(bpf_wrk_dir) - 1'.
> >
> > With 'sizeof(bpf_wrk_dir) - 1' snprintf simply truncates at byte 4095
> > instead of byte 4096.
> > This means that bpf_wrk_dir can again be truncated before the final
> > "/", as it is by now.
> > Am I missing something?
> >
> > Trying your suggestion I have this slightly different warning message:
> >
> > bpf.c: In function ‘bpf_get_work_dir’:
> > bpf.c:784:52: warning: ‘/’ directive output may be truncated writing 1
> > byte into a region of size between 0 and 4095 [-Wformat-truncation=]
> > 784 | snprintf(bpf_wrk_dir, sizeof(bpf_wrk_dir) - 1, "%s/", mnt);
> > | ^
> > bpf.c:784:2: note: ‘snprintf’ output between 2 and 4097 bytes into a
> > destination of size 4095
> > 784 | snprintf(bpf_wrk_dir, sizeof(bpf_wrk_dir) - 1, "%s/", mnt);
> > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Why not rework this to use asprintf and avoid having huge buffers on stack?
Thanks for the suggestion. There are a lot of similar usages in
lib/bpf.c, I'll send a v2 to rework them all.
^ permalink raw reply
* Re: [PATCH] tcp: fix tcp_disconnect() not clear tp->fastopen_rsk sometimes
From: Eric Dumazet @ 2019-09-06 10:33 UTC (permalink / raw)
To: chunguo feng, Yuchung Cheng, Neal Cardwell
Cc: David Miller, Alexei Starovoitov, Daniel Borkmann, netdev,
Yonghong Song, LKML
In-Reply-To: <20190906093429.930-1-chunguo.feng@amlogic.com>
On Fri, Sep 6, 2019 at 11:34 AM chunguo feng <chunguo.feng@amlogic.com> wrote:
>
> From: fengchunguo <chunguo.feng@amlogic.com>
>
> This patch avoids fastopen_rsk not be cleared every times, then occur
> the below BUG_ON:
> tcp_v4_destroy_sock
> ->BUG_ON(tp->fastopen_rsk);
>
> When playback some videos from netwrok,used tcp_disconnect continually.
Wow, tcp_disconnect() being used by something else than syzkaller !
> kthread+0x114/0x140
> ret_from_fork+0x10/0x18
>
> Signed-off-by: fengchunguo <chunguo.feng@amlogic.com>
> ---
> net/ipv4/tcp.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 61082065b26a..f5c354c0b24c 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -2655,6 +2655,7 @@ int tcp_disconnect(struct sock *sk, int flags)
> /* Clean up fastopen related fields */
> tcp_free_fastopen_req(tp);
> inet->defer_connect = 0;
> + tp->fastopen_rsk = 0;
>
> WARN_ON(inet->inet_num && !icsk->icsk_bind_hash);
>
This seems suspicious to me.
Are we leaking a block of memory after your patch ?
If the block of memory has been freed, maybe clear the pointer at the
place the free happened ?
I am traveling to Lisbon for LPC, maybe Yuchung or Neal can take a look.
^ permalink raw reply
* Re: [PATCH] Bluetooth: hidp: Fix error checks in hidp_get/set_raw_report
From: Dan Elkouby @ 2019-09-06 10:40 UTC (permalink / raw)
To: Dan Carpenter
Cc: Marcel Holtmann, Johan Hedberg, David S. Miller, Fabian Henneke,
Brian Norris, Al Viro, Andrea Parri, linux-bluetooth, netdev,
linux-kernel
In-Reply-To: <20190906101306.GA12017@kadam>
On Fri, Sep 6, 2019 at 1:14 PM Dan Carpenter wrote:
> I think we also need to update update ms_ff_worker() which assumes that
> hid_hw_output_report() returns zero on success.
Yes, it looks like that's the case. Should I amend my patch to include
this fix, or should it be a separate patch? I don't have access to any
hardware covered by hid-microsoft, so I won't be able to test it.
> Please use the Fixes
> tag for this since a lot of scripts rely on it to decide what to
> backport.
>
> Fixes: 48d9cc9d85dd ("Bluetooth: hidp: Let hidp_send_message return number of queued bytes")
Will do.
> Otherwise, it looks good. Thanks for catching this.
Thanks for taking a look!
(Sorry for sending this twice, I'm not used to mailing lists and forgot
to reply to all.)
^ permalink raw reply
* Re: [PATCH net-next,v3 0/4] flow_offload: update mangle action representation
From: Pablo Neira Ayuso @ 2019-09-06 10:56 UTC (permalink / raw)
To: Edward Cree
Cc: netfilter-devel, davem, netdev, jakub.kicinski, jiri, saeedm,
vishal, vladbu
In-Reply-To: <679ced4b-8bcd-5479-2773-7c75452c2a32@solarflare.com>
On Fri, Sep 06, 2019 at 11:02:18AM +0100, Edward Cree wrote:
> On 06/09/2019 01:03, Pablo Neira Ayuso wrote:
> > This patch updates the mangle action representation:
> >
> > Patch 1) Undo bitwise NOT operation on the mangle mask (coming from tc
> > pedit userspace).
> >
> > Patch 2) mangle value &= mask from the front-end side.
> >
> > Patch 3) adjust offset, length and coalesce consecutive pedit keys into
> > one single action.
> >
> > Patch 4) add support for payload mangling for netfilter.
> >
> > After this patchset:
> >
> > * Offset to payload does not need to be on the 32-bits boundaries anymore.
> > This patchset adds front-end code to adjust the offset and length coming
> > from the tc pedit representation, so drivers get an exact header field
> > offset and length.
> >
> > * This new front-end code coalesces consecutive pedit actions into one
> > single action, so drivers can mangle IPv6 and ethernet address fields
> > in one go, instead once for each 32-bit word.
> >
> > On the driver side, diffstat -t shows that drivers code to deal with
> > payload mangling gets simplified:
> >
> > INSERTED,DELETED,MODIFIED,FILENAME
> > 46,116,0,drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.c (-70 LOC)
> > 12,28,0,drivers/net/ethernet/chelsio/cxgb4/cxgb4_tc_flower.h (-16 LOC)
> > 26,54,0,drivers/net/ethernet/mellanox/mlx5/core/en_tc.c (-27 LOC)
> > 89,111,0,drivers/net/ethernet/netronome/nfp/flower/action.c (-17 LOC)
> >
> > While, on the front-end side the balance is the following:
> >
> > 123,22,0,net/sched/cls_api.c (+101 LOC)
> >
> > Changes since v2:
> >
> > * Fix is_action_keys_supported() breakage in mlx5 reported by Vlad Buslov.
>
> Still NAK for the same reasons as three versions ago (when it was called
> "netfilter: payload mangling offload support"), you've never managed to
> explain why this extra API complexity is useful. (Reducing LOC does not
> mean you've reduced complexity.)
Oh well...
Patch 1) Mask is inverted for no reason, just because tc pedit needs
this in this way. All drivers reverse this mask.
Patch 2) All drivers mask out meaningless fields in the value field.
Patch 3) Without this patchset, offsets are on the 32-bit boundary.
Drivers need to play with the 32-bit mask to infer what field they are
supposed to mangle... eg. with 32-bit offset alignment, checking if
the use want to alter the ttl/hop_limit need for helper structures to
check the 32-bit mask. Mangling a IPv6 address comes with one single
action...
^ permalink raw reply
* [PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message
From: Dan Elkouby @ 2019-09-06 11:06 UTC (permalink / raw)
To: Dan Carpenter
Cc: Dan Elkouby, Jiri Kosina, Benjamin Tissoires, Marcel Holtmann,
Johan Hedberg, David S. Miller, Brian Norris, Fabian Henneke,
Al Viro, Andrea Parri, linux-input, linux-kernel, linux-bluetooth,
netdev
In-Reply-To: <20190906101306.GA12017@kadam>
hidp_send_message was changed to return non-zero values on success,
which some other bits did not expect. This caused spurious errors to be
propagated through the stack, breaking some drivers, such as hid-sony
for the Dualshock 4 in Bluetooth mode.
As pointed out by Dan Carpenter, hid-microsoft directly relied on that
assumption as well.
Fixes: 48d9cc9d85dd ("Bluetooth: hidp: Let hidp_send_message return number of queued bytes")
Signed-off-by: Dan Elkouby <streetwalkermc@gmail.com>
---
drivers/hid/hid-microsoft.c | 2 +-
net/bluetooth/hidp/core.c | 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/hid/hid-microsoft.c b/drivers/hid/hid-microsoft.c
index 8b3a922bdad3..2cf83856f2e4 100644
--- a/drivers/hid/hid-microsoft.c
+++ b/drivers/hid/hid-microsoft.c
@@ -303,7 +303,7 @@ static void ms_ff_worker(struct work_struct *work)
r->magnitude[MAGNITUDE_WEAK] = ms->weak; /* right actuator */
ret = hid_hw_output_report(hdev, (__u8 *)r, sizeof(*r));
- if (ret)
+ if (ret < 0)
hid_warn(hdev, "failed to send FF report\n");
}
diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 8d889969ae7e..bef84b95e2c4 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -267,7 +267,7 @@ static int hidp_get_raw_report(struct hid_device *hid,
set_bit(HIDP_WAITING_FOR_RETURN, &session->flags);
data[0] = report_number;
ret = hidp_send_ctrl_message(session, report_type, data, 1);
- if (ret)
+ if (ret < 0)
goto err;
/* Wait for the return of the report. The returned report
@@ -343,7 +343,7 @@ static int hidp_set_raw_report(struct hid_device *hid, unsigned char reportnum,
data[0] = reportnum;
set_bit(HIDP_WAITING_FOR_SEND_ACK, &session->flags);
ret = hidp_send_ctrl_message(session, report_type, data, count);
- if (ret)
+ if (ret < 0)
goto err;
/* Wait for the ACK from the device. */
--
2.23.0
^ permalink raw reply related
* Re: [PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message
From: Dan Carpenter @ 2019-09-06 11:11 UTC (permalink / raw)
To: Dan Elkouby
Cc: Jiri Kosina, Benjamin Tissoires, Marcel Holtmann, Johan Hedberg,
David S. Miller, Brian Norris, Fabian Henneke, Al Viro,
Andrea Parri, linux-input, linux-kernel, linux-bluetooth, netdev
In-Reply-To: <20190906110645.27601-1-streetwalkermc@gmail.com>
On Fri, Sep 06, 2019 at 02:06:44PM +0300, Dan Elkouby wrote:
> hidp_send_message was changed to return non-zero values on success,
> which some other bits did not expect. This caused spurious errors to be
> propagated through the stack, breaking some drivers, such as hid-sony
> for the Dualshock 4 in Bluetooth mode.
>
> As pointed out by Dan Carpenter, hid-microsoft directly relied on that
> assumption as well.
>
> Fixes: 48d9cc9d85dd ("Bluetooth: hidp: Let hidp_send_message return number of queued bytes")
>
> Signed-off-by: Dan Elkouby <streetwalkermc@gmail.com>
Thanks!
Reviewed-by: Dan Carpenter <dan.carpenter@oracle.com>
regards,
dan carpenter
^ permalink raw reply
* [PATCH] be2net: make two arrays static const, makes object smaller
From: Colin King @ 2019-09-06 11:19 UTC (permalink / raw)
To: Sathya Perla, Ajit Khaparde, Sriharsha Basavapatna, Somnath Kotur,
David S . Miller, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Don't populate the arrays on the stack but instead make them
static const. Makes the object code smaller by 281 bytes.
Before:
text data bss dec hex filename
87553 5672 0 93225 16c29 benet/be_cmds.o
After:
text data bss dec hex filename
87112 5832 0 92944 16b10 benet/be_cmds.o
(gcc version 9.2.1, amd64)
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/emulex/benet/be_cmds.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 323976c811e9..701c12c9e033 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -2756,7 +2756,7 @@ static int be_flash_BEx(struct be_adapter *adapter,
bool crc_match;
const u8 *p;
- struct flash_comp gen3_flash_types[] = {
+ static const struct flash_comp gen3_flash_types[] = {
{ BE3_ISCSI_PRIMARY_IMAGE_START, OPTYPE_ISCSI_ACTIVE,
BE3_COMP_MAX_SIZE, IMAGE_FIRMWARE_ISCSI},
{ BE3_REDBOOT_START, OPTYPE_REDBOOT,
@@ -2779,7 +2779,7 @@ static int be_flash_BEx(struct be_adapter *adapter,
BE3_PHY_FW_COMP_MAX_SIZE, IMAGE_FIRMWARE_PHY}
};
- struct flash_comp gen2_flash_types[] = {
+ static const struct flash_comp gen2_flash_types[] = {
{ BE2_ISCSI_PRIMARY_IMAGE_START, OPTYPE_ISCSI_ACTIVE,
BE2_COMP_MAX_SIZE, IMAGE_FIRMWARE_ISCSI},
{ BE2_REDBOOT_START, OPTYPE_REDBOOT,
--
2.20.1
^ permalink raw reply related
* Re: r8169: Performance regression and latency instability
From: Juliana Rodrigueiro @ 2019-09-06 11:25 UTC (permalink / raw)
To: Heiner Kallweit, Holger Hoffstätte, Eric Dumazet, netdev
Cc: Thomas Jarosch, Gerd v. Egidy
In-Reply-To: <a757135b-ec79-0ad5-5886-2989330424ee@intra2net.com>
Hi all!
I would like to kindly ask your help to understand this subject, since my
familiarity with the network stack is limited. I'm still
trying to find a solution for the latency problems with Realtek cards I
reported earlier.
Why does the GSO have to be forced on the socket level
if drivers for high performance chips will have it enabled by default?
A consequence of forcing GSO is that TSO is also forced [1] in
sk_setup_caps() via the NETIF_F_GSO_SOFTWARE flag (list of features with
software fallbacks). I'm sorry for my ignorance, but I wonder if TSO
will really be done in software in case the card does have NETIF_F_TSO
capability but "might not work properly" [2]. (Plus tx-checksum and SG
are all off)
Effectively for me, the following patch shows the same good performance
as the 3.14 kernel (or kernel 4.19 with reverted "tcp: switch to GSO
being always on").
diff --git a/net/core/sock.c b/net/core/sock.c
index 9c32e8eb64da..d792d12e0f66 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -1770,7 +1770,7 @@ void sk_setup_caps(struct sock *sk, struct
dst_entry *dst)
sk_dst_set(sk, dst);
sk->sk_route_caps = dst->dev->features | sk->sk_route_forced_caps;
if (sk->sk_route_caps & NETIF_F_GSO)
- sk->sk_route_caps |= NETIF_F_GSO_SOFTWARE;
+ sk->sk_route_caps |= (NETIF_F_GSO_SOFTWARE &
~NETIF_F_ALL_TSO);
sk->sk_route_caps &= ~sk->sk_route_nocaps;
if (sk_can_gso(sk)) {
if (dst->header_len && !xfrm_dst_offload_ok(dst)) {
Any help is highly appreciated.
Best regards,
Juliana.
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/net/core/sock.c?h=linux-4.19.y#n1773
[2]
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/drivers/net/ethernet/realtek/r8169.c?h=linux-4.19.y#n7590
^ permalink raw reply related
* [PATCH] net: hns3: make array spec_opcode static const, makes object smaller
From: Colin King @ 2019-09-06 11:28 UTC (permalink / raw)
To: Yisen Zhuang, Salil Mehta, David S . Miller, Peng Li, netdev
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
Don't populate the array spec_opcode on the stack but instead make it
static const. Makes the object code smaller by 48 bytes.
Before:
text data bss dec hex filename
6914 1040 128 8082 1f92 hns3/hns3vf/hclgevf_cmd.o
After:
text data bss dec hex filename
6866 1040 128 8034 1f62 hns3/hns3vf/hclgevf_cmd.o
(gcc version 9.2.1, amd64)
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.c b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.c
index 4c2c9458648f..d5d1cc5d1b6e 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3vf/hclgevf_cmd.c
@@ -74,7 +74,7 @@ static bool hclgevf_cmd_csq_done(struct hclgevf_hw *hw)
static bool hclgevf_is_special_opcode(u16 opcode)
{
- u16 spec_opcode[] = {0x30, 0x31, 0x32};
+ static const u16 spec_opcode[] = {0x30, 0x31, 0x32};
int i;
for (i = 0; i < ARRAY_SIZE(spec_opcode); i++) {
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] Bluetooth: hidp: Fix assumptions on the return value of hidp_send_message
From: Jiri Kosina @ 2019-09-06 11:31 UTC (permalink / raw)
To: Dan Elkouby
Cc: Dan Carpenter, Benjamin Tissoires, Marcel Holtmann, Johan Hedberg,
David S. Miller, Brian Norris, Fabian Henneke, Al Viro,
Andrea Parri, linux-input, linux-kernel, linux-bluetooth, netdev
In-Reply-To: <20190906110645.27601-1-streetwalkermc@gmail.com>
On Fri, 6 Sep 2019, Dan Elkouby wrote:
> hidp_send_message was changed to return non-zero values on success,
> which some other bits did not expect. This caused spurious errors to be
> propagated through the stack, breaking some drivers, such as hid-sony
> for the Dualshock 4 in Bluetooth mode.
>
> As pointed out by Dan Carpenter, hid-microsoft directly relied on that
> assumption as well.
>
> Fixes: 48d9cc9d85dd ("Bluetooth: hidp: Let hidp_send_message return number of queued bytes")
>
> Signed-off-by: Dan Elkouby <streetwalkermc@gmail.com>
Reviewed-by: Jiri Kosina <jkosina@suse.cz>
Marcel, are you taking this through your tree?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox