* Re: [PATCH net-next] net: sched: set dedicated tcf_walker flag when tp is empty
From: David Miller @ 2019-02-25 18:18 UTC (permalink / raw)
To: vladbu; +Cc: netdev, jhs, xiyou.wangcong, jiri
In-Reply-To: <20190225153831.10037-1-vladbu@mellanox.com>
From: Vlad Buslov <vladbu@mellanox.com>
Date: Mon, 25 Feb 2019 17:38:31 +0200
> Using tcf_walker->stop flag to determine when tcf_walker->fn() was called
> at least once is unreliable. Some classifiers set 'stop' flag on error
> before calling walker callback, other classifiers used to call it with NULL
> filter pointer when empty. In order to prevent further regressions, extend
> tcf_walker structure with dedicated 'nonempty' flag. Set this flag in
> tcf_walker->fn() implementation that is used to check if classifier has
> filters configured.
>
> Fixes: 8b64678e0af8 ("net: sched: refactor tp insert/delete for concurrent execution")
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
> Suggested-by: Cong Wang <xiyou.wangcong@gmail.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net] net: sched: act_tunnel_key: fix NULL pointer dereference during init
From: David Miller @ 2019-02-25 18:16 UTC (permalink / raw)
To: vladbu; +Cc: netdev, dcaratti, jhs, xiyou.wangcong, jiri, wenxu, roid
In-Reply-To: <20190225152827.8741-1-vladbu@mellanox.com>
From: Vlad Buslov <vladbu@mellanox.com>
Date: Mon, 25 Feb 2019 17:28:27 +0200
> Metadata pointer is only initialized for action TCA_TUNNEL_KEY_ACT_SET, but
> it is unconditionally dereferenced in tunnel_key_init() error handler.
> Verify that metadata pointer is not NULL before dereferencing it in
> tunnel_key_init error handling code.
>
> Fixes: ee28bb56ac5b ("net/sched: fix memory leak in act_tunnel_key_init()")
> Signed-off-by: Vlad Buslov <vladbu@mellanox.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net-next v2 5/7] net: sched: pie: add more cases to auto-tune alpha and beta
From: Leslie Monis @ 2019-02-25 18:04 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20190225.094038.1156384775297802527.davem@davemloft.net>
On Mon, Feb 25, 2019 at 09:40:38AM -0800, David Miller wrote:
> From: Leslie Monis <lesliemonis@gmail.com>
> Date: Mon, 25 Feb 2019 15:50:49 +0530
>
> > + power = 100;
> > + while (q->vars.prob < MAX_PROB / (u64)power &&
>
> You have to use div_u64() or similar when doing 64-bit divisons otherwise
> you'll get build failures on 32-bit platforms.
Alright. Thanks for the feedback.
^ permalink raw reply
* Re: [PATCH net-next resend 2/2] net: dsa: mv88e6xxx: Fix phylink_validate for Topaz family
From: David Miller @ 2019-02-25 17:47 UTC (permalink / raw)
To: marek.behun; +Cc: netdev, andrew, f.fainelli
In-Reply-To: <20190225113955.25700-3-marek.behun@nic.cz>
From: Marek Behún <marek.behun@nic.cz>
Date: Mon, 25 Feb 2019 12:39:55 +0100
> The Topaz family should have different phylink_validate method from the
> Peridot, since on Topaz the port supporting 2500BaseX mode is port 5,
> not 9 and 10.
>
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Applied.
^ permalink raw reply
* Re: [PATCH net-next resend 1/2] net: dsa: mv88e6xxx: Default CMODE to 1000BaseX only on 6390X
From: David Miller @ 2019-02-25 17:47 UTC (permalink / raw)
To: marek.behun; +Cc: netdev, andrew, f.fainelli
In-Reply-To: <20190225113955.25700-2-marek.behun@nic.cz>
From: Marek Behún <marek.behun@nic.cz>
Date: Mon, 25 Feb 2019 12:39:54 +0100
> Commit 787799a9d555 sets the SERDES interfaces of 6390 and 6390X to
> 1000BaseX, but this is only needed on 6390X, since there are SERDES
> interfaces which can be used on lower ports on 6390.
>
> This commit fixes this by returning to previous behaviour on 6390.
> (Previous behaviour means that CMODE is not set at all if requested mode
> is NA).
>
> This is needed on Turris MOX, where the 88e6190 is connected to CPU in
> 2500BaseX mode.
>
> Fixes: 787799a9d555 ("net: dsa: mv88e6xxx: Default ports 9/10 6390X CMODE to 1000BaseX")
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net v3 0/2] selftests: pmtu: fix and increase coverage
From: Stefano Brivio @ 2019-02-25 17:46 UTC (permalink / raw)
To: Paolo Abeni; +Cc: netdev, David S. Miller, David Ahern, Sabrina Dubroca
In-Reply-To: <cover.1551106591.git.pabeni@redhat.com>
On Mon, 25 Feb 2019 16:08:35 +0100
Paolo Abeni <pabeni@redhat.com> wrote:
> This series includes a fixup for the pmtu.sh test script, related to IPv6
> address management, and adds coverage for the recently reported and fixed
> PMTU exception issue
>
> v2 -> v3:
> - more cleanups
>
> v1 -> v2:
> - several script cleanups
>
> Paolo Abeni (2):
> selftests: pmtu: disable DAD in all namespaces
> selftests: pmtu: add explicit tests for PMTU exceptions cleanup
Thanks for following up!
Reviewed-by: Stefano Brivio <sbrivio@redhat.com>
--
Stefano
^ permalink raw reply
* Re: [PATCH] tcp: clean up SOCK_DEBUG()
From: David Miller @ 2019-02-25 17:41 UTC (permalink / raw)
To: laoar.shao; +Cc: netdev, shaoyafang
In-Reply-To: <1551090828-31988-1-git-send-email-laoar.shao@gmail.com>
From: Yafang Shao <laoar.shao@gmail.com>
Date: Mon, 25 Feb 2019 18:33:48 +0800
> Per discussion with Daniel[1] and Eric[2], these SOCK_DEBUG() calles in
> TCP are not needed now.
> We'd better clean up it.
>
> [1] https://patchwork.ozlabs.org/patch/1035573/
> [2] https://patchwork.ozlabs.org/patch/1040533/
>
> Signed-off-by: Yafang Shao <laoar.shao@gmail.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next v2 5/7] net: sched: pie: add more cases to auto-tune alpha and beta
From: David Miller @ 2019-02-25 17:40 UTC (permalink / raw)
To: lesliemonis
Cc: jhs, netdev, dave, tahiliani, dhavaljkhandla26, hrishihiraskar,
bmanish15597, sdp.sachin
In-Reply-To: <20190225102051.12268-6-lesliemonis@gmail.com>
From: Leslie Monis <lesliemonis@gmail.com>
Date: Mon, 25 Feb 2019 15:50:49 +0530
> + power = 100;
> + while (q->vars.prob < MAX_PROB / (u64)power &&
You have to use div_u64() or similar when doing 64-bit divisons otherwise
you'll get build failures on 32-bit platforms.
^ permalink raw reply
* Re: [PATCH net-next] tcp: remove unused parameter of tcp_sacktag_bsearch()
From: David Miller @ 2019-02-25 17:38 UTC (permalink / raw)
To: ap420073; +Cc: edumazet, netdev
In-Reply-To: <20190225094233.16231-1-ap420073@gmail.com>
From: Taehee Yoo <ap420073@gmail.com>
Date: Mon, 25 Feb 2019 18:42:33 +0900
> parameter state in the tcp_sacktag_bsearch() is not used.
> So, it can be removed.
>
> Signed-off-by: Taehee Yoo <ap420073@gmail.com>
Applied, thanks Taehee.
^ permalink raw reply
* Re: [PATCH v2] net: dsa: fix a leaked reference by adding missing of_node_put
From: David Miller @ 2019-02-25 17:35 UTC (permalink / raw)
To: wen.yang99
Cc: andrew, vivien.didelot, f.fainelli, netdev, linux-kernel,
wang.yi59
In-Reply-To: <1551079339-30270-1-git-send-email-wen.yang99@zte.com.cn>
From: Wen Yang <wen.yang99@zte.com.cn>
Date: Mon, 25 Feb 2019 15:22:19 +0800
> The call to of_parse_phandle returns a node pointer with refcount
> incremented thus it must be explicitly decremented after the last
> usage.
>
> Detected by coccinelle with the following warnings:
> ./net/dsa/port.c:294:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 284, but without a corresponding object release within this function.
> ./net/dsa/dsa2.c:627:3-9: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 618, but without a corresponding object release within this function.
> ./net/dsa/dsa2.c:630:3-9: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 618, but without a corresponding object release within this function.
> ./net/dsa/dsa2.c:636:3-9: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 618, but without a corresponding object release within this function.
> ./net/dsa/dsa2.c:639:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 618, but without a corresponding object release within this function.
>
> Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
> Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>
> Reviewed-by: "David S. Miller" <davem@davemloft.net>
Applied.
^ permalink raw reply
* Re: [PATCH] net: dsa: read mac address from DT for slave device
From: Florian Fainelli @ 2019-02-25 17:27 UTC (permalink / raw)
To: xiaofeis
Cc: Vinod Koul, David S. Miller, linux-arm-msm, Bjorn Andersson,
Andrew Lunn, Vivien Didelot, Niklas Cassel, netdev
In-Reply-To: <fc02f99d3a84ace86f6c2cb6411b4b33@codeaurora.org>
On 2/25/19 5:28 AM, xiaofeis@codeaurora.org wrote:
> Hi Florian
>
> We have two slave DSA interfaces, wan0 and lan0, one is for wan port,
> and the other is for lan port. Customer has it's mac address pool, they
> want
> to assign the mac address from the pool on wan0, lan0, and other
> interfaces like
> wifi, bt. Coreboot/uboot will populate it to the DTS node, so the driver
> can
> get it from it's node. For DSA slave interface, it already has it's own
> DTS node, it's
> easy to just add one porperty "local-mac-address" there for the usage in
> DSA driver.
>
> If not use DSA framework, normally we will use eth0.x and eth0.y for wan
> and lan.
> On this case, customer usually also assign the MAC address on these
> logical interface
> from it's pool.
OK, but this is not necessary per my previous explanation: the CPU <=>
WAN pipe is a separate broadcast domain (otherwise it is a security hole
since you exposing LAN machines to the outside world), and so there is
no need for a separate MAC address. It might be convenient to have one,
especially for the provider, if they run a management software (e.g.:
TR69), but it is not required per-se.
Let me ask a secondary question here, how many Ethernet MACs connect to
the switch in this configuration? Is there one that is supposed to be
assigned all LAN traffic and one that is supposed to be assigned all WAN
traffic? If so, then what you are doing makes even less
>
> On 2019-02-22 23:43, Florian Fainelli wrote:
>> On 2/22/19 4:58 AM, Vinod Koul wrote:
>>> From: Xiaofei Shen <xiaofeis@codeaurora.org>
>>>
>>> Before creating a slave netdevice, get the mac address from DTS and
>>> apply in case it is valid.
>>
>> Can you explain your use case in details?
>>
>> Assigning a MAC address to a network device that represents a switch
>> port does not quite make sense in general. The switch port is really
>> representing one end of a pipe, so one side you have stations and on the
>> other side, you have the CPU/management Ethernet MAC controller's MAC
>> address which constitutes a station as well. The DSA slave network
>> devices are just software constructs meant to steer traffic towards
>> specific ports of the switch, but they are all from the perpsective of
>> traffic reaching the CPU Port in the first place, therefore traffic that
>> is generally a known unicast Ethernet frame with the CPU's MAC address
>> as MAC DA (and of course all types of unknown MC, management traffic
>> etc.)
>>
>> By default, DSA switch need to come up in a configuration where all
>> ports (except CPU/management) must be strictly separate from every other
>> port such that we can achieve what a standalone Ethernet NIC would do.
>> This works because all ports are isolated from one another, so there is
>> no cross talk and so having the same MAC address (the one from the CPU)
>> on the DSA slave network devices just works, each port is a separate
>> broadcast domain.
>>
>> Once you start bridging one or ore ports, the bridge root port will have
>> a MAC address, most likely the one the CPU/management Ethernet MAC, but
>> similarly, this is not an issue and that's exactly how a software bridge
>> would work as well.
>>
>>>
>>> Signed-off-by: Xiaofei Shen <xiaofeis@codeaurora.org>
>>> Signed-off-by: Vinod Koul <vkoul@kernel.org>
>>> ---
>>> include/net/dsa.h | 1 +
>>> net/dsa/dsa2.c | 1 +
>>> net/dsa/slave.c | 5 ++++-
>>> 3 files changed, 6 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/net/dsa.h b/include/net/dsa.h
>>> index b3eefe8e18fd..aa24ce756679 100644
>>> --- a/include/net/dsa.h
>>> +++ b/include/net/dsa.h
>>> @@ -198,6 +198,7 @@ struct dsa_port {
>>> unsigned int index;
>>> const char *name;
>>> const struct dsa_port *cpu_dp;
>>> + const char *mac;
>>> struct device_node *dn;
>>> unsigned int ageing_time;
>>> u8 stp_state;
>>> diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
>>> index a1917025e155..afb7d9fa42f6 100644
>>> --- a/net/dsa/dsa2.c
>>> +++ b/net/dsa/dsa2.c
>>> @@ -261,6 +261,7 @@ static int dsa_port_setup(struct dsa_port *dp)
>>> int err = 0;
>>>
>>> memset(&dp->devlink_port, 0, sizeof(dp->devlink_port));
>>> + dp->mac = of_get_mac_address(dp->dn);
>>>
>>> if (dp->type != DSA_PORT_TYPE_UNUSED)
>>> err = devlink_port_register(ds->devlink, &dp->devlink_port,
>>> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
>>> index a3fcc1d01615..8e64c4e947c6 100644
>>> --- a/net/dsa/slave.c
>>> +++ b/net/dsa/slave.c
>>> @@ -1308,7 +1308,10 @@ int dsa_slave_create(struct dsa_port *port)
>>> slave_dev->features = master->vlan_features | NETIF_F_HW_TC;
>>> slave_dev->hw_features |= NETIF_F_HW_TC;
>>> slave_dev->ethtool_ops = &dsa_slave_ethtool_ops;
>>> - eth_hw_addr_inherit(slave_dev, master);
>>> + if (port->mac && is_valid_ether_addr(port->mac))
>>> + ether_addr_copy(slave_dev->dev_addr, port->mac);
>>> + else
>>> + eth_hw_addr_inherit(slave_dev, master);
>>> slave_dev->priv_flags |= IFF_NO_QUEUE;
>>> slave_dev->netdev_ops = &dsa_slave_netdev_ops;
>>> slave_dev->switchdev_ops = &dsa_slave_switchdev_ops;
>>>
--
Florian
^ permalink raw reply
* Re: [PATCH 1/3] net: ethernet: add support for PCS and 2.5G speed
From: Florian Fainelli @ 2019-02-25 17:21 UTC (permalink / raw)
To: Parshuram Raju Thombare, nicolas.ferre@microchip.com,
davem@davemloft.net, netdev@vger.kernel.org, andrew@lunn.ch,
hkallweit1@gmail.com, linux-kernel@vger.kernel.org,
Rafal Ciepiela, Piotr Sroka, Jan Kotas
In-Reply-To: <CO2PR07MB2469FCDFEE81BE4D3ECE9C92C17A0@CO2PR07MB2469.namprd07.prod.outlook.com>
On 2/25/19 1:11 AM, Parshuram Raju Thombare wrote:
>> Le 2/22/19 à 12:12 PM, Parshuram Thombare a écrit :
>>> This patch add support for PCS (for SGMII interface) and 2.5Gbps MAC
>>> in Cadence ethernet controller driver.
>>
>> At a high level you don't seem to be making use of PHYLINK so which 2.5Gbps
>> interfaces do you actually support?
>>
>
> New ethernet controller have MAC which support 2.5G speed.
> Also there is addition of PCS and SGMII interface.
I should have asked this more clearly: have you tested with SFP modules
for instance? If you want to be able to reliably support 2500baseT
and/or 2500baseX with hot plugging of such modules, you need to
implement PHYLINK for that network driver, there is no other way around.
>
>>>
>>> Signed-off-by: Parshuram Thombare <pthombar@cadence.com>
>>> ---
>>
>> [snip]
>>
>>> @@ -361,26 +361,50 @@ static int macb_mdio_write(struct mii_bus *bus, int
>> mii_id, int regnum,
>>> * macb_set_tx_clk() - Set a clock to a new frequency
>>> * @clk Pointer to the clock to change
>>> * @rate New frequency in Hz
>>> + * @interafce Phy interface
>>
>> Typo: @interface and this is an unrelated change.
>>
>>> * @dev Pointer to the struct net_device
>>> */
>>> -static void macb_set_tx_clk(struct clk *clk, int speed, struct
>>> net_device *dev)
>>> +static void macb_set_tx_clk(struct clk *clk, int speed,
>>> + phy_interface_t interface, struct net_device *dev)
>>> {
>>> long ferr, rate, rate_rounded;
>>>
>>> if (!clk)
>>> return;
>>>
>>> - switch (speed) {
>>> - case SPEED_10:
>>> + if (interface == PHY_INTERFACE_MODE_GMII ||
>>> + interface == PHY_INTERFACE_MODE_MII) {
>>> + switch (speed) {
>>> + case SPEED_10:> rate = 2500000;
>>
>> You need to add one tab to align rate and break.
>
> Do you mean a tab each for rate and break lines ?
> All switch statements are aligned at a tab. I am not sure how does case and rate got on same line.
It should look like this:
switch (cond) {
case cond1:
do_something();
break;
etc.
>
>>
>>> break;
>>> - case SPEED_100:
>>> + case SPEED_100:
>>> rate = 25000000;
>>> break;
>>> - case SPEED_1000:
>>> + case SPEED_1000:
>>> rate = 125000000;
>>> break;
>>> - default:
>>> + default:
>>> + return;
>>> + }
>>> + } else if (interface == PHY_INTERFACE_MODE_SGMII) {
>>> + switch (speed) {
>>> + case SPEED_10:
>>> + rate = 1250000;
>>> + break;
>>> + case SPEED_100:
>>> + rate = 12500000;
>>> + break;
>>> + case SPEED_1000:
>>> + rate = 125000000;
>>> + break;
>>> + case SPEED_2500:
>>> + rate = 312500000;
>>> + break;
>>> + default:
>>> + return;
>>
>> The indentation is broken here and you can greatly simplify this with a simple
>> function that returns speed * 1250 and does an initial check for unsupported
>> speeds.
>>
>
> I ran checkpatch.pl and all indentation issues were cleared. But I think having function
> is better option, I will make that change.
>
>>> + }
>>> + } else {
>>> return;
>>> }
>>>
>>> @@ -410,30 +434,49 @@ static void macb_handle_link_change(struct
>>> net_device *dev)
>>>
>>> spin_lock_irqsave(&bp->lock, flags);
>>>
>>> - if (phydev->link) {
>>> - if ((bp->speed != phydev->speed) ||
>>> - (bp->duplex != phydev->duplex)) {
>>> - u32 reg;
>>> -
>>> - reg = macb_readl(bp, NCFGR);
>>> - reg &= ~(MACB_BIT(SPD) | MACB_BIT(FD));
>>> - if (macb_is_gem(bp))
>>> - reg &= ~GEM_BIT(GBE);
>>> + if (phydev->link && (bp->speed != phydev->speed ||
>>> + bp->duplex != phydev->duplex)) {
>>> + u32 reg;
>>>
>>> - if (phydev->duplex)
>>> - reg |= MACB_BIT(FD);
>>> + reg = macb_readl(bp, NCFGR);
>>> + reg &= ~(MACB_BIT(SPD) | MACB_BIT(FD));
>>> + if (macb_is_gem(bp))
>>> + reg &= ~GEM_BIT(GBE);
>>> + if (phydev->duplex)
>>> + reg |= MACB_BIT(FD);
>>> + macb_or_gem_writel(bp, NCFGR, reg);
>>> +
>>> + if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII &&
>>> + (phydev->speed == SPEED_1000 ||
>>> + phydev->speed == SPEED_2500)) {
>>> + if (bp->caps & MACB_CAPS_TWO_PT_FIVE_GIG_SPEED) {
>>> + reg = gem_readl(bp, NCR) &
>>> + ~GEM_BIT(TWO_PT_FIVE_GIG);
>>> + gem_writel(bp, NCR, reg);
>>> + }
>>
>> If you are making correct use of the capabilities then there is no point in re-
>> checking them here. If you allowed the MAC to advertise 2.5Gbps then it is de-
>> facto SGMII capable.
>
> PHY_INTERFACE_MODE_SGMII is selected only on the basis of presence of PCS.
> This additional check is to make sure PHY also support 1G/2.5G.
My point is that you should never get to that function with
bp->phy_interface == PHY_INTERFACE_MODE_SGMII if you have done necessary
verifications at the time you connect to the PHY. If you let
PHY_INTERFACE_MODE_SGMII go through on a Cadence IP version that does
not have the MACB_CAPS_TWO_PT_FIVE_GIG_SPEED capability bit set, then
this is broken.
>
>>> + gem_writel(bp, NCFGR, GEM_BIT(GBE) |
>>> + gem_readl(bp, NCFGR));
>>> + if (bp->caps & MACB_CAPS_TWO_PT_FIVE_GIG_SPEED
>> &&
>>> + phydev->speed == SPEED_2500)
>>> + gem_writel(bp, NCR, gem_readl(bp, NCR) |
>>> + GEM_BIT(TWO_PT_FIVE_GIG));
>>> + } else if (phydev->speed == SPEED_1000) {
>>> + gem_writel(bp, NCFGR, GEM_BIT(GBE) |
>>> + gem_readl(bp, NCFGR));
>>> + } else {
>>> + if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII)
>> {
>>> + reg = gem_readl(bp, NCFGR);
>>> + reg &= ~(GEM_BIT(SGMIIEN) |
>> GEM_BIT(PCSSEL));
>>> + gem_writel(bp, NCFGR, reg);
>>> + }
>>> if (phydev->speed == SPEED_100)
>>> - reg |= MACB_BIT(SPD);
>>> - if (phydev->speed == SPEED_1000 &&
>>> - bp->caps & MACB_CAPS_GIGABIT_MODE_AVAILABLE)
>>> - reg |= GEM_BIT(GBE);
>>> -
>>> - macb_or_gem_writel(bp, NCFGR, reg);
>>> -
>>> - bp->speed = phydev->speed;
>>> - bp->duplex = phydev->duplex;
>>> - status_change = 1;
>>> + macb_writel(bp, NCFGR, MACB_BIT(SPD) |
>>> + macb_readl(bp, NCFGR));
>>> }
>>
>> There is a lot of repetition while setting the GBE bit which always set based on
>> speed == 1000 irrespective of the mode, so take that part out of the if () else if ()
>> else () clauses.
>>
>
> Ok, I will change it.
>
>>> +
>>> + bp->speed = phydev->speed;
>>> + bp->duplex = phydev->duplex;
>>> + status_change = 1;
>>> }
>>>
>>> if (phydev->link != bp->link) {
>>> @@ -453,7 +496,8 @@ static void macb_handle_link_change(struct net_device
>> *dev)
>>> /* Update the TX clock rate if and only if the link is
>>> * up and there has been a link change.
>>> */
>>> - macb_set_tx_clk(bp->tx_clk, phydev->speed, dev);
>>> + macb_set_tx_clk(bp->tx_clk, phydev->speed,
>>> + bp->phy_interface, dev);
>>>
>>> netif_carrier_on(dev);
>>> netdev_info(dev, "link up (%d/%s)\n", @@ -543,10
>> +587,16 @@ static
>>> int macb_mii_probe(struct net_device *dev)
>>> }
>>>
>>> /* mask with MAC supported features */
>>> - if (macb_is_gem(bp) && bp->caps &
>> MACB_CAPS_GIGABIT_MODE_AVAILABLE)
>>> - phy_set_max_speed(phydev, SPEED_1000);
>>> - else
>>> - phy_set_max_speed(phydev, SPEED_100);
>>> + if (macb_is_gem(bp)) {
>>
>> You have changed the previous logic that also checked for
>> MACB_CAPS_GIGABIT_MODE_AVAILABLE, why?
>
> My understanding is all GEM (ID >= 0x2) support GIGABIT mode.
> Was there any other reason for this check ?
Well, if anyone would know, it would be you, I don't work for Cadence
nor have access to the internal IP documentation.
>
>>> + linkmode_copy(phydev->supported, PHY_GBIT_FEATURES);
>>> + if (bp->caps & MACB_CAPS_TWO_PT_FIVE_GIG_SPEED)
>>> +
>> linkmode_set_bit(ETHTOOL_LINK_MODE_2500baseT_Full_BIT,
>>> + phydev->supported);
>>> + } else {
>>> + linkmode_copy(phydev->supported, PHY_BASIC_FEATURES);
>>> + }
>>> +
>>> + linkmode_copy(phydev->advertising, phydev->supported);
>>>
>>> if (bp->caps & MACB_CAPS_NO_GIGABIT_HALF)
>>> phy_remove_link_mode(phydev,
>>> @@ -2217,8 +2267,6 @@ static void macb_init_hw(struct macb *bp)
>>> macb_set_hwaddr(bp);
>>>
>>> config = macb_mdc_clk_div(bp);
>>> - if (bp->phy_interface == PHY_INTERFACE_MODE_SGMII)
>>> - config |= GEM_BIT(SGMIIEN) | GEM_BIT(PCSSEL);
>>> config |= MACB_BF(RBOF, NET_IP_ALIGN); /* Make eth data
>> aligned */
>>> config |= MACB_BIT(PAE); /* PAuse Enable */
>>> config |= MACB_BIT(DRFCS); /* Discard Rx FCS */
>>> @@ -3255,6 +3303,23 @@ static void macb_configure_caps(struct macb *bp,
>>> dcfg = gem_readl(bp, DCFG1);
>>> if (GEM_BFEXT(IRQCOR, dcfg) == 0)
>>> bp->caps |= MACB_CAPS_ISR_CLEAR_ON_WRITE;
>>> + if (GEM_BFEXT(NO_PCS, dcfg) == 0)
>>> + bp->caps |= MACB_CAPS_PCS;
>>> + switch (MACB_BFEXT(IDNUM, macb_readl(bp, MID))) {
>>> + case MACB_GEM7016_IDNUM:
>>> + case MACB_GEM7017_IDNUM:
>>> + case MACB_GEM7017A_IDNUM:
>>> + case MACB_GEM7020_IDNUM:
>>> + case MACB_GEM7021_IDNUM:
>>> + case MACB_GEM7021A_IDNUM:
>>> + case MACB_GEM7022_IDNUM:
>>> + if (bp->caps & MACB_CAPS_PCS)
>>> + bp->caps |= MACB_CAPS_TWO_PT_FIVE_GIG_SPEED;
>>> + break;
>>> +
>>> + default:
>>> + break;
>>> + }
>>> dcfg = gem_readl(bp, DCFG2);
>>> if ((dcfg & (GEM_BIT(RX_PKT_BUFF) | GEM_BIT(TX_PKT_BUFF)))
>> == 0)
>>> bp->caps |= MACB_CAPS_FIFO_MODE;
>>> @@ -4110,7 +4175,28 @@ static int macb_probe(struct platform_device
>> *pdev)
>>> else
>>> bp->phy_interface = PHY_INTERFACE_MODE_MII;
>>> } else {
>>> + switch (err) {
>>> + case PHY_INTERFACE_MODE_SGMII:
>>> + if (bp->caps & MACB_CAPS_PCS) {
>>> + bp->phy_interface = PHY_INTERFACE_MODE_SGMII;
>>> + break;
>>> + }
>>
>> If SGMII was selected on a version of the IP that does not support it, then falling
>> back to GMII or MII does not sound correct, this is a hard error that must be
>> handled as such.
>> --
>> Florian
>
> My intention was to continue (instead of failing) with whatever functionality is available.
> Can we have some error message and continue with what is available ?
This is probably not going to help anyone, imagine you incorrectly
specified a 'phy-mode' property in the Device Tree and you have to dig
into the driver in the function that sets the clock rate to find out
what you just defaulted to 1Gbits/GMII for instance. This is not user
friendly at all and will increase the support burden on your end as
well, if SGMII is specified and the IP does not support it, detect it
and return an error, how that propagates, either as a probe failure or
the inability to open the network device, your call.
--
Florian
^ permalink raw reply
* Re: [PATCH net] sit: use ipv6_mod_enabled to check if ipv6 is disabled
From: David Ahern @ 2019-02-25 16:57 UTC (permalink / raw)
To: Eric Dumazet, Hangbin Liu; +Cc: netdev, Stefano Brivio, David S . Miller
In-Reply-To: <1dd57d9d-fed2-67b8-ac28-7ef3681eeed2@gmail.com>
On 2/25/19 9:39 AM, Eric Dumazet wrote:
>
>
> On 02/25/2019 12:17 AM, Hangbin Liu wrote:
>> On Sun, Feb 24, 2019 at 08:24:51PM -0800, Eric Dumazet wrote:
>>>
>>>
>>> On 02/24/2019 08:12 PM, Hangbin Liu wrote:
>>>> ipv6_mod_enabled() is more safe and gentle to check if ipv6 is disabled
>>>> at running time.
>>>>
>>>
>>>
>>> Why is it better exactly ?
>>>
>>> IPv6 can be enabled on the host, but disabled per device
>>>
>>> /proc/sys/net/ipv6/conf/{name}/disable_ipv6
>>
>> Sorry, it looks I didn't make it clear in the commit description.
>> This issue only occurs when IPv6 is disabled at boot time as there is
>> no IPv6 route entry. Disable ipv6 on specific interface is not affected.
>> So check ipv6_mod_enabled() is enough and we don't need to worry about
>> the rcu_read_lock or the dev status.
>>
>> Should I update the commit description?
>
> Certainly. Are you telling us skb->dev could be NULL here ?
>
> Because rcu_read_lock() should already be asserted.
>
Same response as geneve. The existing check is more appropriate and
relevant for the code path: is ipv6 enabled on this device versus is
ipv6 enabled at all.
^ permalink raw reply
* Re: [PATCH net] geneve: use ipv6_mod_enabled to check if ipv6 is disabled
From: David Ahern @ 2019-02-25 16:57 UTC (permalink / raw)
To: Hangbin Liu, netdev; +Cc: Stefano Brivio, David S . Miller
In-Reply-To: <20190225041350.9913-1-liuhangbin@gmail.com>
On 2/24/19 9:13 PM, Hangbin Liu wrote:
> ipv6_mod_enabled() is more safe and gentle to check if ipv6 is disabled
> at running time.
>
> Fixes: c0a47e44c098 ("geneve: should not call rt6_lookup() when ipv6 was disabled")
> Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
> ---
> drivers/net/geneve.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/geneve.c b/drivers/net/geneve.c
> index 3377ac66a347..42c5e43ece68 100644
> --- a/drivers/net/geneve.c
> +++ b/drivers/net/geneve.c
> @@ -1514,7 +1514,7 @@ static void geneve_link_config(struct net_device *dev,
> case AF_INET6: {
> struct rt6_info *rt;
>
> - if (!__in6_dev_get(dev))
> + if (!ipv6_mod_enabled())
> break;
>
> rt = rt6_lookup(geneve->net, &info->key.u.ipv6.dst, NULL, 0,
>
seems to me the existing check is more appropriate and relevant for the
code path: is ipv6 enabled on this device versus is ipv6 enabled at all.
^ permalink raw reply
* Re: INFO: task hung in rtnetlink_rcv_msg
From: Joe Perches @ 2019-02-25 16:55 UTC (permalink / raw)
To: Jesper Dangaard Brouer, Daniel Borkmann
Cc: Jakub Kicinski, syzbot, ast, christian, davem, dsahern, hawk,
idosch, john.fastabend, kafai, ktkhai, linux-kernel, netdev,
petrm, roopa, songliubraving, syzkaller-bugs, xdp-newbies, yhs
In-Reply-To: <20190225093933.748ceab4@carbon>
On Mon, 2019-02-25 at 09:39 +0100, Jesper Dangaard Brouer wrote:
> On Sat, 23 Feb 2019 01:47:00 +0100
> Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> > On 02/22/2019 10:45 PM, Jakub Kicinski wrote:
> > > On Fri, 22 Feb 2019 12:14:57 -0800, Joe Perches wrote:
> > > > On Fri, 2019-02-22 at 12:01 -0800, Jakub Kicinski wrote:
> > > > > Hi!
> > > > >
> > > > > Seems like something funny is going on with get_maintainer.pl since XDP
> > > > > entry got added. We seem to have been CCed on:
> > > >
> > > > I suggest removing the MAINTAINERS line with
> > > >
> > > > K: xdp
> > > >
> > > > as xdp is a pretty generic file/patch content
> > > > regex match for the K: type
> > > >
> > > > $ git grep --name-only xdp | wc -l
> > > > 236
>
> I'm unsure how K: works, but you grep definitely selects some wrong files.
>
> I tried with "xdp_":
> git grep --name-only xdp_
>
> That does catch all the driver that have XDP support, which is the
> point of the exercise (to catch drivers).
>
> It does contain a couple of false-positives:
> drivers/net/ethernet/neterion/vxge/vxge-traffic.c
> drivers/thunderbolt/tb_msgs.h
> drivers/thunderbolt/xdomain.c
> sound/soc/codecs/rt5670.c
>
> Via the pattern '[^a-z]xdp_' I'm only left with the thunderbolt
> false-positive, as it have a data struct's called tb_xdp_*.
Then it's probably better to use
K: \Wxdp_
and if XDP_ is also to be matched, maybe
K: (?i:\Wxdp_)
^ permalink raw reply
* Re: [PATCH v2 bpf-next 4/9] bpf: add bpf helper bpf_skb_ecn_set_ce
From: Eric Dumazet @ 2019-02-25 16:52 UTC (permalink / raw)
To: Daniel Borkmann, Martin Lau
Cc: Lawrence Brakmo, netdev, Alexei Starovoitov, Kernel Team
In-Reply-To: <c9ebe42a-4fc5-74f5-1b80-30ec45772174@iogearbox.net>
On 02/25/2019 02:10 AM, Daniel Borkmann wrote:
> My understanding is that before doing any writes into skb, we should make
> sure the data area is private to us (and offset in linear data). In tc BPF
> (ingress, egress) we use bpf_try_make_writable() helper for this, others
> like act_{pedit,skbmod} or ovs have similar logic before writing into skb,
> note that in all these cases it's mostly about generic writes, so location
> could also be L4, for example.
>
> Difference of above helper compared to net/sched/sch_*.c instances could
> be that it's i) for the qdisc case it's only on egress INET_ECN_set_ce()
> and that there may be a convention that qdiscs specifically may mangle
> it whereas the helper could be called on ingress and egress and confuse
> other subsystems since they won't see original or race by seeing partially
> updated (invalid) packet.
>
> Eric, have a chance to clarify? Perhaps then would make sense to disallow
> the helper in cgroup ingress path.
Good observations Daniel, thanks for bringing this up.
skb_ensure_writable() seems a big hammer for the case we change some bits in IP header.
TCP cloned packets certainly can have their headers mangled, so maybe
we need to use something using skb_header_cloned() instead of skb_cloned()
^ permalink raw reply
* Re: [net-next 05/14] i40e: fix up 32 bit timespec references
From: Jesse Brandeburg @ 2019-02-25 16:47 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Jeff Kirsher, David Miller, Networking, nhorman, sassmann,
jogreene, jacob.e.keller
In-Reply-To: <CAK8P3a3fSofgDdLZ3S+MaEVz8DHnX6HO6xGiRWfCjQ614Kf4Cg@mail.gmail.com>
On Mon, 25 Feb 2019 15:46:18 +0100
Arnd Bergmann <arnd@arndb.de> wrote:
> I just stumbled over code added by this older patch, and can't make sense
> of the commit description here. Was this an attempt to fix a bug, or
> just a cleanup?
Thanks Arnd for mentioning/posting this! This was an attempt at a clean
up, unfortunately I made a mistake which introduced a bug.
> > - then = ns_to_timespec64(delta);
> > mutex_lock(&pf->tmreg_lock);
> >
> > i40e_ptp_read(pf, &now);
> > - now = timespec64_add(now, then);
> > + timespec64_add_ns(&now, delta);
> > i40e_ptp_write(pf, (const struct timespec64 *)&now);
>
> The problem I noticed here is that 'delta' is a user provided 64-bit
> number from clock_adjtime(), and timespec64_add_ns() performs uses
> a repeated addition instead of a div/mod pair. When the number
> is large, we may end up adding a single second 8 billion times,
> which may take a while even on a fast CPU.
>
> Should the commit 0ac30ce43323 ("i40e: fix up 32 bit timespec
> references") just be reverted?
Interestingly, we just were discussing this fix this past Friday, and
have a patch internally to fix the issue correctly. Either way it
is a commit (revert or fix) so we might as well just send the fix ASAP.
Our team should have something posted to intel-wired-lan/netdev shortly,
and we'll get the patch officially to netdev after a brief test.
^ permalink raw reply
* [PATCH net-next] net: marvell: neta: disable comphy when setting mode
From: Marek Behún @ 2019-02-25 16:43 UTC (permalink / raw)
To: netdev; +Cc: Russell King, Marek Behún
The comphy driver for Armada 3700 by Miquèl Raynal (which is currently
in linux-next) does not actually set comphy mode when phy_set_mode_ext
is called. The mode is set at next call of phy_power_on.
Update the driver to semantics similar to mvpp2: helper
mvneta_comphy_init sets comphy mode and powers it on.
When mode is to be changed in mvneta_mac_config, first power the comphy
off, then call mvneta_comphy_init (which sets the mode to new one).
Only do this when new mode is different from old mode.
This should also work for Armada 38x, since in that comphy driver
methods power_on and power_off are unimplemented.
Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
drivers/net/ethernet/marvell/mvneta.c | 28 ++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/net/ethernet/marvell/mvneta.c b/drivers/net/ethernet/marvell/mvneta.c
index ff275780040f..c0a3718b2e2a 100644
--- a/drivers/net/ethernet/marvell/mvneta.c
+++ b/drivers/net/ethernet/marvell/mvneta.c
@@ -3149,11 +3149,26 @@ static int mvneta_setup_txqs(struct mvneta_port *pp)
return 0;
}
+static int mvneta_comphy_init(struct mvneta_port *pp)
+{
+ int ret;
+
+ if (!pp->comphy)
+ return 0;
+
+ ret = phy_set_mode_ext(pp->comphy, PHY_MODE_ETHERNET,
+ pp->phy_interface);
+ if (ret)
+ return ret;
+
+ return phy_power_on(pp->comphy);
+}
+
static void mvneta_start_dev(struct mvneta_port *pp)
{
int cpu;
- WARN_ON(phy_power_on(pp->comphy));
+ WARN_ON(mvneta_comphy_init(pp));
mvneta_max_rx_size_set(pp, pp->pkt_size);
mvneta_txq_max_tx_size_set(pp, pp->pkt_size);
@@ -3525,12 +3540,15 @@ static void mvneta_mac_config(struct net_device *ndev, unsigned int mode,
if (state->speed == SPEED_2500)
new_ctrl4 |= MVNETA_GMAC4_SHORT_PREAMBLE_ENABLE;
- if (pp->comphy &&
+ if (pp->comphy && pp->phy_interface != state->interface &&
(state->interface == PHY_INTERFACE_MODE_SGMII ||
state->interface == PHY_INTERFACE_MODE_1000BASEX ||
- state->interface == PHY_INTERFACE_MODE_2500BASEX))
- WARN_ON(phy_set_mode_ext(pp->comphy, PHY_MODE_ETHERNET,
- state->interface));
+ state->interface == PHY_INTERFACE_MODE_2500BASEX)) {
+ pp->phy_interface = state->interface;
+
+ WARN_ON(phy_power_off(pp->comphy));
+ WARN_ON(mvneta_comphy_init(pp));
+ }
if (new_ctrl0 != gmac_ctrl0)
mvreg_write(pp, MVNETA_GMAC_CTRL_0, new_ctrl0);
--
2.19.2
^ permalink raw reply related
* Re: [PATCH net] sit: use ipv6_mod_enabled to check if ipv6 is disabled
From: Eric Dumazet @ 2019-02-25 16:39 UTC (permalink / raw)
To: Hangbin Liu; +Cc: netdev, Stefano Brivio, David S . Miller, David Ahern
In-Reply-To: <20190225081759.GP10051@dhcp-12-139.nay.redhat.com>
On 02/25/2019 12:17 AM, Hangbin Liu wrote:
> On Sun, Feb 24, 2019 at 08:24:51PM -0800, Eric Dumazet wrote:
>>
>>
>> On 02/24/2019 08:12 PM, Hangbin Liu wrote:
>>> ipv6_mod_enabled() is more safe and gentle to check if ipv6 is disabled
>>> at running time.
>>>
>>
>>
>> Why is it better exactly ?
>>
>> IPv6 can be enabled on the host, but disabled per device
>>
>> /proc/sys/net/ipv6/conf/{name}/disable_ipv6
>
> Sorry, it looks I didn't make it clear in the commit description.
> This issue only occurs when IPv6 is disabled at boot time as there is
> no IPv6 route entry. Disable ipv6 on specific interface is not affected.
> So check ipv6_mod_enabled() is enough and we don't need to worry about
> the rcu_read_lock or the dev status.
>
> Should I update the commit description?
Certainly. Are you telling us skb->dev could be NULL here ?
Because rcu_read_lock() should already be asserted.
^ permalink raw reply
* [PATCH v2 2/2] NETWORKING: avoid use IPCB in cipso_v4_error
From: Nazarov Sergey @ 2019-02-25 16:27 UTC (permalink / raw)
To: David Miller
Cc: paul@paul-moore.com, netdev@vger.kernel.org,
linux-security-module@vger.kernel.org, kuznet@ms2.inr.ac.ru,
yoshfuji@linux-ipv6.org
In-Reply-To: <20190224.173328.1032826011262803545.davem@davemloft.net>
Extract IP options in cipso_v4_error and use __icmp_send.
Signed-off-by: Sergey Nazarov <s-nazarov@yandex.ru>
---
include/net/ip.h | 2 ++
net/ipv4/cipso_ipv4.c | 17 +++++++++++++++--
net/ipv4/ip_options.c | 22 +++++++++++++++++-----
3 files changed, 34 insertions(+), 7 deletions(-)
diff --git a/include/net/ip.h b/include/net/ip.h
index 8866bfc..f0e8d06 100644
--- a/include/net/ip.h
+++ b/include/net/ip.h
@@ -667,6 +667,8 @@ static inline int ip_options_echo(struct net *net, struct ip_options *dopt,
}
void ip_options_fragment(struct sk_buff *skb);
+int __ip_options_compile(struct net *net, struct ip_options *opt,
+ struct sk_buff *skb, __be32 *info);
int ip_options_compile(struct net *net, struct ip_options *opt,
struct sk_buff *skb);
int ip_options_get(struct net *net, struct ip_options_rcu **optp,
diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index 777fa3b..eff86a7 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -1735,13 +1735,26 @@ int cipso_v4_validate(const struct sk_buff *skb, unsigned char **option)
*/
void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway)
{
+ unsigned char optbuf[sizeof(struct ip_options) + 40];
+ struct ip_options *opt = (struct ip_options *)optbuf;
+
if (ip_hdr(skb)->protocol == IPPROTO_ICMP || error != -EACCES)
return;
+ /*
+ * We might be called above the IP layer,
+ * so we can not use icmp_send and IPCB here.
+ */
+
+ memset(opt, 0, sizeof(struct ip_options));
+ opt->optlen = ip_hdr(skb)->ihl*4 - sizeof(struct iphdr);
+ if (__ip_options_compile(dev_net(skb->dev), opt, skb, NULL))
+ return;
+
if (gateway)
- icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0);
+ __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_NET_ANO, 0, opt);
else
- icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0);
+ __icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_ANO, 0, opt);
}
/**
diff --git a/net/ipv4/ip_options.c b/net/ipv4/ip_options.c
index ed194d4..32a3504 100644
--- a/net/ipv4/ip_options.c
+++ b/net/ipv4/ip_options.c
@@ -251,8 +251,9 @@ static void spec_dst_fill(__be32 *spec_dst, struct sk_buff *skb)
* If opt == NULL, then skb->data should point to IP header.
*/
-int ip_options_compile(struct net *net,
- struct ip_options *opt, struct sk_buff *skb)
+int __ip_options_compile(struct net *net,
+ struct ip_options *opt, struct sk_buff *skb,
+ __be32 *info)
{
__be32 spec_dst = htonl(INADDR_ANY);
unsigned char *pp_ptr = NULL;
@@ -468,11 +469,22 @@ int ip_options_compile(struct net *net,
return 0;
error:
- if (skb) {
- icmp_send(skb, ICMP_PARAMETERPROB, 0, htonl((pp_ptr-iph)<<24));
- }
+ if (info)
+ *info = htonl((pp_ptr-iph)<<24);
return -EINVAL;
}
+
+int ip_options_compile(struct net *net,
+ struct ip_options *opt, struct sk_buff *skb)
+{
+ int ret;
+ __be32 info;
+
+ ret = __ip_options_compile(net, opt, skb, &info);
+ if (ret != 0 && skb)
+ icmp_send(skb, ICMP_PARAMETERPROB, 0, info);
+ return ret;
+}
EXPORT_SYMBOL(ip_options_compile);
/*
---
^ permalink raw reply related
* [PATCH v2 1/2] NETWORKING: avoid use IPCB in cipso_v4_error
From: Nazarov Sergey @ 2019-02-25 16:24 UTC (permalink / raw)
To: David Miller
Cc: paul@paul-moore.com, netdev@vger.kernel.org,
linux-security-module@vger.kernel.org, kuznet@ms2.inr.ac.ru,
yoshfuji@linux-ipv6.org
In-Reply-To: <20190224.173328.1032826011262803545.davem@davemloft.net>
Add __icmp_send function having ip_options struct parameter
Signed-off-by: Sergey Nazarov <s-nazarov@yandex.ru>
---
include/net/icmp.h | 9 ++++++++-
net/ipv4/icmp.c | 7 ++++---
2 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/include/net/icmp.h b/include/net/icmp.h
index 6ac3a5b..e0f709d 100644
--- a/include/net/icmp.h
+++ b/include/net/icmp.h
@@ -22,6 +22,7 @@
#include <net/inet_sock.h>
#include <net/snmp.h>
+#include <net/ip.h>
struct icmp_err {
int errno;
@@ -39,7 +40,13 @@ struct icmp_err {
struct sk_buff;
struct net;
-void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info);
+void __icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info,
+ const struct ip_options *opt);
+static inline void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
+{
+ __icmp_send(skb_in, type, code, info, &IPCB(skb_in)->opt);
+}
+
int icmp_rcv(struct sk_buff *skb);
int icmp_err(struct sk_buff *skb, u32 info);
int icmp_init(void);
diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index 065997f..3f24414 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -570,7 +570,8 @@ static void icmp_reply(struct icmp_bxm *icmp_param, struct sk_buff *skb)
* MUST reply to only the first fragment.
*/
-void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
+void __icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info,
+ const struct ip_options *opt)
{
struct iphdr *iph;
int room;
@@ -691,7 +692,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
iph->tos;
mark = IP4_REPLY_MARK(net, skb_in->mark);
- if (ip_options_echo(net, &icmp_param.replyopts.opt.opt, skb_in))
+ if (__ip_options_echo(net, &icmp_param.replyopts.opt.opt, skb_in, opt))
goto out_unlock;
@@ -742,7 +743,7 @@ void icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info)
local_bh_enable();
out:;
}
-EXPORT_SYMBOL(icmp_send);
+EXPORT_SYMBOL(__icmp_send);
static void icmp_socket_deliver(struct sk_buff *skb, u32 info)
---
^ permalink raw reply related
* RE: [PATCH net-next v3 4/4] dt-bindings: net: freescale: enetc: Add connection bindings for ENETC ethernet nodes
From: Claudiu Manoil @ 2019-02-25 16:16 UTC (permalink / raw)
To: Rob Herring
Cc: Shawn Guo, Leo Li, David S . Miller, Alexandru Marginean,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <20190222233732.GA1518@bogus>
>-----Original Message-----
>From: Rob Herring <robh@kernel.org>
>Sent: Saturday, February 23, 2019 1:38 AM
>To: Claudiu Manoil <claudiu.manoil@nxp.com>
>Cc: Shawn Guo <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; David S .
>Miller <davem@davemloft.net>; Alexandru Marginean
><alexandru.marginean@nxp.com>; linux-arm-kernel@lists.infradead.org;
>devicetree@vger.kernel.org; netdev@vger.kernel.org; linux-
>kernel@vger.kernel.org
>Subject: Re: [PATCH net-next v3 4/4] dt-bindings: net: freescale: enetc: Add
>connection bindings for ENETC ethernet nodes
>
>On Fri, Feb 22, 2019 at 05:04:19PM +0200, Claudiu Manoil wrote:
>> Define connection bindings (external PHY connections and internal
>> links) for the ENETC on-chip ethernet controllers.
>>
>> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
>> ---
>> v3 - added this patch to the set
>>
>> .../devicetree/bindings/net/fsl-enetc.txt | 109 +++++++++++++++++++++
>> 1 file changed, 109 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/net/fsl-enetc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/fsl-enetc.txt
>> b/Documentation/devicetree/bindings/net/fsl-enetc.txt
>> new file mode 100644
>> index 0000000..2fbb998
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/fsl-enetc.txt
>> @@ -0,0 +1,109 @@
>> +* ENETC ethernet nodes - external connection bindings
>> +
>> +The ENETC ethernet controllers are PCIe integrated endpoints (IEPs),
>> +on-chip devices discoverable as standard PCIe endpoints, integrated
>> +into Freescale SoCs. The ENETC devices are self contained, the
>> +information needed for device initialization is available in hardware
>> +(PCIe ECAM area). However, depending on board design, their external
>> +connections are configurable.
>> +As usual for SoCs, device tree nodes may be used to define these
>> +external connections. The rest of the document specifies how
>> +external connections for ENETC ethernet controllers may be defined
>> +via device tree nodes.
>> +
>> +Silicon (SoC) availability (<SoC name>: <SoC DT path/name>)
>> + - LS1028A: [arch/arm64] [...]freescale/fsl-ls1028a.dtsi
>
>This doesn't belong in bindings.
>
>> +
>> +
>> +* ENETC nodes
>> +
>> +Defined in the SoC device tree as "pci" child nodes of the
>> +"pci-host-ecam-generic" compatible "pcie" parent node also known as
>> +the Integrated Endpoint Root Complex (IERC) SoC node.
>
>The host controller attachment is also outside the scope of this binding.
>
>> +
>> +Structure - example (LS1028A):
>> +
>> + pcie@1f0000000 {
>> + compatible = "pci-host-ecam-generic";
>> + device_type = "pci";
>> + ...
>> + enetc_port0: pci@0,0 {
>
>The node name 'pci' is reserved for bridges. This should match the device class if
>possible (ethernet).
>
>> + reg = <0x000000 0 0 0 0>;
>> + };
>> + enetc_port1: pci@0,1 {
>> + reg = <0x000100 0 0 0 0>;
>> + };
>> + ...
>> + }
>> +
>> +Each ENETC node has a device number and a function number (expressed
>> +by its "reg" property and pci node name, i.e. "pci@0,1" represents
>> +device number 0 and functions number 1). Only the standard pci "reg"
>> +property is needed here.
>
>There should be a compatible too.
[...]
Ok to simplifying the text and document strictly the enetc device nodes as
"ethernet" nodes, like "ethernet@<dev_no>,<fcn_no>" (i.e ethernet@0,1).
But what would be the compatible string needed for?
The ethernet device driver doesn't need it, probing is done by the pci system.
Is it ok to use a generic name for the compatible string, like, "fsl,enetc", just
to indicate that the relevant driver is located at ethernet/freescale/enetc/
dir in the source code? (under drivers/net, of course)
Thanks.
Claudiu
^ permalink raw reply
* Re: [PATCH net] sctp: get sctphdr by offset in sctp_compute_cksum
From: Xin Long @ 2019-02-25 16:15 UTC (permalink / raw)
To: Neil Horman; +Cc: LKML, network dev, linux-sctp, davem, Marcelo Ricardo Leitner
In-Reply-To: <20190225140720.GC30699@hmswarspite.think-freely.org>
On Mon, Feb 25, 2019 at 10:08 PM Neil Horman <nhorman@tuxdriver.com> wrote:
>
> On Mon, Feb 25, 2019 at 09:20:44PM +0800, Xin Long wrote:
> > On Mon, Feb 25, 2019 at 8:47 PM Neil Horman <nhorman@tuxdriver.com> wrote:
> > >
> > > On Mon, Feb 25, 2019 at 07:25:37PM +0800, Xin Long wrote:
> > > > sctp_hdr(skb) only works when skb->transport_header is set properly.
> > > >
> > > > But in the path of nf_conntrack_in: sctp_packet() -> sctp_error()
> > > >
> > > > skb->transport_header is not guaranteed to be right value for sctp.
> > > > It will cause to fail to check the checksum for sctp packets.
> > > >
> > > > So fix it by using offset, which is always right in all places.
> > > >
> > > > Fixes: e6d8b64b34aa ("net: sctp: fix and consolidate SCTP checksumming code")
> > > > Reported-by: Li Shuang <shuali@redhat.com>
> > > > Signed-off-by: Xin Long <lucien.xin@gmail.com>
> > > > ---
> > > > include/net/sctp/checksum.h | 2 +-
> > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/include/net/sctp/checksum.h b/include/net/sctp/checksum.h
> > > > index 32ee65a..1c6e6c0 100644
> > > > --- a/include/net/sctp/checksum.h
> > > > +++ b/include/net/sctp/checksum.h
> > > > @@ -61,7 +61,7 @@ static inline __wsum sctp_csum_combine(__wsum csum, __wsum csum2,
> > > > static inline __le32 sctp_compute_cksum(const struct sk_buff *skb,
> > > > unsigned int offset)
> > > > {
> > > > - struct sctphdr *sh = sctp_hdr(skb);
> > > > + struct sctphdr *sh = (struct sctphdr *)(skb->data + offset);
> > > > const struct skb_checksum_ops ops = {
> > > > .update = sctp_csum_update,
> > > > .combine = sctp_csum_combine,
> > > > --
> > > > 2.1.0
> > > >
> > > >
> > > Shouldn't you use skb_set_transport_header and skb_transport_header here?
> > you mean:
> > skb_set_transport_header(skb, offset);
> > sh = sctp_hdr(skb);
> > ?
> >
> > There's no place counting on here to set transport_header.
> > It will be a kinda redundant job, yet skb is 'const'.
> >
> I'm not sure what you mean by "theres no place counting here". We have the
> transport header offset, and you're doing the exact same computation that that
> function does. It seems like we should use it in case the underlying
> implementation changes.
1. skb_set_transport_header() and sctp_hdr() are like:
skb->transport_header = skb->data - skb->head;
skb->transport_header += offset
sh = skb->head + skb->transport_header;
2. in this patch:
sh = (struct sctphdr *)(skb->data + offset); only
I think the 2nd one is better.
I feel it's weird to set transport_header here if it's only for
sctp_hdr(skb) in here.
As for "underlying implementation changes", I don't know exactly the case
but there are quite a few places doing things like:
*hdr = (struct *hdr *)(skb->data + hdroff);
I'd think it's safe. no?
>
> I understand what you are saying regarding the use of a const variable there,
> but perhaps thats an argument for removing the const storage classifier. Better
> still, it would be good to figure out why all paths to this function don't
> already set the transport header offset to begin with (addressing your redundant
> comment)
The issue was reported when going to nf_conntrack by br_netfilter's
bridge-nf-call-iptables.
As you can see on nf_conntrack_in() path, even iphdr is got by:
iph = skb_header_pointer(skb, nhoff, sizeof(_iph), &_iph);
It's impossible to set skb->transport_header when we're not sure iphdr
in linearized memory.
^ permalink raw reply
* [PATCH net-next 0/2] net: phy: marvell10g: Clean .get_features by using C45 helpers
From: Maxime Chevallier @ 2019-02-25 16:14 UTC (permalink / raw)
To: davem
Cc: Maxime Chevallier, netdev, linux-kernel, Andrew Lunn,
Florian Fainelli, Heiner Kallweit, Russell King, linux-arm-kernel,
Antoine Tenart, thomas.petazzoni, gregory.clement, miquel.raynal
Recent work on C45 helpers by Heiner made the
genphy_c45_pma_read_abilities function generic enough to use as a
default .get_featutes implementation.
This series removes the remaining redundant code in
mv3310_get_features(), and makes the 2110 PHY use
genphy_c45_pma_read_abilities() directly, since it doesn't have the
issue with the wrong abilities being reported.
Maxime Chevallier (2):
net: phy: marvell10g: Let genphy_c45_pma_read_abilities set Aneg bit
net: phy: marvell10g: Use the generic C45 helper to read the 2110
features
drivers/net/phy/marvell10g.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
--
2.19.2
^ permalink raw reply
* [PATCH net-next 1/2] net: phy: marvell10g: Let genphy_c45_pma_read_abilities set Aneg bit
From: Maxime Chevallier @ 2019-02-25 16:14 UTC (permalink / raw)
To: davem
Cc: Maxime Chevallier, netdev, linux-kernel, Andrew Lunn,
Florian Fainelli, Heiner Kallweit, Russell King, linux-arm-kernel,
Antoine Tenart, thomas.petazzoni, gregory.clement, miquel.raynal
In-Reply-To: <20190225161407.19510-1-maxime.chevallier@bootlin.com>
The genphy_c45_pma_read_abilities helper now sets the Autoneg ability
in phydev->supported according to what the AN MMD reports.
We therefore don't need to manually do that in mv3310_get_features().
Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Suggested-by: Heiner Kallweit <hkallweit1@gmail.com>
---
drivers/net/phy/marvell10g.c | 10 ----------
1 file changed, 10 deletions(-)
diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
index 580e91deadbc..ab71da55c1e0 100644
--- a/drivers/net/phy/marvell10g.c
+++ b/drivers/net/phy/marvell10g.c
@@ -268,16 +268,6 @@ static int mv3310_get_features(struct phy_device *phydev)
{
int ret, val;
- if (phydev->c45_ids.devices_in_package & MDIO_DEVS_AN) {
- val = phy_read_mmd(phydev, MDIO_MMD_AN, MDIO_STAT1);
- if (val < 0)
- return val;
-
- if (val & MDIO_AN_STAT1_ABLE)
- linkmode_set_bit(ETHTOOL_LINK_MODE_Autoneg_BIT,
- phydev->supported);
- }
-
ret = genphy_c45_pma_read_abilities(phydev);
if (ret)
return ret;
--
2.19.2
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox