* 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: 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: [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: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: [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: [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: 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: [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 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 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: [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
* [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
* [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 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 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 5/6] net: dsa: Allow proper internal use of VLANs
From: Vladimir Oltean @ 2019-08-20 0:00 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>
Below commit:
commit 2ea7a679ca2abd251c1ec03f20508619707e1749
Author: Andrew Lunn <andrew@lunn.ch>
Date: Tue Nov 7 00:04:24 2017 +0100
net: dsa: Don't add vlans when vlan filtering is disabled
The software bridge can be build with vlan filtering support
included. However, by default it is turned off. In its turned off
state, it still passes VLANs via switchev, even though they are not to
be used. Don't pass these VLANs to the hardware. Only do so when vlan
filtering is enabled.
This fixes at least one corner case. There are still issues in other
corners, such as when vlan_filtering is later enabled.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: David S. Miller <davem@davemloft.net>
stubs out SWITCHDEV_OBJ_ID_PORT_VLAN objects notified by the bridge core
to DSA drivers when vlan_filtering is 0.
This is generally a good thing, because it allows dsa_8021q to make
private use of VLANs in that mode.
So it makes sense to move the check for the bridge presence and
vlan_filtering setting one layer above. We don't want calls from
dsa_8021q to be prevented by this, only from the bridge core.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
net/dsa/port.c | 10 ++--------
net/dsa/slave.c | 8 ++++++++
2 files changed, 10 insertions(+), 8 deletions(-)
diff --git a/net/dsa/port.c b/net/dsa/port.c
index f75301456430..c1cc41c1eeb6 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -348,10 +348,7 @@ int dsa_port_vlan_add(struct dsa_port *dp,
.vlan = vlan,
};
- if (!dp->bridge_dev || br_vlan_enabled(dp->bridge_dev))
- return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
-
- return 0;
+ return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_ADD, &info);
}
int dsa_port_vlan_del(struct dsa_port *dp,
@@ -363,10 +360,7 @@ int dsa_port_vlan_del(struct dsa_port *dp,
.vlan = vlan,
};
- if (!dp->bridge_dev || br_vlan_enabled(dp->bridge_dev))
- return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info);
-
- return 0;
+ return dsa_port_notify(dp, DSA_NOTIFIER_VLAN_DEL, &info);
}
int dsa_port_vid_add(struct dsa_port *dp, u16 vid, u16 flags)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 33f41178afcc..6a02eb8988d1 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -341,6 +341,8 @@ static int dsa_slave_port_obj_add(struct net_device *dev,
case SWITCHDEV_OBJ_ID_PORT_VLAN:
if (obj->orig_dev != dev)
return -EOPNOTSUPP;
+ if (dp->bridge_dev && !br_vlan_enabled(dp->bridge_dev))
+ return 0;
err = dsa_port_vlan_add(dp, SWITCHDEV_OBJ_PORT_VLAN(obj),
trans);
break;
@@ -373,6 +375,8 @@ static int dsa_slave_port_obj_del(struct net_device *dev,
case SWITCHDEV_OBJ_ID_PORT_VLAN:
if (obj->orig_dev != dev)
return -EOPNOTSUPP;
+ if (dp->bridge_dev && !br_vlan_enabled(dp->bridge_dev))
+ return 0;
err = dsa_port_vlan_del(dp, SWITCHDEV_OBJ_PORT_VLAN(obj));
break;
default:
@@ -1073,6 +1077,8 @@ static int dsa_slave_vlan_rx_add_vid(struct net_device *dev, __be16 proto,
* need to emulate the switchdev prepare + commit phase.
*/
if (dp->bridge_dev) {
+ if (!br_vlan_enabled(dp->bridge_dev))
+ return 0;
/* br_vlan_get_info() returns -EINVAL or -ENOENT if the
* device, respectively the VID is not found, returning
* 0 means success, which is a failure for us here.
@@ -1097,6 +1103,8 @@ static int dsa_slave_vlan_rx_kill_vid(struct net_device *dev, __be16 proto,
* need to emulate the switchdev prepare + commit phase.
*/
if (dp->bridge_dev) {
+ if (!br_vlan_enabled(dp->bridge_dev))
+ return 0;
/* br_vlan_get_info() returns -EINVAL or -ENOENT if the
* device, respectively the VID is not found, returning
* 0 means success, which is a failure for us here.
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 4/6] net: dsa: Don't program the VLAN as pvid on the upstream port
From: Vladimir Oltean @ 2019-08-20 0:00 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")
programs the VLAN from the bridge into the specified port as well as the
upstream port, with the same set of flags.
Consider the typical case of installing pvid 1 on user port 1, pvid 2 on
user port 2, etc. The upstream port would end up having a pvid equal to
the last user port whose pvid was programmed from the bridge. Less than
useful.
So just don't change the pvid of the upstream port and let it be
whatever the driver set it internally to be.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
net/dsa/switch.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/net/dsa/switch.c b/net/dsa/switch.c
index 84ab2336131e..02ccc53f1926 100644
--- a/net/dsa/switch.c
+++ b/net/dsa/switch.c
@@ -239,17 +239,21 @@ dsa_switch_vlan_prepare_bitmap(struct dsa_switch *ds,
const struct switchdev_obj_port_vlan *vlan,
const unsigned long *bitmap)
{
+ struct switchdev_obj_port_vlan v = *vlan;
int port, err;
if (!ds->ops->port_vlan_prepare || !ds->ops->port_vlan_add)
return -EOPNOTSUPP;
for_each_set_bit(port, bitmap, ds->num_ports) {
- err = dsa_port_vlan_check(ds, port, vlan);
+ if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port))
+ v.flags &= ~BRIDGE_VLAN_INFO_PVID;
+
+ err = dsa_port_vlan_check(ds, port, &v);
if (err)
return err;
- err = ds->ops->port_vlan_prepare(ds, port, vlan);
+ err = ds->ops->port_vlan_prepare(ds, port, &v);
if (err)
return err;
}
@@ -262,10 +266,14 @@ dsa_switch_vlan_add_bitmap(struct dsa_switch *ds,
const struct switchdev_obj_port_vlan *vlan,
const unsigned long *bitmap)
{
+ struct switchdev_obj_port_vlan v = *vlan;
int port;
- for_each_set_bit(port, bitmap, ds->num_ports)
- ds->ops->port_vlan_add(ds, port, vlan);
+ for_each_set_bit(port, bitmap, ds->num_ports) {
+ if (dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port))
+ v.flags &= ~BRIDGE_VLAN_INFO_PVID;
+ ds->ops->port_vlan_add(ds, port, &v);
+ }
}
static int dsa_switch_vlan_add(struct dsa_switch *ds,
--
2.17.1
^ permalink raw reply related
* [PATCH net-next 6/6] net: dsa: tag_8021q: Restore bridge pvid when enabling vlan_filtering
From: Vladimir Oltean @ 2019-08-20 0:00 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>
The bridge core assumes that enabling/disabling vlan_filtering will
translate into the simple toggling of a flag for switchdev drivers.
That is clearly not the case for sja1105, which alters the VLAN table
and the pvids in order to obtain port separation in standalone mode.
So, since the bridge will not call any vlan operation through switchdev
after enabling vlan_filtering, we need to ensure we're in a functional
state ourselves.
Hence read the pvid that the bridge is aware of, and program that into
our ports.
Signed-off-by: Vladimir Oltean <olteanv@gmail.com>
---
net/dsa/tag_8021q.c | 32 +++++++++++++++++++++++++++++++-
1 file changed, 31 insertions(+), 1 deletion(-)
diff --git a/net/dsa/tag_8021q.c b/net/dsa/tag_8021q.c
index 67a1bc635a7b..6423beb1efcd 100644
--- a/net/dsa/tag_8021q.c
+++ b/net/dsa/tag_8021q.c
@@ -93,6 +93,33 @@ int dsa_8021q_rx_source_port(u16 vid)
}
EXPORT_SYMBOL_GPL(dsa_8021q_rx_source_port);
+static int dsa_port_restore_pvid(struct dsa_switch *ds, int port)
+{
+ struct bridge_vlan_info vinfo;
+ struct net_device *slave;
+ u16 pvid;
+ int err;
+
+ if (!dsa_is_user_port(ds, port))
+ return 0;
+
+ slave = ds->ports[port].slave;
+
+ err = br_vlan_get_pvid(slave, &pvid);
+ if (err < 0) {
+ dev_err(ds->dev, "Couldn't determine bridge PVID\n");
+ return err;
+ }
+
+ err = br_vlan_get_info(slave, pvid, &vinfo);
+ if (err < 0) {
+ dev_err(ds->dev, "Couldn't determine PVID attributes\n");
+ return err;
+ }
+
+ return dsa_port_vid_add(&ds->ports[port], pvid, vinfo.flags);
+}
+
/* RX VLAN tagging (left) and TX VLAN tagging (right) setup shown for a single
* front-panel switch port (here swp0).
*
@@ -223,7 +250,10 @@ int dsa_port_setup_8021q_tagging(struct dsa_switch *ds, int port, bool enabled)
return err;
}
- return 0;
+ if (!enabled)
+ err = dsa_port_restore_pvid(ds, port);
+
+ return err;
}
EXPORT_SYMBOL_GPL(dsa_port_setup_8021q_tagging);
--
2.17.1
^ permalink raw reply related
* Re: [PATCH v5 11/17] net: sgi: ioc3-eth: no need to stop queue set_multicast_list
From: Jakub Kicinski @ 2019-08-20 0:04 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-12-tbogendoerfer@suse.de>
On Mon, 19 Aug 2019 18:31:34 +0200, Thomas Bogendoerfer wrote:
> netif_stop_queue()/netif_wake_qeue() aren't needed for changing
> multicast filters. Use spinlocks instead for proper protection
> of private struct.
>
> Signed-off-by: Thomas Bogendoerfer <tbogendoerfer@suse.de>
> ---
> drivers/net/ethernet/sgi/ioc3-eth.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/ethernet/sgi/ioc3-eth.c b/drivers/net/ethernet/sgi/ioc3-eth.c
> index d862f28887f9..7f85a3bfef14 100644
> --- a/drivers/net/ethernet/sgi/ioc3-eth.c
> +++ b/drivers/net/ethernet/sgi/ioc3-eth.c
> @@ -1542,8 +1542,7 @@ static void ioc3_set_multicast_list(struct net_device *dev)
> struct netdev_hw_addr *ha;
> u64 ehar = 0;
>
> - netif_stop_queue(dev); /* Lock out others. */
> -
> + spin_lock_irq(&ip->ioc3_lock);
What does this lock protect? 🤔 No question that stopping TX queues
makes little sense, but this function is only called from
ndo_set_rx_mode(), so with rtnl_lock held.
I thought it may protect ip->emcr, but that one is accessed with no
locking from the ioc3_timer() -> ioc3_setup_duplex() path..
> if (dev->flags & IFF_PROMISC) { /* Set promiscuous. */
> ip->emcr |= EMCR_PROMISC;
> writel(ip->emcr, ®s->emcr);
> @@ -1572,7 +1571,7 @@ static void ioc3_set_multicast_list(struct net_device *dev)
> writel(ip->ehar_l, ®s->ehar_l);
> }
>
> - netif_wake_queue(dev); /* Let us get going again. */
> + spin_unlock_irq(&ip->ioc3_lock);
> }
>
> module_pci_driver(ioc3_driver);
^ permalink raw reply
* Re: [PATCH v5 12/17] net: sgi: ioc3-eth: use dma-direct for dma allocations
From: Jakub Kicinski @ 2019-08-20 0:07 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-13-tbogendoerfer@suse.de>
On Mon, 19 Aug 2019 18:31:35 +0200, Thomas Bogendoerfer wrote:
> @@ -1386,18 +1427,24 @@ static netdev_tx_t ioc3_start_xmit(struct sk_buff *skb, struct net_device *dev)
> unsigned long b2 = (data | 0x3fffUL) + 1UL;
> unsigned long s1 = b2 - data;
> unsigned long s2 = data + len - b2;
> + dma_addr_t d;
>
> desc->cmd = cpu_to_be32(len | ETXD_INTWHENDONE |
> ETXD_B1V | ETXD_B2V | w0);
> desc->bufcnt = cpu_to_be32((s1 << ETXD_B1CNT_SHIFT) |
> (s2 << ETXD_B2CNT_SHIFT));
> - desc->p1 = cpu_to_be64(ioc3_map(skb->data, 1));
> - desc->p2 = cpu_to_be64(ioc3_map((void *)b2, 1));
> + d = dma_map_single(ip->dma_dev, skb->data, s1, DMA_TO_DEVICE);
You'll need to check the DMA address with dma_mapping_error(dev, addr),
otherwise static checkers will get upset.
> + desc->p1 = cpu_to_be64(ioc3_map(d, PCI64_ATTR_PREF));
> + d = dma_map_single(ip->dma_dev, (void *)b2, s1, DMA_TO_DEVICE);
> + desc->p2 = cpu_to_be64(ioc3_map(d, PCI64_ATTR_PREF));
^ permalink raw reply
* Re: [PATCH net-next 2/6] net: bridge: Populate the pvid flag in br_vlan_get_info
From: Nikolay Aleksandrov @ 2019-08-20 0:12 UTC (permalink / raw)
To: Vladimir Oltean, f.fainelli, vivien.didelot, andrew, idosch,
roopa, davem
Cc: netdev
In-Reply-To: <20190820000002.9776-3-olteanv@gmail.com>
On 8/20/19 2:59 AM, Vladimir Oltean wrote:
> 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);
>
Looks good, thanks!
Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>
^ permalink raw reply
* [PATCH V40 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode
From: Matthew Garrett @ 2019-08-20 0:17 UTC (permalink / raw)
To: jmorris
Cc: linux-security-module, linux-kernel, linux-api, David Howells,
Alexei Starovoitov, Matthew Garrett, Kees Cook, netdev,
Chun-Yi Lee, Daniel Borkmann
In-Reply-To: <20190820001805.241928-1-matthewgarrett@google.com>
From: David Howells <dhowells@redhat.com>
bpf_read() and bpf_read_str() could potentially be abused to (eg) allow
private keys in kernel memory to be leaked. Disable them if the kernel
has been locked down in confidentiality mode.
Suggested-by: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Signed-off-by: Matthew Garrett <mjg59@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
cc: netdev@vger.kernel.org
cc: Chun-Yi Lee <jlee@suse.com>
cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Daniel Borkmann <daniel@iogearbox.net>
Signed-off-by: James Morris <jmorris@namei.org>
---
include/linux/security.h | 1 +
kernel/trace/bpf_trace.c | 10 ++++++++++
security/lockdown/lockdown.c | 1 +
3 files changed, 12 insertions(+)
diff --git a/include/linux/security.h b/include/linux/security.h
index 0b2529dbf0f4..e604f4c67f03 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -118,6 +118,7 @@ enum lockdown_reason {
LOCKDOWN_INTEGRITY_MAX,
LOCKDOWN_KCORE,
LOCKDOWN_KPROBES,
+ LOCKDOWN_BPF_READ,
LOCKDOWN_CONFIDENTIALITY_MAX,
};
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 1c9a4745e596..33a954c367f3 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -139,8 +139,13 @@ BPF_CALL_3(bpf_probe_read, void *, dst, u32, size, const void *, unsafe_ptr)
{
int ret;
+ ret = security_locked_down(LOCKDOWN_BPF_READ);
+ if (ret < 0)
+ goto out;
+
ret = probe_kernel_read(dst, unsafe_ptr, size);
if (unlikely(ret < 0))
+out:
memset(dst, 0, size);
return ret;
@@ -566,6 +571,10 @@ BPF_CALL_3(bpf_probe_read_str, void *, dst, u32, size,
{
int ret;
+ ret = security_locked_down(LOCKDOWN_BPF_READ);
+ if (ret < 0)
+ goto out;
+
/*
* The strncpy_from_unsafe() call will likely not fill the entire
* buffer, but that's okay in this circumstance as we're probing
@@ -577,6 +586,7 @@ BPF_CALL_3(bpf_probe_read_str, void *, dst, u32, size,
*/
ret = strncpy_from_unsafe(dst, unsafe_ptr, size);
if (unlikely(ret < 0))
+out:
memset(dst, 0, size);
return ret;
diff --git a/security/lockdown/lockdown.c b/security/lockdown/lockdown.c
index 27b2cf51e443..2397772c56bd 100644
--- a/security/lockdown/lockdown.c
+++ b/security/lockdown/lockdown.c
@@ -33,6 +33,7 @@ static char *lockdown_reasons[LOCKDOWN_CONFIDENTIALITY_MAX+1] = {
[LOCKDOWN_INTEGRITY_MAX] = "integrity",
[LOCKDOWN_KCORE] = "/proc/kcore access",
[LOCKDOWN_KPROBES] = "use of kprobes",
+ [LOCKDOWN_BPF_READ] = "use of bpf to read kernel RAM",
[LOCKDOWN_CONFIDENTIALITY_MAX] = "confidentiality",
};
--
2.23.0.rc1.153.gdeed80330f-goog
^ permalink raw reply related
* Re: [PATCH net] ipv6: Fix return value of ipv6_mc_may_pull() for malformed packets
From: David Miller @ 2019-08-20 0:20 UTC (permalink / raw)
To: sbrivio; +Cc: gnault, haliu, edumazet, linus.luessing, netdev
In-Reply-To: <20190819121252.5fccbef2@redhat.com>
From: Stefano Brivio <sbrivio@redhat.com>
Date: Mon, 19 Aug 2019 12:12:52 +0200
> I don't see this on net.git, but it's in your stable bundle on
> Patchwork. Should I resend? Thanks.
I applied it on my laptop while travelling and never pushed it out so it
just rot there, sorry.
Fixed, should be in 'net' now.
^ permalink raw reply
* [PATCH v2] net/ncsi: Ensure 32-bit boundary for data cksum
From: Terry S. Duncan @ 2019-08-20 0:24 UTC (permalink / raw)
To: Samuel Mendoza-Jonas, David S . Miller, netdev, linux-kernel
Cc: openbmc, William Kennington, Joel Stanley, Terry S. Duncan
The NCSI spec indicates that if the data does not end on a 32 bit
boundary, one to three padding bytes equal to 0x00 shall be present to
align the checksum field to a 32-bit boundary.
Signed-off-by: Terry S. Duncan <terry.s.duncan@linux.intel.com>
---
net/ncsi/ncsi-cmd.c | 2 +-
net/ncsi/ncsi-rsp.c | 9 ++++++---
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/net/ncsi/ncsi-cmd.c b/net/ncsi/ncsi-cmd.c
index 5c3fad8cba57..eab4346b0a39 100644
--- a/net/ncsi/ncsi-cmd.c
+++ b/net/ncsi/ncsi-cmd.c
@@ -54,7 +54,7 @@ static void ncsi_cmd_build_header(struct ncsi_pkt_hdr *h,
checksum = ncsi_calculate_checksum((unsigned char *)h,
sizeof(*h) + nca->payload);
pchecksum = (__be32 *)((void *)h + sizeof(struct ncsi_pkt_hdr) +
- nca->payload);
+ ALIGN(nca->payload, 4));
*pchecksum = htonl(checksum);
}
diff --git a/net/ncsi/ncsi-rsp.c b/net/ncsi/ncsi-rsp.c
index 7581bf919885..d876bd55f356 100644
--- a/net/ncsi/ncsi-rsp.c
+++ b/net/ncsi/ncsi-rsp.c
@@ -47,7 +47,8 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
if (ntohs(h->code) != NCSI_PKT_RSP_C_COMPLETED ||
ntohs(h->reason) != NCSI_PKT_RSP_R_NO_ERROR) {
netdev_dbg(nr->ndp->ndev.dev,
- "NCSI: non zero response/reason code\n");
+ "NCSI: non zero response/reason code %04xh, %04xh\n",
+ ntohs(h->code), ntohs(h->reason));
return -EPERM;
}
@@ -55,7 +56,7 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
* sender doesn't support checksum according to NCSI
* specification.
*/
- pchecksum = (__be32 *)((void *)(h + 1) + payload - 4);
+ pchecksum = (__be32 *)((void *)(h + 1) + ALIGN(payload, 4) - 4);
if (ntohl(*pchecksum) == 0)
return 0;
@@ -63,7 +64,9 @@ static int ncsi_validate_rsp_pkt(struct ncsi_request *nr,
sizeof(*h) + payload - 4);
if (*pchecksum != htonl(checksum)) {
- netdev_dbg(nr->ndp->ndev.dev, "NCSI: checksum mismatched\n");
+ netdev_dbg(nr->ndp->ndev.dev,
+ "NCSI: checksum mismatched; recd: %08x calc: %08x\n",
+ *pchecksum, htonl(checksum));
return -EINVAL;
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net] ipv6/addrconf: allow adding multicast addr if IFA_F_MCAUTOJOIN is set
From: David Miller @ 2019-08-20 0:32 UTC (permalink / raw)
To: liuhangbin; +Cc: netdev, challa, dsahern, jishi
In-Reply-To: <20190813135232.27146-1-liuhangbin@gmail.com>
From: Hangbin Liu <liuhangbin@gmail.com>
Date: Tue, 13 Aug 2019 21:52:32 +0800
> The ip address autojoin is not working for IPv6 as ipv6_add_addr()
> will return -EADDRNOTAVAIL when adding a multicast address.
>
> Reported-by: Jianlin Shi <jishi@redhat.com>
> Fixes: 93a714d6b53d ("multicast: Extend ip address command to enable multicast group join/leave on")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
I don't understand how all of this works and why ipv6_add_addr(), which
seems designed explicitly to exclude multicast addresses, should accept
them and what all of the possible fallout might be from such a change.
Your commit message is way too terse and makes it impossible to evaluate
your change. Really, a change of this nature should have a couple paragraphs
of text explaining the existing situation, what is wrong with it, how you
are fixing it, and why you are fixing it that way.
Thanks.
^ 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