* Re: [RFC PATCH 2/4] net: enable UDP gro on demand.
From: Willem de Bruijn @ 2018-09-17 14:07 UTC (permalink / raw)
To: Paolo Abeni
Cc: Network Development, David Miller, Willem de Bruijn,
steffen.klassert
In-Reply-To: <ceb3e427666e5b498aae13af1a97241e8216bcc6.camel@redhat.com>
On Mon, Sep 17, 2018 at 6:18 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On Sun, 2018-09-16 at 14:23 -0400, Willem de Bruijn wrote:
> > That udp gro implementation is clearly less complete than yours in
> > this patchset. The point I wanted to bring up for discussion is not the
> > protocol implementation, but the infrastructure for enabling it
> > conditionally.
>
> I'm still [trying to] processing your patchset ;) So please perdon me
> for any obvious interpretation mistakes...
>
> > Assuming cycle cost is comparable, what do you think of using the
> > existing sk offload callbacks to enable this on a per-socket basis?
>
> I have no objection about that, if there are no performance drawbacks.
> In my measures retpoline costs is relevant for every indirect call
> added. Using the existing sk offload approach will require an
> additional indirect call per packet compared to the implementation
> here.
Fair enough. The question is whether it is significant to the real workload.
This is also an issue with GRO processing in general, all those callbacks
as well as the two cacheline lookups to get to each callback.
> > As for the protocol-wide knob, I do strongly prefer something that can
> > work for all protocols, not just UDP.
>
> I like the general infrastructure idea. I think there is some agreement
> in avoiding the addition of more user-controllable knobs, as we already
> have a lot of them. If I read your patch correctly, user-space need to
> enable/disable the UDO GSO explicitly via procfs, right?
No, like other GRO callbacks, the feature is enabled by default.
Patch 7/8 disables the most expensive part behind a static key
until a socket actually registers a GRO callback, whether tunnel
or the new application GRO.
Patch 6/9 makes it possible to disable any protocol completely,
indeed through a sysctl.
> I tried to look for something that does not require user action.
>
> > I also implemented a version that
> > atomically swaps the struct ptr instead of the flag based approach I sent
> > for review. I'm fairly agnostic about that point.
>
> I think/fear security oriented guys may scream for the somewhat large
> deconstification ?!?
Hmm.. yes, interesting point. Since const pointers are a compile time
feature, in practice I don't think that they buy any protection against
callback pointer rewriting. Let me think about that some more.
>
> > One subtle issue is that I
> > believe we need to keep the gro_complete callbacks enabled, as gro
> > packets may be queued for completion when gro_receive gets disabled.
>
> Good point, thanks! I missed that.
>
> Cheers,
>
> Paolo
>
>
^ permalink raw reply
* Re: [PATCH net-next RFC 7/8] udp: gro behind static key
From: Willem de Bruijn @ 2018-09-17 14:10 UTC (permalink / raw)
To: steffen.klassert
Cc: Network Development, Paolo Abeni, David Miller, Willem de Bruijn
In-Reply-To: <20180917090323.GF23674@gauss3.secunet.de>
On Mon, Sep 17, 2018 at 5:03 AM Steffen Klassert
<steffen.klassert@secunet.com> wrote:
>
> On Fri, Sep 14, 2018 at 01:59:40PM -0400, Willem de Bruijn wrote:
> > From: Willem de Bruijn <willemb@google.com>
> >
> > Avoid the socket lookup cost in udp_gro_receive if no socket has a
> > gro callback configured.
> >
> > Signed-off-by: Willem de Bruijn <willemb@google.com>
>
> ...
>
> > diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> > index 4f6aa95a9b12..f44fe328aa0f 100644
> > --- a/net/ipv4/udp_offload.c
> > +++ b/net/ipv4/udp_offload.c
> > @@ -405,7 +405,7 @@ static struct sk_buff *udp4_gro_receive(struct list_head *head,
> > {
> > struct udphdr *uh = udp_gro_udphdr(skb);
> >
> > - if (unlikely(!uh))
> > + if (unlikely(!uh) || !static_branch_unlikely(&udp_encap_needed_key))
> > goto flush;
>
> If you use udp_encap_needed_key to enalbe UDP GRO, then a UDP
> encapsulation socket will enable it too. Not sure if this is
> intentional.
Yes. That is already the case to a certain point. The function was
introduced with tunnels and is enabled by tunnels, but so far only
compiles out the encap_rcv() branch in udp_qeueue_rcv_skb.
With patch 7/8 it also toggles the GRO path. Critically, both are
enabled as soon as a tunnel is registered.
>
> That said, enabling UDP GRO on a UDP encapsulation socket
> (ESP in UPD etc.) will fail badly as then encrypted ESP
> packets might be merged together. So we somehow should
> make sure that this does not happen.
Absolutely. This initial implementation probably breaks UDP tunnels
badly. That needs to be addressed.
>
> Anyway, this reminds me that we can support GRO for
> UDP encapsulation. It just requires separate GRO
> callbacks for the different encapsulation types.
^ permalink raw reply
* Re: [PATCH net-next RFC 7/8] udp: gro behind static key
From: Willem de Bruijn @ 2018-09-17 14:12 UTC (permalink / raw)
To: Paolo Abeni
Cc: Network Development, steffen.klassert, David Miller,
Willem de Bruijn
In-Reply-To: <203cd0cd80a6876b5d5de42e234aac5ff6a5ebf5.camel@redhat.com>
On Mon, Sep 17, 2018 at 6:24 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On Fri, 2018-09-14 at 13:59 -0400, Willem de Bruijn wrote:
> > diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c
> > index 4f6aa95a9b12..f44fe328aa0f 100644
> > --- a/net/ipv4/udp_offload.c
> > +++ b/net/ipv4/udp_offload.c
> > @@ -405,7 +405,7 @@ static struct sk_buff *udp4_gro_receive(struct list_head *head,
> > {
> > struct udphdr *uh = udp_gro_udphdr(skb);
> >
> > - if (unlikely(!uh))
> > + if (unlikely(!uh) || !static_branch_unlikely(&udp_encap_needed_key))
> > goto flush;
> >
> > /* Don't bother verifying checksum if we're going to flush anyway. */
>
> If I read this correctly, once udp_encap_needed_key is enabled, it will
> never be turned off, because the tunnel and encap socket shut down does
> not cope with udp_encap_needed_key.
>
> Perhaps we should take care of that, too.
Agreed. For now I reused what's already there, but I can extend
that with refcounting using static_branch_inc/static_branch_dec.
^ permalink raw reply
* Re: [PATCH net-next RFC 7/8] udp: gro behind static key
From: Willem de Bruijn @ 2018-09-17 14:19 UTC (permalink / raw)
To: steffen.klassert
Cc: Network Development, Paolo Abeni, David Miller, Willem de Bruijn
In-Reply-To: <20180917103728.GG23674@gauss3.secunet.de>
On Mon, Sep 17, 2018 at 6:37 AM Steffen Klassert
<steffen.klassert@secunet.com> wrote:
>
> On Fri, Sep 14, 2018 at 01:59:40PM -0400, Willem de Bruijn wrote:
> > From: Willem de Bruijn <willemb@google.com>
> >
> > Avoid the socket lookup cost in udp_gro_receive if no socket has a
> > gro callback configured.
>
> It would be nice if we could do GRO not just for GRO configured
> sockets, but also for flows that are going to be IPsec transformed
> or directly forwarded.
I thought about that, as we have GSO. An egregious hack enables
GRO for all registered local sockets that support it and for any flow
for which no local socket is registered:
@@ -365,11 +369,13 @@ struct sk_buff *udp_gro_receive(struct list_head
*head, struct sk_buff *skb,
rcu_read_lock();
sk = (*lookup)(skb, uh->source, uh->dest);
- if (sk && udp_sk(sk)->gro_receive)
- goto unflush;
- goto out_unlock;
+ if (!sk)
+ gro_receive_cb = udp_gro_receive_cb;
+ else if (!udp_sk(sk)->gro_receive)
+ goto out_unlock;
+ else
+ gro_receive_cb = udp_sk(sk)->gro_receive;
@@ -392,7 +398,7 @@ struct sk_buff *udp_gro_receive(struct list_head
*head, struct sk_buff *skb,
skb_gro_pull(skb, sizeof(struct udphdr)); /* pull
encapsulating udp header */
skb_gro_postpull_rcsum(skb, uh, sizeof(struct udphdr));
- pp = call_gro_receive_sk(udp_sk(sk)->gro_receive, sk, head, skb);
+ pp = call_gro_receive_sk(gro_receive_cb, sk, head, skb);
But not having a local socket does not imply forwarding path, of course.
>
> Maybe in case that forwarding is enabled on the receiving device,
> inet_gro_receive() could do a route lookup and allow GRO if the
> route lookup returned at forwarding route.
That's a better solution, if the cost is acceptable. We do have to
be careful against increasing per packet cycle cost in this path
given that it's a possible vector for DoS attempts.
> For flows that are likely software segmented after that, it
> would be worth to build packet chains insted of merging the
> payload. Packets of the same flow could travel together, but
> it would save the cost of the packet merging and segmenting.
With software GSO that is faster, as it would have to allocate
all the separate segment skbs in skb_segment later. Though
there is some complexity if MTUs differ.
With hardware UDP GSO, having a single skb will be cheaper in
the forwarding path. Using napi_gro_frags, device drivers really
do only end up allocating one skb for the GSO packet.
> This could be done similar to what I proposed for the list
> receive case:
>
> https://www.spinics.net/lists/netdev/msg522706.html
>
> How GRO should be done could be even configured
> by replacing the net_offload pointer similar
> to what Paolo propsed in his pachset with
> the inet_update_offload() function.
Right. The above hack also already has to use two distinct
callback assignments.
^ permalink raw reply
* [PATCH] net: phy: phylink: fix SFP interface autodetection
From: Baruch Siach @ 2018-09-17 14:19 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli
Cc: netdev, Russell King, Antoine Tenart, Gregory CLEMENT,
Baruch Siach
When the switching to the SFP detected link mode update the main
link_interface field as well. Otherwise, the link fails to come up when
the configured 'phy-mode' defers from the SFP detected mode.
This fixes 1GB SFP module link up on eth3 of the Macchiatobin board that
is configured in the DT to "2500base-x" phy-mode.
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
drivers/net/phy/phylink.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 3ba5cf2a8a5f..3ece48c86841 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1631,6 +1631,7 @@ static int phylink_sfp_module_insert(void *upstream,
if (pl->link_an_mode != MLO_AN_INBAND ||
pl->link_config.interface != config.interface) {
pl->link_config.interface = config.interface;
+ pl->link_interface = config.interface;
pl->link_an_mode = MLO_AN_INBAND;
changed = true;
--
2.18.0
^ permalink raw reply related
* Re: [PATCH] bonding: avoid repeated display of same link status change
From: Eric Dumazet @ 2018-09-17 14:38 UTC (permalink / raw)
To: mk.singh, netdev
Cc: Jay Vosburgh, Veaceslav Falico, Andy Gospodarek, David S. Miller,
linux-kernel
In-Reply-To: <20180917072059.32657-1-mk.singh@oracle.com>
On 09/17/2018 12:20 AM, mk.singh@oracle.com wrote:
> From: Manish Kumar Singh <mk.singh@oracle.com>
>
> When link status change needs to be committed and rtnl lock couldn't be
> taken, avoid redisplay of same link status change message.
>
> Signed-off-by: Manish Kumar Singh <mk.singh@oracle.com>
> ---
> drivers/net/bonding/bond_main.c | 6 ++++--
> include/net/bonding.h | 1 +
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 217b790d22ed..fb4e3aff1677 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -2087,7 +2087,7 @@ static int bond_miimon_inspect(struct bonding *bond)
> bond_propose_link_state(slave, BOND_LINK_FAIL);
> commit++;
> slave->delay = bond->params.downdelay;
> - if (slave->delay) {
> + if (slave->delay && !bond->rtnl_needed) {
> netdev_info(bond->dev, "link status down for %sinterface %s, disabling it in %d ms\n",
> (BOND_MODE(bond) ==
> BOND_MODE_ACTIVEBACKUP) ?
> @@ -2127,7 +2127,7 @@ static int bond_miimon_inspect(struct bonding *bond)
> commit++;
> slave->delay = bond->params.updelay;
>
> - if (slave->delay) {
> + if (slave->delay && !bond->rtnl_needed) {
> netdev_info(bond->dev, "link status up for interface %s, enabling it in %d ms\n",
> slave->dev->name,
> ignore_updelay ? 0 :
> @@ -2301,9 +2301,11 @@ static void bond_mii_monitor(struct work_struct *work)
> if (!rtnl_trylock()) {
> delay = 1;
> should_notify_peers = false;
> + bond->rtnl_needed = true;
How can you set a shared variable with no synchronization ?
A bool is particularly dangerous here, at least on some arches.
> goto re_arm;
> }
>
> + bond->rtnl_needed = false;
> bond_for_each_slave(bond, slave, iter) {
> bond_commit_link_state(slave, BOND_SLAVE_NOTIFY_LATER);
> }
> diff --git a/include/net/bonding.h b/include/net/bonding.h
> index 808f1d167349..50d61cf77855 100644
> --- a/include/net/bonding.h
> +++ b/include/net/bonding.h
> @@ -234,6 +234,7 @@ struct bonding {
> struct dentry *debug_dir;
> #endif /* CONFIG_DEBUG_FS */
> struct rtnl_link_stats64 bond_stats;
> + bool rtnl_needed;
> };
>
> #define bond_slave_get_rcu(dev) \
>
^ permalink raw reply
* Re: [PATCH v2 2/4] dt-bindings: net: qcom: Add binding for shared mdio bus
From: Andrew Lunn @ 2018-09-17 14:50 UTC (permalink / raw)
To: Wang Dongsheng; +Cc: timur, davem, yu.zheng, netdev, devicetree
In-Reply-To: <1537174411-34510-3-git-send-email-dongsheng.wang@hxt-semitech.com>
On Mon, Sep 17, 2018 at 04:53:29PM +0800, Wang Dongsheng wrote:
> This property copy from "ibm,emac.txt" to describe a shared MIDO bus.
> Since emac include MDIO, so If the motherboard has more than one PHY
> connected to an MDIO bus, this property will point to the MAC device
> that has the MDIO bus.
>
> Signed-off-by: Wang Dongsheng <dongsheng.wang@hxt-semitech.com>
> ---
> V2: s/Since QDF2400 emac/Since emac/
> ---
> Documentation/devicetree/bindings/net/qcom-emac.txt | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/qcom-emac.txt b/Documentation/devicetree/bindings/net/qcom-emac.txt
> index 346e6c7f47b7..50db71771358 100644
> --- a/Documentation/devicetree/bindings/net/qcom-emac.txt
> +++ b/Documentation/devicetree/bindings/net/qcom-emac.txt
> @@ -24,6 +24,9 @@ Internal PHY node:
> The external phy child node:
> - reg : The phy address
>
> +Optional properties:
> +- mdio-device : Shared MIDO bus.
Hi Dongsheng
I don't see why you need this property. The ethernet interface has a
phy-handle which points to a PHY. That is all you need to find the PHY.
emac0: ethernet@feb20000 {
compatible = "qcom,fsm9900-emac";
reg = <0xfeb20000 0x10000>,
<0xfeb36000 0x1000>;
interrupts = <76>;
clocks = <&gcc 0>, <&gcc 1>, <&gcc 3>, <&gcc 4>, <&gcc 5>,
<&gcc 6>, <&gcc 7>;
clock-names = "axi_clk", "cfg_ahb_clk", "high_speed_clk",
"mdio_clk", "tx_clk", "rx_clk", "sys_clk";
internal-phy = <&emac_sgmii>;
phy-handle = <&phy0>;
#address-cells = <1>;
#size-cells = <0>;
phy0: ethernet-phy@0 {
reg = <0>;
};
phy1: ethernet-phy@1 {
reg = <1>;
};
pinctrl-names = "default";
pinctrl-0 = <&mdio_pins_a>;
};
emac1: ethernet@38900000 {
compatible = "qcom,fsm9900-emac";
...
...
phy-handle = <&phy1>;
};
Andrew
^ permalink raw reply
* Re: [PATCH net] pppoe: fix reception of frames with no mac header
From: David Miller @ 2018-09-17 14:51 UTC (permalink / raw)
To: g.nault; +Cc: netdev, mostrows, eric.dumazet
In-Reply-To: <274ac54fa02052104201d4738a6326a637e87a83.1536935190.git.g.nault@alphalink.fr>
From: Guillaume Nault <g.nault@alphalink.fr>
Date: Fri, 14 Sep 2018 16:28:05 +0200
> pppoe_rcv() needs to look back at the Ethernet header in order to
> lookup the PPPoE session. Therefore we need to ensure that the mac
> header is big enough to contain an Ethernet header. Otherwise
> eth_hdr(skb)->h_source might access invalid data.
...
> Fixes: 224cf5ad14c0 ("ppp: Move the PPP drivers")
> Reported-by: syzbot+f5f6080811c849739212@syzkaller.appspotmail.com
> Signed-off-by: Guillaume Nault <g.nault@alphalink.fr>
Applied and queued up for -stable, thanks.
^ permalink raw reply
* Re: [PATCH net] ipv6: fix possible use-after-free in ip6_xmit()
From: David Miller @ 2018-09-17 14:58 UTC (permalink / raw)
To: edumazet; +Cc: netdev, eric.dumazet
In-Reply-To: <20180914190232.184779-1-edumazet@google.com>
From: Eric Dumazet <edumazet@google.com>
Date: Fri, 14 Sep 2018 12:02:31 -0700
> In the unlikely case ip6_xmit() has to call skb_realloc_headroom(),
> we need to call skb_set_owner_w() before consuming original skb,
> otherwise we risk a use-after-free.
>
> Bring IPv6 in line with what we do in IPv4 to fix this.
>
> Fixes: 1da177e4c3f41 ("Linux-2.6.12-rc2")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: syzbot <syzkaller@googlegroups.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net] bnxt_en: Fix VF mac address regression.
From: David Miller @ 2018-09-17 14:58 UTC (permalink / raw)
To: michael.chan; +Cc: netdev, seth.forshee, loseweigh
In-Reply-To: <1536954089-6061-1-git-send-email-michael.chan@broadcom.com>
From: Michael Chan <michael.chan@broadcom.com>
Date: Fri, 14 Sep 2018 15:41:29 -0400
> The recent commit to always forward the VF MAC address to the PF for
> approval may not work if the PF driver or the firmware is older. This
> will cause the VF driver to fail during probe:
>
> bnxt_en 0000:00:03.0 (unnamed net_device) (uninitialized): hwrm req_type 0xf seq id 0x5 error 0xffff
> bnxt_en 0000:00:03.0 (unnamed net_device) (uninitialized): VF MAC address 00:00:17:02:05:d0 not approved by the PF
> bnxt_en 0000:00:03.0: Unable to initialize mac address.
> bnxt_en: probe of 0000:00:03.0 failed with error -99
>
> We fix it by treating the error as fatal only if the VF MAC address is
> locally generated by the VF.
>
> Fixes: 707e7e966026 ("bnxt_en: Always forward VF MAC address to the PF.")
> Reported-by: Seth Forshee <seth.forshee@canonical.com>
> Reported-by: Siwei Liu <loseweigh@gmail.com>
> Signed-off-by: Michael Chan <michael.chan@broadcom.com>
> ---
> Please queue this for stable as well. Thanks.
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH v2 0/2] hv_netvsc: associate VF and PV device by serial number
From: David Miller @ 2018-09-17 14:59 UTC (permalink / raw)
To: stephen; +Cc: kys, haiyangz, sthemmin, devel, netdev, linux-pci
In-Reply-To: <20180914195457.20433-1-sthemmin@microsoft.com>
From: Stephen Hemminger <stephen@networkplumber.org>
Date: Fri, 14 Sep 2018 12:54:55 -0700
> The Hyper-V implementation of PCI controller has concept of 32 bit serial number
> (not to be confused with PCI-E serial number). This value is sent in the protocol
> from the host to indicate SR-IOV VF device is attached to a synthetic NIC.
>
> Using the serial number (instead of MAC address) to associate the two devices
> avoids lots of potential problems when there are duplicate MAC addresses from
> tunnels or layered devices.
>
> The patch set is broken into two parts, one is for the PCI controller
> and the other is for the netvsc device. Normally, these go through different
> trees but sending them together here for better review. The PCI changes
> were submitted previously, but the main review comment was "why do you
> need this?". This is why.
>
> v2 - slot name can be shorter.
> remove locking when creating pci_slots; see comment for explaination
Series applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next v3 02/17] zinc: introduce minimal cryptography library
From: Jason A. Donenfeld @ 2018-09-17 14:59 UTC (permalink / raw)
To: Andy Lutomirski
Cc: David Miller, Herbert Xu, Andrew Lutomirski, Andrew Lunn,
Eric Biggers, Greg Kroah-Hartman, Ard Biesheuvel, LKML, Netdev,
Samuel Neves, Jean-Philippe Aumasson, Linux Crypto Mailing List
In-Reply-To: <40088431-D4EE-4D1C-A369-2CE15F758483@amacapital.net>
On Mon, Sep 17, 2018 at 4:56 PM Andy Lutomirski <luto@amacapital.net> wrote:
> Would you accept Ard’s (and/or Eric Biggers’, perhaps, as an alternative)? Or, if you really want Herbert’s review, can you ask him to review it? (Hi Herbert!)
Preferably Eric's.
^ permalink raw reply
* Re: [net-next PATCH] tls: async support causes out-of-bounds access in crypto APIs
From: David Miller @ 2018-09-17 15:02 UTC (permalink / raw)
To: john.fastabend
Cc: vakul.garg, davejwatson, doronrk, netdev, alexei.starovoitov,
daniel
In-Reply-To: <20180914200146.6302.50472.stgit@john-Precision-Tower-5810>
From: John Fastabend <john.fastabend@gmail.com>
Date: Fri, 14 Sep 2018 13:01:46 -0700
> When async support was added it needed to access the sk from the async
> callback to report errors up the stack. The patch tried to use space
> after the aead request struct by directly setting the reqsize field in
> aead_request. This is an internal field that should not be used
> outside the crypto APIs. It is used by the crypto code to define extra
> space for private structures used in the crypto context. Users of the
> API then use crypto_aead_reqsize() and add the returned amount of
> bytes to the end of the request memory allocation before posting the
> request to encrypt/decrypt APIs.
>
> So this breaks (with general protection fault and KASAN error, if
> enabled) because the request sent to decrypt is shorter than required
> causing the crypto API out-of-bounds errors. Also it seems unlikely the
> sk is even valid by the time it gets to the callback because of memset
> in crypto layer.
>
> Anyways, fix this by holding the sk in the skb->sk field when the
> callback is set up and because the skb is already passed through to
> the callback handler via void* we can access it in the handler. Then
> in the handler we need to be careful to NULL the pointer again before
> kfree_skb. I added comments on both the setup (in tls_do_decryption)
> and when we clear it from the crypto callback handler
> tls_decrypt_done(). After this selftests pass again and fixes KASAN
> errors/warnings.
>
> Fixes: 94524d8fc965 ("net/tls: Add support for async decryption of tls records")
> Signed-off-by: John Fastabend <john.fastabend@gmail.com>
Applied, thanks John.
^ permalink raw reply
* Re: [Patch net-next] ipv4: initialize ra_mutex in inet_init_net()
From: David Miller @ 2018-09-17 15:02 UTC (permalink / raw)
To: xiyou.wangcong; +Cc: netdev, ktkhai
In-Reply-To: <20180914203242.2712-1-xiyou.wangcong@gmail.com>
From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Fri, 14 Sep 2018 13:32:42 -0700
> ra_mutex is a IPv4 specific mutex, it is inside struct netns_ipv4,
> but its initialization is in the generic netns code, setup_net().
>
> Move it to IPv4 specific net init code, inet_init_net().
>
> Fixes: d9ff3049739e ("net: Replace ip_ra_lock with per-net mutex")
> Cc: Kirill Tkhai <ktkhai@virtuozzo.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>
Please take into consideration Kirill's feedback.
Thank you.
^ permalink raw reply
* Re: [PATCH v2 net] net/ipv4: defensive cipso option parsing
From: Paul Moore @ 2018-09-17 20:31 UTC (permalink / raw)
To: snu; +Cc: netdev, aams, dwmw, yujuan.qi, sveith, stable
In-Reply-To: <20180917174653.17046-1-snu@amazon.com>
On Mon, Sep 17, 2018 at 1:49 PM Stefan Nuernberger <snu@amazon.com> wrote:
> commit 40413955ee26 ("Cipso: cipso_v4_optptr enter infinite loop") fixed
> a possible infinite loop in the IP option parsing of CIPSO. The fix
> assumes that ip_options_compile filtered out all zero length options and
> that no other one-byte options beside IPOPT_END and IPOPT_NOOP exist.
> While this assumption currently holds true, add explicit checks for zero
> length and invalid length options to be safe for the future. Even though
> ip_options_compile should have validated the options, the introduction of
> new one-byte options can still confuse this code without the additional
> checks.
>
> Signed-off-by: Stefan Nuernberger <snu@amazon.com>
> Cc: David Woodhouse <dwmw@amazon.co.uk>
> Cc: Simon Veith <sveith@amazon.de>
> Cc: stable@vger.kernel.org
> ---
> net/ipv4/cipso_ipv4.c | 11 +++++++----
> 1 file changed, 7 insertions(+), 4 deletions(-)
See my previous comments about the necessity of this patch, but beyond
that it looks fine to me.
Acked-by: Paul Moore <paul@paul-moore.com>
> diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
> index 82178cc69c96..777fa3b7fb13 100644
> --- a/net/ipv4/cipso_ipv4.c
> +++ b/net/ipv4/cipso_ipv4.c
> @@ -1512,7 +1512,7 @@ static int cipso_v4_parsetag_loc(const struct cipso_v4_doi *doi_def,
> *
> * Description:
> * Parse the packet's IP header looking for a CIPSO option. Returns a pointer
> - * to the start of the CIPSO option on success, NULL if one if not found.
> + * to the start of the CIPSO option on success, NULL if one is not found.
> *
> */
> unsigned char *cipso_v4_optptr(const struct sk_buff *skb)
> @@ -1522,10 +1522,8 @@ unsigned char *cipso_v4_optptr(const struct sk_buff *skb)
> int optlen;
> int taglen;
>
> - for (optlen = iph->ihl*4 - sizeof(struct iphdr); optlen > 0; ) {
> + for (optlen = iph->ihl*4 - sizeof(struct iphdr); optlen > 1; ) {
Not worth re-spinning this patch, but looking at this a bit closer, we
could probably optimize the "optlen > 1" tweak a bit further by using
CIPSO_V4_HDR_LEN instead of "1" since we only care about CIPSO headers
here.
Although given the nature of IPv4 options, I'm not sure this would
ever really have an impact, let alone a noticeable impact.
> switch (optptr[0]) {
> - case IPOPT_CIPSO:
> - return optptr;
> case IPOPT_END:
> return NULL;
> case IPOPT_NOOP:
> @@ -1534,6 +1532,11 @@ unsigned char *cipso_v4_optptr(const struct sk_buff *skb)
> default:
> taglen = optptr[1];
> }
> + if (!taglen || taglen > optlen)
> + return NULL;
> + if (optptr[0] == IPOPT_CIPSO)
> + return optptr;
> +
> optlen -= taglen;
> optptr += taglen;
> }
> --
> 2.19.0
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH net] tls: fix currently broken MSG_PEEK behavior
From: David Miller @ 2018-09-17 15:04 UTC (permalink / raw)
To: daniel; +Cc: davejwatson, doronrk, alexei.starovoitov, john.fastabend, netdev
In-Reply-To: <20180914210055.2558-1-daniel@iogearbox.net>
From: Daniel Borkmann <daniel@iogearbox.net>
Date: Fri, 14 Sep 2018 23:00:55 +0200
> In kTLS MSG_PEEK behavior is currently failing, strace example:
...
> As can be seen from strace, there are two TLS records sent,
> i) 'test_read_peek' and ii) '_mult_recs\0' where we end up
> peeking 'test_read_peektest_read_peektest'. This is clearly
> wrong, and what happens is that given peek cannot call into
> tls_sw_advance_skb() to unpause strparser and proceed with
> the next skb, we end up looping over the current one, copying
> the 'test_read_peek' over and over into the user provided
> buffer.
>
> Here, we can only peek into the currently held skb (current,
> full TLS record) as otherwise we would end up having to hold
> all the original skb(s) (depending on the peek depth) in a
> separate queue when unpausing strparser to process next
> records, minimally intrusive is to return only up to the
> current record's size (which likely was what c46234ebb4d1
> ("tls: RX path for ktls") originally intended as well). Thus,
> after patch we properly peek the first record:
...
> Fixes: c46234ebb4d1 ("tls: RX path for ktls")
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net] net: dsa: mv88e6xxx: Fix ATU Miss Violation
From: David Miller @ 2018-09-17 15:05 UTC (permalink / raw)
To: andrew; +Cc: netdev, f.fainelli
In-Reply-To: <1536961572-30935-1-git-send-email-andrew@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Fri, 14 Sep 2018 23:46:12 +0200
> Fix a cut/paste error and a typo which results in ATU miss violations
> not being reported.
>
> Fixes: 0977644c5005 ("net: dsa: mv88e6xxx: Decode ATU problem interrupt")
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net-next] net: dsa: gswip: Fix return value check in gswip_probe()
From: David Miller @ 2018-09-17 15:07 UTC (permalink / raw)
To: weiyongjun1
Cc: andrew, vivien.didelot, f.fainelli, hauke, netdev,
kernel-janitors
In-Reply-To: <1536975201-184967-1-git-send-email-weiyongjun1@huawei.com>
From: Wei Yongjun <weiyongjun1@huawei.com>
Date: Sat, 15 Sep 2018 01:33:21 +0000
> In case of error, the function devm_ioremap_resource() returns ERR_PTR()
> and never returns NULL. The NULL test in the return value check should
> be replaced with IS_ERR().
>
> Fixes: 14fceff4771e ("net: dsa: Add Lantiq / Intel DSA driver for vrx200")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: lantiq: Fix return value check in xrx200_probe()
From: David Miller @ 2018-09-17 15:07 UTC (permalink / raw)
To: weiyongjun1; +Cc: hauke, netdev, kernel-janitors
In-Reply-To: <1536975230-185101-1-git-send-email-weiyongjun1@huawei.com>
From: Wei Yongjun <weiyongjun1@huawei.com>
Date: Sat, 15 Sep 2018 01:33:50 +0000
> In case of error, the function devm_ioremap_resource() returns ERR_PTR()
> and never returns NULL. The NULL test in the return value check should
> be replaced with IS_ERR().
>
> Fixes: fe1a56420cf2 ("net: lantiq: Add Lantiq / Intel VRX200 Ethernet driver")
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: hns: make function hns_gmac_wait_fifo_clean() static
From: David Miller @ 2018-09-17 15:07 UTC (permalink / raw)
To: weiyongjun1
Cc: yisen.zhuang, salil.mehta, lipeng321, matthias.bgg, netdev,
kernel-janitors
In-Reply-To: <1536975729-187303-1-git-send-email-weiyongjun1@huawei.com>
From: Wei Yongjun <weiyongjun1@huawei.com>
Date: Sat, 15 Sep 2018 01:42:09 +0000
> Fixes the following sparse warning:
>
> drivers/net/ethernet/hisilicon/hns/hns_dsaf_gmac.c:322:5: warning:
> symbol 'hns_gmac_wait_fifo_clean' was not declared. Should it be static?
>
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
Applied.
^ permalink raw reply
* Re: [PATCH] net: phy: phylink: fix SFP interface autodetection
From: Russell King - ARM Linux @ 2018-09-17 15:12 UTC (permalink / raw)
To: Baruch Siach
Cc: Andrew Lunn, Florian Fainelli, netdev, Antoine Tenart,
Gregory CLEMENT
In-Reply-To: <18cf7eb4b6253f922d3dc6cb70eb7d050c32c35e.1537193997.git.baruch@tkos.co.il>
On Mon, Sep 17, 2018 at 05:19:57PM +0300, Baruch Siach wrote:
> When the switching to the SFP detected link mode update the main
> link_interface field as well. Otherwise, the link fails to come up when
> the configured 'phy-mode' defers from the SFP detected mode.
>
> This fixes 1GB SFP module link up on eth3 of the Macchiatobin board that
> is configured in the DT to "2500base-x" phy-mode.
link_interface isn't supposed to track the SFP link mode. In any case,
this is only used when a PHY is attached. For a PHY on a SFP,
phylink_connect_phy() should be using link_config.interface and not
link_interface there.
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up
According to speedtest.net: 13Mbps down 490kbps up
^ permalink raw reply
* Re: [PATCH v3 net-next 07/12] net: ethernet: Add helper to remove a supported link mode
From: Simon Horman @ 2018-09-17 15:13 UTC (permalink / raw)
To: Andrew Lunn; +Cc: David Miller, netdev, Florian Fainelli
In-Reply-To: <1536709999-13420-8-git-send-email-andrew@lunn.ch>
On Wed, Sep 12, 2018 at 01:53:14AM +0200, Andrew Lunn wrote:
> Some MAC hardware cannot support a subset of link modes. e.g. often
> 1Gbps Full duplex is supported, but Half duplex is not. Add a helper
> to remove such a link mode.
>
> Signed-off-by: Andrew Lunn <andrew@lunn.ch>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
> drivers/net/ethernet/apm/xgene/xgene_enet_hw.c | 6 +++---
> drivers/net/ethernet/cadence/macb_main.c | 5 ++---
> drivers/net/ethernet/freescale/fec_main.c | 3 ++-
> drivers/net/ethernet/microchip/lan743x_main.c | 2 +-
> drivers/net/ethernet/renesas/ravb_main.c | 3 ++-
> .../net/ethernet/stmicro/stmmac/stmmac_main.c | 12 ++++++++----
> drivers/net/phy/phy_device.c | 18 ++++++++++++++++++
> drivers/net/usb/lan78xx.c | 2 +-
> include/linux/phy.h | 1 +
> 9 files changed, 38 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c b/drivers/net/ethernet/apm/xgene/xgene_enet_hw.c
> index 078a04dc1182..4831f9de5945 100644
...
> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
> index aff5516b781e..fb2a1125780d 100644
> --- a/drivers/net/ethernet/renesas/ravb_main.c
> +++ b/drivers/net/ethernet/renesas/ravb_main.c
> @@ -1074,7 +1074,8 @@ static int ravb_phy_init(struct net_device *ndev)
> }
>
> /* 10BASE is not supported */
> - phydev->supported &= ~PHY_10BT_FEATURES;
> + phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Half_BIT);
> + phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_10baseT_Full_BIT);
>
> phy_attached_info(phydev);
>
...
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index db1172db1e7c..e9ca83a438b0 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1765,6 +1765,24 @@ int phy_set_max_speed(struct phy_device *phydev, u32 max_speed)
> }
> EXPORT_SYMBOL(phy_set_max_speed);
>
> +/**
> + * phy_remove_link_mode - Remove a supported link mode
> + * @phydev: phy_device structure to remove link mode from
> + * @link_mode: Link mode to be removed
> + *
> + * Description: Some MACs don't support all link modes which the PHY
> + * does. e.g. a 1G MAC often does not support 1000Half. Add a helper
> + * to remove a link mode.
> + */
> +void phy_remove_link_mode(struct phy_device *phydev, u32 link_mode)
> +{
> + WARN_ON(link_mode > 31);
> +
> + phydev->supported &= ~BIT(link_mode);
> + phydev->advertising = phydev->supported;
> +}
> +EXPORT_SYMBOL(phy_remove_link_mode);
> +
> static void of_set_phy_supported(struct phy_device *phydev)
> {
> struct device_node *node = phydev->mdio.dev.of_node;
Hi Andrew,
I believe that for the RAVB the overall effect of this change is that
10-BaseT modes are no longer advertised (although both with and without
this patch they are not supported).
Unfortunately on R-Car Gen3 M3-W (r8a7796) based Salvator-X board
I have observed that this results in the link no longer being negotiated
on one switch (the one I usually use) while it seemed fine on another.
^ permalink raw reply
* Re: [PATCH net-next 0/5] net: lantiq: Minor fixes for vrx200 and gswip
From: David Miller @ 2018-09-17 15:13 UTC (permalink / raw)
To: hauke
Cc: netdev, andrew, vivien.didelot, f.fainelli, john, linux-mips, dev,
hauke.mehrtens, devicetree
In-Reply-To: <20180915120849.24630-1-hauke@hauke-m.de>
From: Hauke Mehrtens <hauke@hauke-m.de>
Date: Sat, 15 Sep 2018 14:08:44 +0200
> These are mostly minor fixes to problems addresses in the latests round
> of the review of the original series adding these driver, which were not
> applied before the patches got merged into net-next.
> In addition it fixes a data bus error on poweroff.
Series applied.
^ permalink raw reply
* Re: [PATCH v2 net] net: aquantia: memory corruption on jumbo frames
From: David Miller @ 2018-09-17 15:15 UTC (permalink / raw)
To: igor.russkikh; +Cc: nikita.danilov, netdev, f.gerold
In-Reply-To: <67762c5cd115c74d743ba184c97def9a4734eebd.1537021802.git.igor.russkikh@aquantia.com>
From: Igor Russkikh <igor.russkikh@aquantia.com>
Date: Sat, 15 Sep 2018 18:03:39 +0300
> From: Friedemann Gerold <f.gerold@b-c-s.de>
>
> This patch fixes skb_shared area, which will be corrupted
> upon reception of 4K jumbo packets.
>
> Originally build_skb usage purpose was to reuse page for skb to eliminate
> needs of extra fragments. But that logic does not take into account that
> skb_shared_info should be reserved at the end of skb data area.
>
> In case packet data consumes all the page (4K), skb_shinfo location
> overflows the page. As a consequence, __build_skb zeroed shinfo data above
> the allocated page, corrupting next page.
>
> The issue is rarely seen in real life because jumbo are normally larger
> than 4K and that causes another code path to trigger.
> But it 100% reproducible with simple scapy packet, like:
>
> sendp(IP(dst="192.168.100.3") / TCP(dport=443) \
> / Raw(RandString(size=(4096-40))), iface="enp1s0")
>
> Fixes: 018423e90bee ("net: ethernet: aquantia: Add ring support code")
>
> Reported-by: Friedemann Gerold <f.gerold@b-c-s.de>
> Reported-by: Michael Rauch <michael@rauch.be>
> Signed-off-by: Friedemann Gerold <f.gerold@b-c-s.de>
> Tested-by: Nikita Danilov <nikita.danilov@aquantia.com>
> Signed-off-by: Igor Russkikh <igor.russkikh@aquantia.com>
APplied and queued up for -stable.
^ permalink raw reply
* [PATCH net-next] ravb: do not write 1 to reserved bits
From: Simon Horman @ 2018-09-17 15:19 UTC (permalink / raw)
To: David Miller, Sergei Shtylyov
Cc: Magnus Damm, netdev, linux-renesas-soc, Kazuya Mizuguchi,
Simon Horman
From: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
EtherAVB hardware requires 0 to be written to status register bits in
order to clear them, however, care must be taken not to:
1. Clear other bits, by writing zero to them
2. Write one to reserved bits
This patch corrects the ravb driver with respect to the second point above.
This is done by defining reserved bit masks for the affected registers and,
after auditing the code, ensure all sites that may write a one to a
reserved bit use are suitably masked.
Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com>
Signed-off-by: Simon Horman <horms+renesas@verge.net.au>
---
v2 [Simon Horman]
* Cover ravb_timestamp_interrupt() by this change
* Use enum value rather than #define for reserved masks
* Reword changelog
v1 [Kazuya Mizuguchi]
---
drivers/net/ethernet/renesas/ravb.h | 6 ++++++
drivers/net/ethernet/renesas/ravb_main.c | 11 ++++++-----
drivers/net/ethernet/renesas/ravb_ptp.c | 2 +-
3 files changed, 13 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/renesas/ravb.h b/drivers/net/ethernet/renesas/ravb.h
index 1470fc12282b..bca219edcf94 100644
--- a/drivers/net/ethernet/renesas/ravb.h
+++ b/drivers/net/ethernet/renesas/ravb.h
@@ -428,6 +428,7 @@ enum EIS_BIT {
EIS_CULF1 = 0x00000080,
EIS_TFFF = 0x00000100,
EIS_QFS = 0x00010000,
+ EIS_RESERVED = (u32)(GENMASK(31, 17) | GENMASK(15, 11)),
};
/* RIC0 */
@@ -472,6 +473,7 @@ enum RIS0_BIT {
RIS0_FRF15 = 0x00008000,
RIS0_FRF16 = 0x00010000,
RIS0_FRF17 = 0x00020000,
+ RIS0_RESERVED = (u32)GENMASK(31, 18),
};
/* RIC1 */
@@ -528,6 +530,7 @@ enum RIS2_BIT {
RIS2_QFF16 = 0x00010000,
RIS2_QFF17 = 0x00020000,
RIS2_RFFF = 0x80000000,
+ RIS2_RESERVED = (u32)GENMASK_ULL(30, 18),
};
/* TIC */
@@ -544,6 +547,8 @@ enum TIS_BIT {
TIS_FTF1 = 0x00000002, /* Undocumented? */
TIS_TFUF = 0x00000100,
TIS_TFWF = 0x00000200,
+ TIS_RESERVED = (u32)(GENMASK_ULL(31, 20) | GENMASK_ULL(15, 12) | \
+ GENMASK_ULL(7, 4))
};
/* ISS */
@@ -617,6 +622,7 @@ enum GIC_BIT {
enum GIS_BIT {
GIS_PTCF = 0x00000001, /* Undocumented? */
GIS_PTMF = 0x00000004,
+ GIS_RESERVED = (u32)GENMASK(15, 10),
};
/* GIE (R-Car Gen3 only) */
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
index fb2a1125780d..cddb0c2856c8 100644
--- a/drivers/net/ethernet/renesas/ravb_main.c
+++ b/drivers/net/ethernet/renesas/ravb_main.c
@@ -739,10 +739,11 @@ static void ravb_error_interrupt(struct net_device *ndev)
u32 eis, ris2;
eis = ravb_read(ndev, EIS);
- ravb_write(ndev, ~EIS_QFS, EIS);
+ ravb_write(ndev, ~(EIS_QFS | EIS_RESERVED), EIS);
if (eis & EIS_QFS) {
ris2 = ravb_read(ndev, RIS2);
- ravb_write(ndev, ~(RIS2_QFF0 | RIS2_RFFF), RIS2);
+ ravb_write(ndev, ~(RIS2_QFF0 | RIS2_RFFF | RIS2_RESERVED),
+ RIS2);
/* Receive Descriptor Empty int */
if (ris2 & RIS2_QFF0)
@@ -795,7 +796,7 @@ static bool ravb_timestamp_interrupt(struct net_device *ndev)
u32 tis = ravb_read(ndev, TIS);
if (tis & TIS_TFUF) {
- ravb_write(ndev, ~TIS_TFUF, TIS);
+ ravb_write(ndev, ~(TIS_TFUF | TIS_RESERVED), TIS);
ravb_get_tx_tstamp(ndev);
return true;
}
@@ -930,7 +931,7 @@ static int ravb_poll(struct napi_struct *napi, int budget)
/* Processing RX Descriptor Ring */
if (ris0 & mask) {
/* Clear RX interrupt */
- ravb_write(ndev, ~mask, RIS0);
+ ravb_write(ndev, ~(mask | RIS0_RESERVED), RIS0);
if (ravb_rx(ndev, "a, q))
goto out;
}
@@ -938,7 +939,7 @@ static int ravb_poll(struct napi_struct *napi, int budget)
if (tis & mask) {
spin_lock_irqsave(&priv->lock, flags);
/* Clear TX interrupt */
- ravb_write(ndev, ~mask, TIS);
+ ravb_write(ndev, ~(mask | TIS_RESERVED), TIS);
ravb_tx_free(ndev, q, true);
netif_wake_subqueue(ndev, q);
mmiowb();
diff --git a/drivers/net/ethernet/renesas/ravb_ptp.c b/drivers/net/ethernet/renesas/ravb_ptp.c
index 0721b5c35d91..dce2a40a31e3 100644
--- a/drivers/net/ethernet/renesas/ravb_ptp.c
+++ b/drivers/net/ethernet/renesas/ravb_ptp.c
@@ -315,7 +315,7 @@ void ravb_ptp_interrupt(struct net_device *ndev)
}
}
- ravb_write(ndev, ~gis, GIS);
+ ravb_write(ndev, ~(gis | GIS_RESERVED), GIS);
}
void ravb_ptp_init(struct net_device *ndev, struct platform_device *pdev)
--
2.11.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox