* [PATCH net-next 3/6] net: dsa: Delete the VID from the upstream port as well
From: Vladimir Oltean @ 2019-08-19 23:59 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, idosch, roopa, nikolay, davem
Cc: netdev, Vladimir Oltean
In-Reply-To: <20190820000002.9776-1-olteanv@gmail.com>
Commit b2f81d304cee ("net: dsa: add CPU and DSA ports as VLAN members")
is littering a lot. After deleting a VLAN added on a DSA port, it still
remains installed in the hardware filter of the upstream port. Fix this.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
net/dsa/switch.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 09d9286b27cc..84ab2336131e 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -295,11 +295,20 @@ static int dsa_switch_vlan_del(struct dsa_switch *ds,
struct dsa_notifier_vlan_info *info)
{
const struct switchdev_obj_port_vlan *vlan = info->vlan;
+ int port;
if (!ds->ops->port_vlan_del)
return -EOPNOTSUPP;
+ /* Build a mask of VLAN members */
+ bitmap_zero(ds->bitmap, ds->num_ports);
if (ds->index == info->sw_index)
+ set_bit(info->port, ds->bitmap);
+ for (port = 0; port < ds->num_ports; port++)
+ if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port))
+ set_bit(port, ds->bitmap);
+
+ for_each_set_bit(port, ds->bitmap, ds->num_ports)
return ds->ops->port_vlan_del(ds, info->port, vlan);
return 0;
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 1/6] net: dsa: tag_8021q: Future-proof the reserved fields in the custom VID
From: Vladimir Oltean @ 2019-08-19 23:59 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, idosch, roopa, nikolay, davem
Cc: netdev, Vladimir Oltean
In-Reply-To: <20190820000002.9776-1-olteanv@gmail.com>
After witnessing the discussion in https://lkml.org/lkml/2019/8/14/151
w.r.t. ioctl extensibility, it became clear that such an issue might
prevent that the 3 RSV bits inside the DSA 802.1Q tag might also suffer
the same fate and be useless for further extension.
So clearly specify that the reserved bits should currently be
transmitted as zero and ignored on receive. The DSA tagger already does
this (and has always did), and is the only known user so far (no
Wireshark dissection plugin, etc). So there should be no incompatibility
to speak of.
Fixes: 0471dd429cea ("net: dsa: tag_8021q: Create a stable binary format")
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
net/dsa/tag_8021q.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index 6ebbd799c4eb..67a1bc635a7b 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -28,6 +28,7 @@
*
* RSV - VID[9]:
* To be used for further expansion of SWITCH_ID or for other purposes.
+ * Must be transmitted as zero and ignored on receive.
*
* SWITCH_ID - VID[8:6]:
* Index of switch within DSA tree. Must be between 0 and
@@ -35,6 +36,7 @@
*
* RSV - VID[5:4]:
* To be used for further expansion of PORT or for other purposes.
+ * Must be transmitted as zero and ignored on receive.
*
* PORT - VID[3:0]:
* Index of switch port. Must be between 0 and DSA_MAX_PORTS - 1.
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 2/6] net: bridge: Populate the pvid flag in br_vlan_get_info
From: Vladimir Oltean @ 2019-08-19 23:59 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, idosch, roopa, nikolay, davem
Cc: netdev, Vladimir Oltean
In-Reply-To: <20190820000002.9776-1-olteanv@gmail.com>
Currently this simplified code snippet fails:
br_vlan_get_pvid(netdev, &pvid);
br_vlan_get_info(netdev, pvid, &vinfo);
ASSERT(!(vinfo.flags & BRIDGE_VLAN_INFO_PVID));
It is intuitive that the pvid of a netdevice should have the
BRIDGE_VLAN_INFO_PVID flag set.
However I can't seem to pinpoint a commit where this behavior was
introduced. It seems like it's been like that since forever.
At a first glance it would make more sense to just handle the
BRIDGE_VLAN_INFO_PVID flag in __vlan_add_flags. However, as Nikolay
explains:
There are a few reasons why we don't do it, most importantly because
we need to have only one visible pvid at any single time, even if it's
stale - it must be just one. Right now that rule will not be violated
by this change, but people will try using this flag and could see two
pvids simultaneously. You can see that the pvid code is even using
memory barriers to propagate the new value faster and everywhere the
pvid is read only once. That is the reason the flag is set
dynamically when dumping entries, too. A second (weaker) argument
against would be given the above we don't want another way to do the
same thing, specifically if it can provide us with two pvids (e.g. if
walking the vlan list) or if it can provide us with a pvid different
from the one set in the vg. [Obviously, I'm talking about RCU
pvid/vlan use cases similar to the dumps. The locked cases are fine.
I would like to avoid explaining why this shouldn't be relied upon
without locking]
So instead of introducing the above change and making sure of the pvid
uniqueness under RCU, simply dynamically populate the pvid flag in
br_vlan_get_info().
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
net/bridge/br_vlan.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
index f5b2aeebbfe9..bb98984cd27d 100644
--- a/net/bridge/br_vlan.c
+++ b/net/bridge/br_vlan.c
@@ -1281,6 +1281,8 @@ int br_vlan_get_info(const struct net_device *dev, u16 vid,
p_vinfo->vid = vid;
p_vinfo->flags = v->flags;
+ if (vid == br_get_pvid(vg))
+ p_vinfo->flags |= BRIDGE_VLAN_INFO_PVID;
return 0;
}
EXPORT_SYMBOL_GPL(br_vlan_get_info);
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 0/6] Dynamic toggling of vlan_filtering for SJA1105 DSA
From: Vladimir Oltean @ 2019-08-19 23:59 UTC (permalink / raw)
To: f.fainelli, vivien.didelot, andrew, idosch, roopa, nikolay, davem
Cc: netdev, Vladimir Oltean
This patchset addresses a few limitations in DSA and the bridge core
that made it impossible for this sequence of commands to work:
ip link add name br0 type bridge
ip link set dev swp2 master br0
echo 1 > /sys/class/net/br0/bridge/vlan_filtering
Only this sequence was previously working:
ip link add name br0 type bridge vlan_filtering 1
ip link set dev swp2 master br0
On SJA1105, the situation is further complicated by the fact that
toggling vlan_filtering is causing a switch reset. However, the hardware
state restoration logic is already there in the driver. It is a matter
of the layers above which need a few fixups.
Also see this discussion thread:
https://www.spinics.net/lists/netdev/msg581042.html
Patch 1/6 is not functionally related but also related to dsa_8021q
handling of VLANs and this is a good opportunity to bring up the subject
for discussion.
Vladimir Oltean (6):
net: dsa: tag_8021q: Future-proof the reserved fields in the custom
VID
net: bridge: Populate the pvid flag in br_vlan_get_info
net: dsa: Delete the VID from the upstream port as well
net: dsa: Don't program the VLAN as pvid on the upstream port
net: dsa: Allow proper internal use of VLANs
net: dsa: tag_8021q: Restore bridge pvid when enabling vlan_filtering
net/bridge/br_vlan.c | 2 ++
net/dsa/port.c | 10 ++--------
net/dsa/slave.c | 8 ++++++++
net/dsa/switch.c | 25 +++++++++++++++++++++----
net/dsa/tag_8021q.c | 34 +++++++++++++++++++++++++++++++++-
5 files changed, 66 insertions(+), 13 deletions(-)
--
2.17.1
^ permalink raw reply
* Re: [net-next v2 04/14] ice: fix set pause param autoneg check
From: David Miller @ 2019-08-19 23:59 UTC (permalink / raw)
To: jakub.kicinski
Cc: jeffrey.t.kirsher, paul.greenwalt, netdev, nhorman, sassmann,
andrewx.bowers
In-Reply-To: <20190819161142.6f4cc14d@cakuba.netronome.com>
From: Jakub Kicinski <jakub.kicinski@netronome.com>
Date: Mon, 19 Aug 2019 16:11:42 -0700
> On Mon, 19 Aug 2019 09:16:58 -0700, Jeff Kirsher wrote:
>> + pcaps = devm_kzalloc(&vsi->back->pdev->dev, sizeof(*pcaps),
>> + GFP_KERNEL);
>> + if (!pcaps)
>> + return -ENOMEM;
>> +
>> + /* Get current PHY config */
>> + status = ice_aq_get_phy_caps(pi, false, ICE_AQC_REPORT_SW_CFG, pcaps,
>> + NULL);
>> + if (status) {
>> + devm_kfree(&vsi->back->pdev->dev, pcaps);
>> + return -EIO;
>> + }
>> +
>> + is_an = ((pcaps->caps & ICE_AQC_PHY_AN_MODE) ?
>> + AUTONEG_ENABLE : AUTONEG_DISABLE);
>> +
>> + devm_kfree(&vsi->back->pdev->dev, pcaps);
>
> Is it just me or is this use of devm_k*alloc absolutely pointless?
Yeah it looks like an overzealous use of these interfaces to me too.
^ permalink raw reply
* Re: [PATCH v5 10/17] net: sgi: ioc3-eth: rework skb rx handling
From: Jakub Kicinski @ 2019-08-19 23:55 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Jonathan Corbet, Ralf Baechle, Paul Burton, James Hogan,
Dmitry Torokhov, Lee Jones, David S. Miller, Srinivas Kandagatla,
Alessandro Zummo, Alexandre Belloni, Greg Kroah-Hartman,
Jiri Slaby, Evgeniy Polyakov, linux-doc, linux-kernel, linux-mips,
linux-input, netdev, linux-rtc, linux-serial
In-Reply-To: <20190819163144.3478-11-tbogendoerfer@suse.de>
On Mon, 19 Aug 2019 18:31:33 +0200, Thomas Bogendoerfer wrote:
> Buffers alloacted by alloc_skb() are already cache aligned so there
> is no need for an extra align done by ioc3_alloc_skb. And instead
> of skb_put/skb_trim simply use one skb_put after frame size is known
> during receive.
>
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> ---
> drivers/net/ethernet/sgi/ioc3-eth.c | 50 ++++++++-----------------------------
> 1 file changed, 11 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
> index c875640926d6..d862f28887f9 100644
> --- a/drivers/net/ethernet/sgi/ioc3-eth.c
> +++ b/drivers/net/ethernet/sgi/ioc3-eth.c
> @@ -11,7 +11,6 @@
> *
> * To do:
> *
> - * o Handle allocation failures in ioc3_alloc_skb() more gracefully.
> * o Handle allocation failures in ioc3_init_rings().
> * o Use prefetching for large packets. What is a good lower limit for
> * prefetching?
> @@ -72,6 +71,12 @@
> #define TX_RING_ENTRIES 128
> #define TX_RING_MASK (TX_RING_ENTRIES - 1)
>
> +/* BEWARE: The IOC3 documentation documents the size of rx buffers as
> + * 1644 while it's actually 1664. This one was nasty to track down...
> + */
> +#define RX_OFFSET 10
> +#define RX_BUF_SIZE 1664
> +
> #define ETCSR_FD ((17 << ETCSR_IPGR2_SHIFT) | (11 << ETCSR_IPGR1_SHIFT) | 21)
> #define ETCSR_HD ((21 << ETCSR_IPGR2_SHIFT) | (21 << ETCSR_IPGR1_SHIFT) | 21)
>
> @@ -111,31 +116,6 @@ static void ioc3_init(struct net_device *dev);
> static const char ioc3_str[] = "IOC3 Ethernet";
> static const struct ethtool_ops ioc3_ethtool_ops;
>
> -/* We use this to acquire receive skb's that we can DMA directly into. */
> -
> -#define IOC3_CACHELINE 128UL
Is the cache line on the platform this driver works on 128B?
This looks like a DMA engine alignment requirement, more than an
optimization.
The comment in __alloc_skb() says:
/* We do our best to align skb_shared_info on a separate cache
* line. It usually works because kmalloc(X > SMP_CACHE_BYTES) gives
* aligned memory blocks, unless SLUB/SLAB debug is enabled.
* Both skb->head and skb_shared_info are cache line aligned.
*/
note the "unless".
> -static inline unsigned long aligned_rx_skb_addr(unsigned long addr)
> -{
> - return (~addr + 1) & (IOC3_CACHELINE - 1UL);
> -}
> -
> -static inline struct sk_buff *ioc3_alloc_skb(unsigned long length,
> - unsigned int gfp_mask)
> -{
> - struct sk_buff *skb;
> -
> - skb = alloc_skb(length + IOC3_CACHELINE - 1, gfp_mask);
> - if (likely(skb)) {
> - int offset = aligned_rx_skb_addr((unsigned long)skb->data);
> -
> - if (offset)
> - skb_reserve(skb, offset);
> - }
> -
> - return skb;
> -}
> -
> static inline unsigned long ioc3_map(void *ptr, unsigned long vdev)
> {
> #ifdef CONFIG_SGI_IP27
> @@ -148,12 +128,6 @@ static inline unsigned long ioc3_map(void *ptr, unsigned long vdev)
> #endif
> }
>
> -/* BEWARE: The IOC3 documentation documents the size of rx buffers as
> - * 1644 while it's actually 1664. This one was nasty to track down ...
> - */
> -#define RX_OFFSET 10
> -#define RX_BUF_ALLOC_SIZE (1664 + RX_OFFSET + IOC3_CACHELINE)
> -
> #define IOC3_SIZE 0x100000
>
> static inline u32 mcr_pack(u32 pulse, u32 sample)
> @@ -534,10 +508,10 @@ static inline void ioc3_rx(struct net_device *dev)
> err = be32_to_cpu(rxb->err); /* It's valid ... */
> if (err & ERXBUF_GOODPKT) {
> len = ((w0 >> ERXBUF_BYTECNT_SHIFT) & 0x7ff) - 4;
> - skb_trim(skb, len);
> + skb_put(skb, len);
> skb->protocol = eth_type_trans(skb, dev);
>
> - new_skb = ioc3_alloc_skb(RX_BUF_ALLOC_SIZE, GFP_ATOMIC);
> + new_skb = alloc_skb(RX_BUF_SIZE, GFP_ATOMIC);
> if (!new_skb) {
> /* Ouch, drop packet and just recycle packet
> * to keep the ring filled.
> @@ -546,6 +520,7 @@ static inline void ioc3_rx(struct net_device *dev)
> new_skb = skb;
> goto next;
> }
> + new_skb->dev = dev;
Assigning dev pointer seems unrelated to the rest of the patch?
> if (likely(dev->features & NETIF_F_RXCSUM))
> ioc3_tcpudp_checksum(skb,
> @@ -556,8 +531,6 @@ static inline void ioc3_rx(struct net_device *dev)
>
> ip->rx_skbs[rx_entry] = NULL; /* Poison */
>
> - /* Because we reserve afterwards. */
> - skb_put(new_skb, (1664 + RX_OFFSET));
> rxb = (struct ioc3_erxbuf *)new_skb->data;
> skb_reserve(new_skb, RX_OFFSET);
>
> @@ -846,16 +819,15 @@ static void ioc3_alloc_rings(struct net_device *dev)
> for (i = 0; i < RX_BUFFS; i++) {
> struct sk_buff *skb;
>
> - skb = ioc3_alloc_skb(RX_BUF_ALLOC_SIZE, GFP_ATOMIC);
> + skb = alloc_skb(RX_BUF_SIZE, GFP_ATOMIC);
> if (!skb) {
> show_free_areas(0, NULL);
> continue;
> }
> + skb->dev = dev;
>
> ip->rx_skbs[i] = skb;
>
> - /* Because we reserve afterwards. */
> - skb_put(skb, (1664 + RX_OFFSET));
> rxb = (struct ioc3_erxbuf *)skb->data;
> rxr[i] = cpu_to_be64(ioc3_map(rxb, 1));
> skb_reserve(skb, RX_OFFSET);
^ permalink raw reply
* Re: [PATCH v5 09/17] net: sgi: ioc3-eth: use defines for constants dealing with desc rings
From: Jakub Kicinski @ 2019-08-19 23:53 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Jonathan Corbet, Ralf Baechle, Paul Burton, James Hogan,
Dmitry Torokhov, Lee Jones, David S. Miller, Srinivas Kandagatla,
Alessandro Zummo, Alexandre Belloni, Greg Kroah-Hartman,
Jiri Slaby, Evgeniy Polyakov, linux-doc, linux-kernel, linux-mips,
linux-input, netdev, linux-rtc, linux-serial
In-Reply-To: <20190819163144.3478-10-tbogendoerfer@suse.de>
On Mon, 19 Aug 2019 18:31:32 +0200, Thomas Bogendoerfer wrote:
> Descriptor ring sizes of the IOC3 are more or less fixed size. To
> make clearer where there is a relation to ring sizes use defines.
>
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
Reviewed-by: Jakub Kicinski <jakub.kicinski@netronome.com>
^ permalink raw reply
* Re: [PATCH v5 00/17] Use MFD framework for SGI IOC3 drivers
From: Jakub Kicinski @ 2019-08-19 23:51 UTC (permalink / raw)
To: Thomas Bogendoerfer
Cc: Jonathan Corbet, Ralf Baechle, Paul Burton, James Hogan,
Dmitry Torokhov, Lee Jones, David S. Miller, Srinivas Kandagatla,
Alessandro Zummo, Alexandre Belloni, Greg Kroah-Hartman,
Jiri Slaby, Evgeniy Polyakov, linux-doc, linux-kernel, linux-mips,
linux-input, netdev, linux-rtc, linux-serial
In-Reply-To: <20190819163144.3478-1-tbogendoerfer@suse.de>
On Mon, 19 Aug 2019 18:31:23 +0200, Thomas Bogendoerfer wrote:
> - requested by Jakub I've splitted ioc3 ethernet driver changes into
> more steps to make the transition more visible;
Thanks a lot for doing that!
^ permalink raw reply
* Re: [PATCH net-next v7 2/3] net: phy: add support for clause 37 auto-negotiation
From: Tao Ren @ 2019-08-19 23:22 UTC (permalink / raw)
To: René van Dorst
Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S . Miller,
Vladimir Oltean, Arun Parameswaran, Justin Chen,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
openbmc@lists.ozlabs.org
In-Reply-To: <20190819231526.Horde.8CjxfcGbCnfBNA-nXmq1PJt@www.vdorst.com>
On 8/19/19 4:15 PM, René van Dorst wrote:
> Hi Tao,
>
> Quoting Tao Ren <taoren@fb.com>:
>
>> On 8/11/19 4:40 PM, Tao Ren wrote:
>>> From: Heiner Kallweit <hkallweit1@gmail.com>
>>>
>>> This patch adds support for clause 37 1000Base-X auto-negotiation.
>>>
>>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>>> Signed-off-by: Tao Ren <taoren@fb.com>
>>
>> A kind reminder: could someone help to review the patch when you have bandwidth?
>>
>
> FWIW: I have a similar setup with my device. MAC -> PHY -> SFP cage.
> PHY is a Qualcomm at8031 and is used as a RGMII-to-SerDes converter.
> SerDes only support 100Base-FX and 1000Base-X in this converter mode.
> PHY also supports a RJ45 port but that is not wired on my device.
>
> I converted [0] at803x driver to make use of the PHYLINK API for SFP cage and
> also of these new c37 functions.
>
> In autoneg on and off, it detects the link and can ping a host on the network.
> Tested with 1gbit BiDi optical(1000Base-X) and RJ45 module(SGMII).
> Both work and both devices detects unplug and plug-in of the cable.
>
> output of ethtool:
>
> Autoneg on
> Settings for lan5:
> Supported ports: [ TP MII ]
> Supported link modes: 100baseT/Half 100baseT/Full
> 1000baseT/Full
> 1000baseX/Full
> Supported pause frame use: Symmetric Receive-only
> Supports auto-negotiation: Yes
> Supported FEC modes: Not reported
> Advertised link modes: 100baseT/Half 100baseT/Full
> 1000baseT/Full
> 1000baseX/Full
> Advertised pause frame use: Symmetric Receive-only
> Advertised auto-negotiation: Yes
> Advertised FEC modes: Not reported
> Link partner advertised link modes: 1000baseX/Full
> Link partner advertised pause frame use: Symmetric Receive-only
> Link partner advertised auto-negotiation: Yes
> Link partner advertised FEC modes: Not reported
> Speed: 1000Mb/s
> Duplex: Full
> Port: MII
> PHYAD: 7
> Transceiver: internal
> Auto-negotiation: on
> Supports Wake-on: g
> Wake-on: d
> Link detected: yes
>
> Autoneg off
> Settings for lan5:
> Supported ports: [ TP MII ]
> Supported link modes: 100baseT/Half 100baseT/Full
> 1000baseT/Full
> 1000baseX/Full
> Supported pause frame use: Symmetric Receive-only
> Supports auto-negotiation: Yes
> Supported FEC modes: Not reported
> Advertised link modes: 1000baseT/Full
> Advertised pause frame use: Symmetric Receive-only
> Advertised auto-negotiation: No
> Advertised FEC modes: Not reported
> Speed: 1000Mb/s
> Duplex: Full
> Port: MII
> PHYAD: 7
> Transceiver: internal
> Auto-negotiation: off
> Supports Wake-on: g
> Wake-on: d
> Link detected: yes
>
> Tested-by: René van Dorst <opensource@vdorst.com>
>
> Greats,
>
> René
Great. Thank you for the testing, René.
Cheers,
Tao
^ permalink raw reply
* Re: [net-next v2 00/14][pull request] 100GbE Intel Wired LAN Driver Updates 2019-08-19
From: Jakub Kicinski @ 2019-08-19 23:16 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: davem, netdev, nhorman, sassmann
In-Reply-To: <20190819161708.3763-1-jeffrey.t.kirsher@intel.com>
On Mon, 19 Aug 2019 09:16:54 -0700, Jeff Kirsher wrote:
> This series contains updates to ice driver only.
FWIW from API perspective this set LGTM, however, the code doesn't
always seem well thought out ;)
^ permalink raw reply
* Re: [PATCH net-next v7 2/3] net: phy: add support for clause 37 auto-negotiation
From: René van Dorst @ 2019-08-19 23:15 UTC (permalink / raw)
To: Tao Ren
Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S . Miller,
Vladimir Oltean, Arun Parameswaran, Justin Chen, netdev,
linux-kernel, openbmc
In-Reply-To: <3af5d897-7f97-a223-2d7b-56e09b83dcb5@fb.com>
Hi Tao,
Quoting Tao Ren <taoren@fb.com>:
> On 8/11/19 4:40 PM, Tao Ren wrote:
>> From: Heiner Kallweit <hkallweit1@gmail.com>
>>
>> This patch adds support for clause 37 1000Base-X auto-negotiation.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> Signed-off-by: Tao Ren <taoren@fb.com>
>
> A kind reminder: could someone help to review the patch when you
> have bandwidth?
>
FWIW: I have a similar setup with my device. MAC -> PHY -> SFP cage.
PHY is a Qualcomm at8031 and is used as a RGMII-to-SerDes converter.
SerDes only support 100Base-FX and 1000Base-X in this converter mode.
PHY also supports a RJ45 port but that is not wired on my device.
I converted [0] at803x driver to make use of the PHYLINK API for SFP cage and
also of these new c37 functions.
In autoneg on and off, it detects the link and can ping a host on the network.
Tested with 1gbit BiDi optical(1000Base-X) and RJ45 module(SGMII).
Both work and both devices detects unplug and plug-in of the cable.
output of ethtool:
Autoneg on
Settings for lan5:
Supported ports: [ TP MII ]
Supported link modes: 100baseT/Half 100baseT/Full
1000baseT/Full
1000baseX/Full
Supported pause frame use: Symmetric Receive-only
Supports auto-negotiation: Yes
Supported FEC modes: Not reported
Advertised link modes: 100baseT/Half 100baseT/Full
1000baseT/Full
1000baseX/Full
Advertised pause frame use: Symmetric Receive-only
Advertised auto-negotiation: Yes
Advertised FEC modes: Not reported
Link partner advertised link modes: 1000baseX/Full
Link partner advertised pause frame use: Symmetric Receive-only
Link partner advertised auto-negotiation: Yes
Link partner advertised FEC modes: Not reported
Speed: 1000Mb/s
Duplex: Full
Port: MII
PHYAD: 7
Transceiver: internal
Auto-negotiation: on
Supports Wake-on: g
Wake-on: d
Link detected: yes
Autoneg off
Settings for lan5:
Supported ports: [ TP MII ]
Supported link modes: 100baseT/Half 100baseT/Full
1000baseT/Full
1000baseX/Full
Supported pause frame use: Symmetric Receive-only
Supports auto-negotiation: Yes
Supported FEC modes: Not reported
Advertised link modes: 1000baseT/Full
Advertised pause frame use: Symmetric Receive-only
Advertised auto-negotiation: No
Advertised FEC modes: Not reported
Speed: 1000Mb/s
Duplex: Full
Port: MII
PHYAD: 7
Transceiver: internal
Auto-negotiation: off
Supports Wake-on: g
Wake-on: d
Link detected: yes
Tested-by: René van Dorst <opensource@vdorst.com>
Greats,
René
[0]
https://github.com/vDorst/linux-1/blob/1d8cb01bc8047bda94c076676e47b09d2f31069d/drivers/net/phy/at803x.c
>
> Cheers,
>
> Tao
>
>> ---
>> Changes in v7:
>> - Update "if (AUTONEG_ENABLE != phydev->autoneg)" to
>> "if (phydev->autoneg != AUTONEG_ENABLE)" so checkpatch.pl is happy.
>> Changes in v6:
>> - add "Signed-off-by: Tao Ren <taoren@fb.com>"
>> Changes in v1-v5:
>> - nothing changed. It's given v5 just to align with the version of
>> patch series.
>>
>> drivers/net/phy/phy_device.c | 139 +++++++++++++++++++++++++++++++++++
>> include/linux/phy.h | 5 ++
>> 2 files changed, 144 insertions(+)
>>
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index 252a712d1b2b..301a794b2963 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -1617,6 +1617,40 @@ static int genphy_config_advert(struct
>> phy_device *phydev)
>> return changed;
>> }
>>
>> +/**
>> + * genphy_c37_config_advert - sanitize and advertise
>> auto-negotiation parameters
>> + * @phydev: target phy_device struct
>> + *
>> + * Description: Writes MII_ADVERTISE with the appropriate values,
>> + * after sanitizing the values to make sure we only advertise
>> + * what is supported. Returns < 0 on error, 0 if the PHY's advertisement
>> + * hasn't changed, and > 0 if it has changed. This function is intended
>> + * for Clause 37 1000Base-X mode.
>> + */
>> +static int genphy_c37_config_advert(struct phy_device *phydev)
>> +{
>> + u16 adv = 0;
>> +
>> + /* Only allow advertising what this PHY supports */
>> + linkmode_and(phydev->advertising, phydev->advertising,
>> + phydev->supported);
>> +
>> + if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
>> + phydev->advertising))
>> + adv |= ADVERTISE_1000XFULL;
>> + if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
>> + phydev->advertising))
>> + adv |= ADVERTISE_1000XPAUSE;
>> + if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
>> + phydev->advertising))
>> + adv |= ADVERTISE_1000XPSE_ASYM;
>> +
>> + return phy_modify_changed(phydev, MII_ADVERTISE,
>> + ADVERTISE_1000XFULL | ADVERTISE_1000XPAUSE |
>> + ADVERTISE_1000XHALF | ADVERTISE_1000XPSE_ASYM,
>> + adv);
>> +}
>> +
>> /**
>> * genphy_config_eee_advert - disable unwanted eee mode advertisement
>> * @phydev: target phy_device struct
>> @@ -1726,6 +1760,54 @@ int genphy_config_aneg(struct phy_device *phydev)
>> }
>> EXPORT_SYMBOL(genphy_config_aneg);
>>
>> +/**
>> + * genphy_c37_config_aneg - restart auto-negotiation or write BMCR
>> + * @phydev: target phy_device struct
>> + *
>> + * Description: If auto-negotiation is enabled, we configure the
>> + * advertising, and then restart auto-negotiation. If it is not
>> + * enabled, then we write the BMCR. This function is intended
>> + * for use with Clause 37 1000Base-X mode.
>> + */
>> +int genphy_c37_config_aneg(struct phy_device *phydev)
>> +{
>> + int err, changed;
>> +
>> + if (phydev->autoneg != AUTONEG_ENABLE)
>> + return genphy_setup_forced(phydev);
>> +
>> + err = phy_modify(phydev, MII_BMCR, BMCR_SPEED1000 | BMCR_SPEED100,
>> + BMCR_SPEED1000);
>> + if (err)
>> + return err;
>> +
>> + changed = genphy_c37_config_advert(phydev);
>> + if (changed < 0) /* error */
>> + return changed;
>> +
>> + if (!changed) {
>> + /* Advertisement hasn't changed, but maybe aneg was never on to
>> + * begin with? Or maybe phy was isolated?
>> + */
>> + int ctl = phy_read(phydev, MII_BMCR);
>> +
>> + if (ctl < 0)
>> + return ctl;
>> +
>> + if (!(ctl & BMCR_ANENABLE) || (ctl & BMCR_ISOLATE))
>> + changed = 1; /* do restart aneg */
>> + }
>> +
>> + /* Only restart aneg if we are advertising something different
>> + * than we were before.
>> + */
>> + if (changed > 0)
>> + return genphy_restart_aneg(phydev);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(genphy_c37_config_aneg);
>> +
>> /**
>> * genphy_aneg_done - return auto-negotiation status
>> * @phydev: target phy_device struct
>> @@ -1864,6 +1946,63 @@ int genphy_read_status(struct phy_device *phydev)
>> }
>> EXPORT_SYMBOL(genphy_read_status);
>>
>> +/**
>> + * genphy_c37_read_status - check the link status and update
>> current link state
>> + * @phydev: target phy_device struct
>> + *
>> + * Description: Check the link, then figure out the current state
>> + * by comparing what we advertise with what the link partner
>> + * advertises. This function is for Clause 37 1000Base-X mode.
>> + */
>> +int genphy_c37_read_status(struct phy_device *phydev)
>> +{
>> + int lpa, err, old_link = phydev->link;
>> +
>> + /* Update the link, but return if there was an error */
>> + err = genphy_update_link(phydev);
>> + if (err)
>> + return err;
>> +
>> + /* why bother the PHY if nothing can have changed */
>> + if (phydev->autoneg == AUTONEG_ENABLE && old_link && phydev->link)
>> + return 0;
>> +
>> + phydev->duplex = DUPLEX_UNKNOWN;
>> + phydev->pause = 0;
>> + phydev->asym_pause = 0;
>> +
>> + if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete) {
>> + lpa = phy_read(phydev, MII_LPA);
>> + if (lpa < 0)
>> + return lpa;
>> +
>> + linkmode_mod_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
>> + phydev->lp_advertising, lpa & LPA_LPACK);
>> + linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
>> + phydev->lp_advertising, lpa & LPA_1000XFULL);
>> + linkmode_mod_bit(ETHTOOL_LINK_MODE_Pause_BIT,
>> + phydev->lp_advertising, lpa & LPA_1000XPAUSE);
>> + linkmode_mod_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
>> + phydev->lp_advertising,
>> + lpa & LPA_1000XPAUSE_ASYM);
>> +
>> + phy_resolve_aneg_linkmode(phydev);
>> + } else if (phydev->autoneg == AUTONEG_DISABLE) {
>> + int bmcr = phy_read(phydev, MII_BMCR);
>> +
>> + if (bmcr < 0)
>> + return bmcr;
>> +
>> + if (bmcr & BMCR_FULLDPLX)
>> + phydev->duplex = DUPLEX_FULL;
>> + else
>> + phydev->duplex = DUPLEX_HALF;
>> + }
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL(genphy_c37_read_status);
>> +
>> /**
>> * genphy_soft_reset - software reset the PHY via BMCR_RESET bit
>> * @phydev: target phy_device struct
>> diff --git a/include/linux/phy.h b/include/linux/phy.h
>> index 462b90b73f93..81a2921512ee 100644
>> --- a/include/linux/phy.h
>> +++ b/include/linux/phy.h
>> @@ -1077,6 +1077,11 @@ int genphy_suspend(struct phy_device *phydev);
>> int genphy_resume(struct phy_device *phydev);
>> int genphy_loopback(struct phy_device *phydev, bool enable);
>> int genphy_soft_reset(struct phy_device *phydev);
>> +
>> +/* Clause 37 */
>> +int genphy_c37_config_aneg(struct phy_device *phydev);
>> +int genphy_c37_read_status(struct phy_device *phydev);
>> +
>> static inline int genphy_no_soft_reset(struct phy_device *phydev)
>> {
>> return 0;
>>
^ permalink raw reply
* Re: What to do when a bridge port gets its pvid deleted?
From: Nikolay Aleksandrov @ 2019-08-19 23:12 UTC (permalink / raw)
To: Vladimir Oltean, Ido Schimmel
Cc: Florian Fainelli, Roopa Prabhu, netdev, Andrew Lunn,
Vivien Didelot, stephen
In-Reply-To: <031521d2-2fb5-dd0f-2cb0-a75daa76022d@cumulusnetworks.com>
On 8/20/19 2:01 AM, Nikolay Aleksandrov wrote:
> On 8/20/19 12:10 AM, Vladimir Oltean wrote:
> [snip]
>> It's good to know that it's there (like you said, it explains some
>> things) but I can't exactly say that removing it helps in any way.
>> In fact, removing it only overwrites the dsa_8021q VLANs with 1 during
>> bridge_join, while not actually doing anything upon a vlan_filtering
>> toggle.
>> So the sja1105 driver is in a way shielded by DSA from the bridge, for
>> the better.
>> It still appears to be true that the bridge doesn't think it's
>> necessary to notify through SWITCHDEV_OBJ_ID_PORT_VLAN again. So my
>> best bet is to restore the pvid on my own.
>> However I've already stumbled upon an apparent bug while trying to do
>> that. Does this look off? If it doesn't, I'll submit it as a patch:
>>
>> commit 788f03991aa576fc0b4b26ca330af0f412c55582
>> Author: Vladimir Oltean <olteanv@gmail.com>
>> Date: Mon Aug 19 22:57:00 2019 +0300
>>
>> net: bridge: Keep the BRIDGE_VLAN_INFO_PVID flag in net_bridge_vlan
>>
>> Currently this simplified code snippet fails:
>>
>> br_vlan_get_pvid(netdev, &pvid);
>> br_vlan_get_info(netdev, pvid, &vinfo);
>> ASSERT(!(vinfo.flags & BRIDGE_VLAN_INFO_PVID));
>>
>> It is intuitive that the pvid of a netdevice should have the
>> BRIDGE_VLAN_INFO_PVID flag set.
>>
>> However I can't seem to pinpoint a commit where this behavior was
>> introduced. It seems like it's been like that since forever.
>>
>> Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
>>
>> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
>> index 021cc9f66804..f49b2758bcab 100644
>> --- a/net/bridge/br_vlan.c
>> +++ b/net/bridge/br_vlan.c
>> @@ -68,10 +68,13 @@ static bool __vlan_add_flags(struct
>> net_bridge_vlan *v, u16 flags)
>> else
>> vg = nbp_vlan_group(v->port);
>>
>> - if (flags & BRIDGE_VLAN_INFO_PVID)
>> + if (flags & BRIDGE_VLAN_INFO_PVID) {
>> ret = __vlan_add_pvid(vg, v->vid);
>> - else
>> + v->flags |= BRIDGE_VLAN_INFO_PVID;
>> + } else {
>> ret = __vlan_delete_pvid(vg, v->vid);
>> + v->flags &= ~BRIDGE_VLAN_INFO_PVID;
>> + }
>>
>> if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
>> v->flags |= BRIDGE_VLAN_INFO_UNTAGGED;
>>
>
> Hi Vladimir,
> I know it looks logical to have it, but there are a few reasons why we don't
> do it, most importantly because we need to have only one visible pvid at any single
> time, even if it's stale - it must be just one. Right now that rule will not
> be violated by your change, but people will try using this flag and could see
Obviously, I'm talking about RCU pvid/vlan use cases similar to the dumps.
The locked cases are fine.
> two pvids simultaneously. You can see that the pvid code is even using memory
> barriers to propagate the new value faster and everywhere the pvid is read only once.
> That is the reason the flag is set dynamically when dumping entries, too.
> A second (weaker) argument against would be given the above we don't want
> another way to do the same thing, specifically if it can provide us with
i.e. I would like to avoid explaining why this shouldn't be relied upon without locking
> two pvids (e.g. if walking the vlan list) or if it can provide us with a pvid
> different from the one set in the vg.
> If you do need that flag, you can change br_vlan_get_info() to set the flag like
> the other places do - if the vid matches the current vg pvid, or you could change
> the whole pvid handling code to this new scheme as long as it can guarantee a single
> pvid when walking the vlan list and fast pvid retrieval in the fast-path.
>
> Cheers,
> Nik
>
>
^ permalink raw reply
* Re: [net-next v2 04/14] ice: fix set pause param autoneg check
From: Jakub Kicinski @ 2019-08-19 23:11 UTC (permalink / raw)
To: Jeff Kirsher
Cc: davem, Paul Greenwalt, netdev, nhorman, sassmann, Andrew Bowers
In-Reply-To: <20190819161708.3763-5-jeffrey.t.kirsher@intel.com>
On Mon, 19 Aug 2019 09:16:58 -0700, Jeff Kirsher wrote:
> + pcaps = devm_kzalloc(&vsi->back->pdev->dev, sizeof(*pcaps),
> + GFP_KERNEL);
> + if (!pcaps)
> + return -ENOMEM;
> +
> + /* Get current PHY config */
> + status = ice_aq_get_phy_caps(pi, false, ICE_AQC_REPORT_SW_CFG, pcaps,
> + NULL);
> + if (status) {
> + devm_kfree(&vsi->back->pdev->dev, pcaps);
> + return -EIO;
> + }
> +
> + is_an = ((pcaps->caps & ICE_AQC_PHY_AN_MODE) ?
> + AUTONEG_ENABLE : AUTONEG_DISABLE);
> +
> + devm_kfree(&vsi->back->pdev->dev, pcaps);
Is it just me or is this use of devm_k*alloc absolutely pointless?
^ permalink raw reply
* Re: What to do when a bridge port gets its pvid deleted?
From: Nikolay Aleksandrov @ 2019-08-19 23:01 UTC (permalink / raw)
To: Vladimir Oltean, Ido Schimmel
Cc: Florian Fainelli, Roopa Prabhu, netdev, Andrew Lunn,
Vivien Didelot, stephen
In-Reply-To: <CA+h21hrt9SXPDZq8i1=dZsa4iPHzKwzHnTGUM+ysXascUoKOpQ@mail.gmail.com>
On 8/20/19 12:10 AM, Vladimir Oltean wrote:
[snip]
> It's good to know that it's there (like you said, it explains some
> things) but I can't exactly say that removing it helps in any way.
> In fact, removing it only overwrites the dsa_8021q VLANs with 1 during
> bridge_join, while not actually doing anything upon a vlan_filtering
> toggle.
> So the sja1105 driver is in a way shielded by DSA from the bridge, for
> the better.
> It still appears to be true that the bridge doesn't think it's
> necessary to notify through SWITCHDEV_OBJ_ID_PORT_VLAN again. So my
> best bet is to restore the pvid on my own.
> However I've already stumbled upon an apparent bug while trying to do
> that. Does this look off? If it doesn't, I'll submit it as a patch:
>
> commit 788f03991aa576fc0b4b26ca330af0f412c55582
> Author: Vladimir Oltean <olteanv@gmail.com>
> Date: Mon Aug 19 22:57:00 2019 +0300
>
> net: bridge: Keep the BRIDGE_VLAN_INFO_PVID flag in net_bridge_vlan
>
> Currently this simplified code snippet fails:
>
> br_vlan_get_pvid(netdev, &pvid);
> br_vlan_get_info(netdev, pvid, &vinfo);
> ASSERT(!(vinfo.flags & BRIDGE_VLAN_INFO_PVID));
>
> It is intuitive that the pvid of a netdevice should have the
> BRIDGE_VLAN_INFO_PVID flag set.
>
> However I can't seem to pinpoint a commit where this behavior was
> introduced. It seems like it's been like that since forever.
>
> Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
>
> diff --git a/net/bridge/br_vlan.c b/net/bridge/br_vlan.c
> index 021cc9f66804..f49b2758bcab 100644
> --- a/net/bridge/br_vlan.c
> +++ b/net/bridge/br_vlan.c
> @@ -68,10 +68,13 @@ static bool __vlan_add_flags(struct
> net_bridge_vlan *v, u16 flags)
> else
> vg = nbp_vlan_group(v->port);
>
> - if (flags & BRIDGE_VLAN_INFO_PVID)
> + if (flags & BRIDGE_VLAN_INFO_PVID) {
> ret = __vlan_add_pvid(vg, v->vid);
> - else
> + v->flags |= BRIDGE_VLAN_INFO_PVID;
> + } else {
> ret = __vlan_delete_pvid(vg, v->vid);
> + v->flags &= ~BRIDGE_VLAN_INFO_PVID;
> + }
>
> if (flags & BRIDGE_VLAN_INFO_UNTAGGED)
> v->flags |= BRIDGE_VLAN_INFO_UNTAGGED;
>
Hi Vladimir,
I know it looks logical to have it, but there are a few reasons why we don't
do it, most importantly because we need to have only one visible pvid at any single
time, even if it's stale - it must be just one. Right now that rule will not
be violated by your change, but people will try using this flag and could see
two pvids simultaneously. You can see that the pvid code is even using memory
barriers to propagate the new value faster and everywhere the pvid is read only once.
That is the reason the flag is set dynamically when dumping entries, too.
A second (weaker) argument against would be given the above we don't want
another way to do the same thing, specifically if it can provide us with
two pvids (e.g. if walking the vlan list) or if it can provide us with a pvid
different from the one set in the vg.
If you do need that flag, you can change br_vlan_get_info() to set the flag like
the other places do - if the vid matches the current vg pvid, or you could change
the whole pvid handling code to this new scheme as long as it can guarantee a single
pvid when walking the vlan list and fast pvid retrieval in the fast-path.
Cheers,
Nik
^ permalink raw reply
* Re: [PATCH net-next 0/1] net: fec: add C45 MDIO read/write support
From: Andrew Lunn @ 2019-08-19 22:54 UTC (permalink / raw)
To: Marco Hartmann
Cc: Andy Duan, davem@davemloft.net, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org, Christian Herber
In-Reply-To: <1566234659-7164-1-git-send-email-marco.hartmann@nxp.com>
On Mon, Aug 19, 2019 at 05:11:14PM +0000, Marco Hartmann wrote:
> As of yet, the Fast Ethernet Controller (FEC) driver only supports Clause 22
> conform MDIO transactions. IEEE 802.3ae Clause 45 defines a modified MDIO
> protocol that uses a two staged access model in order to increase the address
> space.
>
> This patch adds support for Clause 45 conform MDIO read and write operations to
> the FEC driver.
Hi Marco
Do all versions of the FEC hardware support C45? Or do we need to make
use of the quirk support in this driver to just enable it for some
revisions of FEC?
Thanks
Andrew
^ permalink raw reply
* Re: KASAN: use-after-free Read in tls_push_sg
From: Jakub Kicinski @ 2019-08-19 22:42 UTC (permalink / raw)
To: syzbot
Cc: aviadye, borisp, daniel, davejwatson, davem, john.fastabend,
linux-kernel, netdev, syzkaller-bugs
In-Reply-To: <0000000000000d1491058919b662@google.com>
On Fri, 17 May 2019 11:40:05 -0700, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 35c99ffa Merge tag 'for_linus' of git://git.kernel.org/pub..
> git tree: net-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=10ff3322a00000
> kernel config: https://syzkaller.appspot.com/x/.config?x=82f0809e8f0a8c87
> dashboard link: https://syzkaller.appspot.com/bug?extid=66fbe4719f6ef22754ee
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+66fbe4719f6ef22754ee@syzkaller.appspotmail.com
Most likely:
#syz fix: net/tls: fix page double free on TX cleanup
^ permalink raw reply
* Re: WARNING: ODEBUG bug in tls_sw_free_resources_tx
From: Jakub Kicinski @ 2019-08-19 22:37 UTC (permalink / raw)
To: syzbot
Cc: aviadye, borisp, daniel, davejwatson, davem, john.fastabend,
linux-kernel, netdev, syzkaller-bugs
In-Reply-To: <00000000000062c5c3057a095d25@google.com>
On Tue, 06 Nov 2018 17:52:03 -0800, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 7c6c54b505b8 Merge branch 'i2c/for-next' of git://git.kern..
> git tree: net-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=1276246d400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=136ed5b316dbf1d8
> dashboard link: https://syzkaller.appspot.com/bug?extid=70ab6a1f8151888c4ea0
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+70ab6a1f8151888c4ea0@syzkaller.appspotmail.com
This was most likely fixed by John:
#syz fix: net/tls: remove close callback sock unlock/lock around TX work flush
^ permalink raw reply
* Re: BUG: unable to handle kernel paging request in tls_prots
From: Jakub Kicinski @ 2019-08-19 22:34 UTC (permalink / raw)
To: syzbot
Cc: ast, bpf, daniel, davem, edumazet, john.fastabend, kafai, kuznet,
linux-kernel, netdev, songliubraving, syzkaller-bugs, yhs,
yoshfuji
In-Reply-To: <000000000000d7bcbb058c3758a1@google.com>
On Wed, 26 Jun 2019 03:17:05 -0700, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 904d88d7 qmi_wwan: Fix out-of-bounds read
> git tree: net
> console output: https://syzkaller.appspot.com/x/log.txt?x=14a8b865a00000
> kernel config: https://syzkaller.appspot.com/x/.config?x=137ec2016ea3870d
> dashboard link: https://syzkaller.appspot.com/bug?extid=4207c7f3a443366d8aa2
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15576c71a00000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+4207c7f3a443366d8aa2@syzkaller.appspotmail.com
That's one of the bugs John fixed:
#syz fix: bpf: sockmap/tls, close can race with map free
^ permalink raw reply
* Re: [PATCH net-next v7 2/3] net: phy: add support for clause 37 auto-negotiation
From: Tao Ren @ 2019-08-19 22:18 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S . Miller,
Vladimir Oltean, Arun Parameswaran, Justin Chen,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
openbmc@lists.ozlabs.org
In-Reply-To: <20190811234010.3673592-1-taoren@fb.com>
On 8/11/19 4:40 PM, Tao Ren wrote:
> From: Heiner Kallweit <hkallweit1@gmail.com>
>
> This patch adds support for clause 37 1000Base-X auto-negotiation.
>
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> Signed-off-by: Tao Ren <taoren@fb.com>
A kind reminder: could someone help to review the patch when you have bandwidth?
Cheers,
Tao
> ---
> Changes in v7:
> - Update "if (AUTONEG_ENABLE != phydev->autoneg)" to
> "if (phydev->autoneg != AUTONEG_ENABLE)" so checkpatch.pl is happy.
> Changes in v6:
> - add "Signed-off-by: Tao Ren <taoren@fb.com>"
> Changes in v1-v5:
> - nothing changed. It's given v5 just to align with the version of
> patch series.
>
> drivers/net/phy/phy_device.c | 139 +++++++++++++++++++++++++++++++++++
> include/linux/phy.h | 5 ++
> 2 files changed, 144 insertions(+)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 252a712d1b2b..301a794b2963 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1617,6 +1617,40 @@ static int genphy_config_advert(struct phy_device *phydev)
> return changed;
> }
>
> +/**
> + * genphy_c37_config_advert - sanitize and advertise auto-negotiation parameters
> + * @phydev: target phy_device struct
> + *
> + * Description: Writes MII_ADVERTISE with the appropriate values,
> + * after sanitizing the values to make sure we only advertise
> + * what is supported. Returns < 0 on error, 0 if the PHY's advertisement
> + * hasn't changed, and > 0 if it has changed. This function is intended
> + * for Clause 37 1000Base-X mode.
> + */
> +static int genphy_c37_config_advert(struct phy_device *phydev)
> +{
> + u16 adv = 0;
> +
> + /* Only allow advertising what this PHY supports */
> + linkmode_and(phydev->advertising, phydev->advertising,
> + phydev->supported);
> +
> + if (linkmode_test_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
> + phydev->advertising))
> + adv |= ADVERTISE_1000XFULL;
> + if (linkmode_test_bit(ETHTOOL_LINK_MODE_Pause_BIT,
> + phydev->advertising))
> + adv |= ADVERTISE_1000XPAUSE;
> + if (linkmode_test_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
> + phydev->advertising))
> + adv |= ADVERTISE_1000XPSE_ASYM;
> +
> + return phy_modify_changed(phydev, MII_ADVERTISE,
> + ADVERTISE_1000XFULL | ADVERTISE_1000XPAUSE |
> + ADVERTISE_1000XHALF | ADVERTISE_1000XPSE_ASYM,
> + adv);
> +}
> +
> /**
> * genphy_config_eee_advert - disable unwanted eee mode advertisement
> * @phydev: target phy_device struct
> @@ -1726,6 +1760,54 @@ int genphy_config_aneg(struct phy_device *phydev)
> }
> EXPORT_SYMBOL(genphy_config_aneg);
>
> +/**
> + * genphy_c37_config_aneg - restart auto-negotiation or write BMCR
> + * @phydev: target phy_device struct
> + *
> + * Description: If auto-negotiation is enabled, we configure the
> + * advertising, and then restart auto-negotiation. If it is not
> + * enabled, then we write the BMCR. This function is intended
> + * for use with Clause 37 1000Base-X mode.
> + */
> +int genphy_c37_config_aneg(struct phy_device *phydev)
> +{
> + int err, changed;
> +
> + if (phydev->autoneg != AUTONEG_ENABLE)
> + return genphy_setup_forced(phydev);
> +
> + err = phy_modify(phydev, MII_BMCR, BMCR_SPEED1000 | BMCR_SPEED100,
> + BMCR_SPEED1000);
> + if (err)
> + return err;
> +
> + changed = genphy_c37_config_advert(phydev);
> + if (changed < 0) /* error */
> + return changed;
> +
> + if (!changed) {
> + /* Advertisement hasn't changed, but maybe aneg was never on to
> + * begin with? Or maybe phy was isolated?
> + */
> + int ctl = phy_read(phydev, MII_BMCR);
> +
> + if (ctl < 0)
> + return ctl;
> +
> + if (!(ctl & BMCR_ANENABLE) || (ctl & BMCR_ISOLATE))
> + changed = 1; /* do restart aneg */
> + }
> +
> + /* Only restart aneg if we are advertising something different
> + * than we were before.
> + */
> + if (changed > 0)
> + return genphy_restart_aneg(phydev);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(genphy_c37_config_aneg);
> +
> /**
> * genphy_aneg_done - return auto-negotiation status
> * @phydev: target phy_device struct
> @@ -1864,6 +1946,63 @@ int genphy_read_status(struct phy_device *phydev)
> }
> EXPORT_SYMBOL(genphy_read_status);
>
> +/**
> + * genphy_c37_read_status - check the link status and update current link state
> + * @phydev: target phy_device struct
> + *
> + * Description: Check the link, then figure out the current state
> + * by comparing what we advertise with what the link partner
> + * advertises. This function is for Clause 37 1000Base-X mode.
> + */
> +int genphy_c37_read_status(struct phy_device *phydev)
> +{
> + int lpa, err, old_link = phydev->link;
> +
> + /* Update the link, but return if there was an error */
> + err = genphy_update_link(phydev);
> + if (err)
> + return err;
> +
> + /* why bother the PHY if nothing can have changed */
> + if (phydev->autoneg == AUTONEG_ENABLE && old_link && phydev->link)
> + return 0;
> +
> + phydev->duplex = DUPLEX_UNKNOWN;
> + phydev->pause = 0;
> + phydev->asym_pause = 0;
> +
> + if (phydev->autoneg == AUTONEG_ENABLE && phydev->autoneg_complete) {
> + lpa = phy_read(phydev, MII_LPA);
> + if (lpa < 0)
> + return lpa;
> +
> + linkmode_mod_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
> + phydev->lp_advertising, lpa & LPA_LPACK);
> + linkmode_mod_bit(ETHTOOL_LINK_MODE_1000baseX_Full_BIT,
> + phydev->lp_advertising, lpa & LPA_1000XFULL);
> + linkmode_mod_bit(ETHTOOL_LINK_MODE_Pause_BIT,
> + phydev->lp_advertising, lpa & LPA_1000XPAUSE);
> + linkmode_mod_bit(ETHTOOL_LINK_MODE_Asym_Pause_BIT,
> + phydev->lp_advertising,
> + lpa & LPA_1000XPAUSE_ASYM);
> +
> + phy_resolve_aneg_linkmode(phydev);
> + } else if (phydev->autoneg == AUTONEG_DISABLE) {
> + int bmcr = phy_read(phydev, MII_BMCR);
> +
> + if (bmcr < 0)
> + return bmcr;
> +
> + if (bmcr & BMCR_FULLDPLX)
> + phydev->duplex = DUPLEX_FULL;
> + else
> + phydev->duplex = DUPLEX_HALF;
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL(genphy_c37_read_status);
> +
> /**
> * genphy_soft_reset - software reset the PHY via BMCR_RESET bit
> * @phydev: target phy_device struct
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 462b90b73f93..81a2921512ee 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -1077,6 +1077,11 @@ int genphy_suspend(struct phy_device *phydev);
> int genphy_resume(struct phy_device *phydev);
> int genphy_loopback(struct phy_device *phydev, bool enable);
> int genphy_soft_reset(struct phy_device *phydev);
> +
> +/* Clause 37 */
> +int genphy_c37_config_aneg(struct phy_device *phydev);
> +int genphy_c37_read_status(struct phy_device *phydev);
> +
> static inline int genphy_no_soft_reset(struct phy_device *phydev)
> {
> return 0;
>
^ permalink raw reply
* [PATCH] Fix a double free bug in rsi_91x_deinit
From: Hui Peng @ 2019-08-19 22:02 UTC (permalink / raw)
To: security
Cc: Hui Peng, Mathias Payer, Kalle Valo, David S. Miller,
linux-wireless, netdev, linux-kernel
`dev` (struct rsi_91x_usbdev *) field of adapter
(struct rsi_91x_usbdev *) is allocated and initialized in
`rsi_init_usb_interface`. If any error is detected in information
read from the device side, `rsi_init_usb_interface` will be
freed. However, in the higher level error handling code in
`rsi_probe`, if error is detected, `rsi_91x_deinit` is called
again, in which `dev` will be freed again, resulting double free.
This patch fixes the double free by removing the free operation on
`dev` in `rsi_init_usb_interface`, because `rsi_91x_deinit` is also
used in `rsi_disconnect`, in that code path, the `dev` field is not
(and thus needs to be) freed.
This bug was found in v4.19, but is also present in the latest version
of kernel.
Reported-by: Hui Peng <benquike@gmail.com>
Reported-by: Mathias Payer <mathias.payer@nebelwelt.net>
Signed-off-by: Hui Peng <benquike@gmail.com>
---
drivers/net/wireless/rsi/rsi_91x_usb.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/wireless/rsi/rsi_91x_usb.c b/drivers/net/wireless/rsi/rsi_91x_usb.c
index c0a163e40402..ac917227f708 100644
--- a/drivers/net/wireless/rsi/rsi_91x_usb.c
+++ b/drivers/net/wireless/rsi/rsi_91x_usb.c
@@ -640,7 +640,6 @@ static int rsi_init_usb_interface(struct rsi_hw *adapter,
kfree(rsi_dev->tx_buffer);
fail_eps:
- kfree(rsi_dev);
return status;
}
--
2.22.1
^ permalink raw reply related
* Re: [PATCH net-next] netdevsim: Fix build error without CONFIG_INET
From: Jakub Kicinski @ 2019-08-19 21:59 UTC (permalink / raw)
To: YueHaibing; +Cc: davem, idosch, jiri, mcroce, linux-kernel, netdev
In-Reply-To: <20190819120825.74460-1-yuehaibing@huawei.com>
On Mon, 19 Aug 2019 20:08:25 +0800, YueHaibing wrote:
> If CONFIG_INET is not set, building fails:
>
> drivers/net/netdevsim/dev.o: In function `nsim_dev_trap_report_work':
> dev.c:(.text+0x67b): undefined reference to `ip_send_check'
>
> Add CONFIG_INET Kconfig dependency to fix this.
>
> Reported-by: Hulk Robot <hulkci@huawei.com>
> Fixes: da58f90f11f5 ("netdevsim: Add devlink-trap support")
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Hmm.. I'd rather the test module did not have hard dependencies on
marginally important config options. We have done a pretty good job
so far limiting the requirements though separating the code out at
compilation object level. The more tests depend on netdevsim and the
more bots we have running tests against randconfig - the more important
this is.
This missing reference here is for calculating a checksum over a
constant header.. could we perhaps just hard code the checksum?
^ permalink raw reply
* Re: INFO: task hung in tls_sw_release_resources_tx
From: Jakub Kicinski @ 2019-08-19 21:35 UTC (permalink / raw)
To: Eric Biggers
Cc: Steffen Klassert, syzbot, ast, aviadye, borisp, bpf, daniel,
davejwatson, davem, hdanton, john.fastabend, netdev,
syzkaller-bugs, herbert, linux-crypto
In-Reply-To: <20190819141255.010a323a@cakuba.netronome.com>
On Mon, 19 Aug 2019 14:12:55 -0700, Jakub Kicinski wrote:
> Looks like the dup didn't tickle syzbot the right way. Let me retry
> sending this directly to the original report.
Oh, no, my bad, there was just a third bug of the same nature.
tls_sw_release_resources_tx got renamed at some point, hence
the duplicate report.
^ permalink raw reply
* Re: general protection fault in tls_setsockopt
From: Jakub Kicinski @ 2019-08-19 21:29 UTC (permalink / raw)
To: syzbot
Cc: ast, aviadye, bhole_prashant_q7, borisp, daniel, davejwatson,
davem, john.fastabend, linux-kernel, linux-kselftest, netdev,
shuah, songliubraving, syzkaller-bugs
In-Reply-To: <000000000000d917f4058dd525cf@google.com>
On Tue, 16 Jul 2019 16:58:06 -0700, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: a131c2bf Merge tag 'acpi-5.3-rc1-2' of git://git.kernel.or..
> git tree: net-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=1603e9c0600000
> kernel config: https://syzkaller.appspot.com/x/.config?x=8bff73c5ba9e876
> dashboard link: https://syzkaller.appspot.com/bug?extid=23d9570edec63669d890
> compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=13560870600000
#syz fix: net/tls: partially revert fix transition through disconnect with close
^ permalink raw reply
* Re: KASAN: slab-out-of-bounds Read in tls_write_space
From: Jakub Kicinski @ 2019-08-19 21:22 UTC (permalink / raw)
To: syzbot
Cc: aviadye, borisp, davejwatson, davem, linux-kernel, netdev,
syzkaller-bugs
In-Reply-To: <0000000000000a5b840576bad225@google.com>
On Tue, 25 Sep 2018 16:54:03 -0700, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: bd4d08daeb95 Merge branch 'net-dsa-b53-SGMII-modes-fixes'
> git tree: net-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=110b6a1a400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=e79b9299baeb2298
> dashboard link: https://syzkaller.appspot.com/bug?extid=12638b747fd208f6cff0
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=167d9b9e400000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15c4003a400000
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+12638b747fd208f6cff0@syzkaller.appspotmail.com
#syz dup: general protection fault in tls_write_space
^ permalink raw reply
* Re: KASAN: use-after-free Read in tls_write_space
From: Jakub Kicinski @ 2019-08-19 21:22 UTC (permalink / raw)
To: syzbot
Cc: aviadye, borisp, davejwatson, davem, linux-kernel, netdev,
syzkaller-bugs
In-Reply-To: <0000000000003dab1605704fb71d@google.com>
On Fri, 06 Jul 2018 00:36:02 -0700, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 1a84d7fdb5fc Merge branch 'mlxsw-Add-resource-scale-tests'
> git tree: net-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=17dec75c400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=a63be0c83e84d370
> dashboard link: https://syzkaller.appspot.com/bug?extid=2134b6b74dec9f8c760f
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+2134b6b74dec9f8c760f@syzkaller.appspotmail.com
#syz dup: general protection fault in tls_write_space
^ 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