* [PATCH] selftests: fib_tests: sleep after changing carrier. again.
From: Thadeu Lima de Souza Cascardo @ 2019-02-22 10:27 UTC (permalink / raw)
To: netdev; +Cc: linux-kselftest, Thadeu Lima de Souza Cascardo, David Ahern
Just like commit e2ba732a1681 ("selftests: fib_tests: sleep after
changing carrier"), wait one second to allow linkwatch to propagate the
carrier change to the stack.
There are two sets of carrier tests. The first slept after the carrier
was set to off, and when the second set ran, it was likely that the
linkwatch would be able to run again without much delay, reducing the
likelihood of a race. However, if you run 'fib_tests.sh -t carrier' on a
loop, you will quickly notice the failures.
Sleeping on the second set of tests make the failures go away.
Cc: David Ahern <dsahern@gmail.com>
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@canonical.com>
---
tools/testing/selftests/net/fib_tests.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/net/fib_tests.sh b/tools/testing/selftests/net/fib_tests.sh
index 802b4af18729..1080ff55a788 100755
--- a/tools/testing/selftests/net/fib_tests.sh
+++ b/tools/testing/selftests/net/fib_tests.sh
@@ -388,6 +388,7 @@ fib_carrier_unicast_test()
set -e
$IP link set dev dummy0 carrier off
+ sleep 1
set +e
echo " Carrier down"
--
2.20.1
^ permalink raw reply related
* Re: [PATCH 1/5] net: dsa: fix a leaked reference by adding a missing of_node_put
From: Linus Walleij @ 2019-02-22 10:38 UTC (permalink / raw)
To: Wen Yang
Cc: Andrew Lunn, Vivien Didelot, Florian Fainelli, David S. Miller,
netdev, linux-kernel@vger.kernel.org, Alexandre Belloni,
UNGLinuxDriver, Felix Fietkau, Lorenzo Bianconi, Kalle Valo,
Matthias Brugger, linux-wireless, Linux ARM,
moderated list:ARM/Mediatek SoC support, anirudh, John Linn,
Michal Simek, wang.yi59
In-Reply-To: <1550819742-32155-1-git-send-email-wen.yang99@zte.com.cn>
On Fri, Feb 22, 2019 at 8:15 AM Wen Yang <wen.yang99@zte.com.cn> wrote:
> The call to of_get_child_by_name returns a node pointer with refcount
> incremented thus it must be explicitly decremented after the last
> usage.
>
> Detected by coccinelle with the following warnings:
> ./drivers/net/dsa/rtl8366rb.c:510:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 501, but without a corresponding object release within this function.
> ./drivers/net/dsa/rtl8366rb.c:518:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 501, but without a corresponding object release within this function.
> ./drivers/net/dsa/rtl8366rb.c:540:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 501, but without a corresponding object release within this function.
> ./drivers/net/dsa/rtl8366rb.c:548:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 501, but without a corresponding object release within this function.
> ./drivers/net/dsa/rtl8366rb.c:556:2-8: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 501, but without a corresponding object release within this function.
> ./drivers/net/dsa/rtl8366rb.c:561:1-7: ERROR: missing of_node_put; acquired a node pointer with refcount incremented on line 501, but without a corresponding object release within this function.
>
> Signed-off-by: Wen Yang <wen.yang99@zte.com.cn>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Andrew Lunn <andrew@lunn.ch>
> Cc: Vivien Didelot <vivien.didelot@gmail.com>
> Cc: Florian Fainelli <f.fainelli@gmail.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: netdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Yours,
Linus Walleij
^ permalink raw reply
* Re: [Bridge] [PATCH net] net: bridge: remove ipv6 zero address check in mcast queries
From: Nikolay Aleksandrov @ 2019-02-22 11:16 UTC (permalink / raw)
To: Hangbin Liu
Cc: Linus Lüssing, netdev, roopa, bridge, davem, yinxu,
Sebastian Gottschall
In-Reply-To: <20190222075705.GN10051@dhcp-12-139.nay.redhat.com>
On 22/02/2019 09:57, Hangbin Liu wrote:
> Hi Nikolay,
>
> On Thu, Feb 21, 2019 at 03:20:14PM +0200, Nikolay Aleksandrov wrote:
>>>
>>> Yes, I agree. But this "regression" could be fixed by setting up correct
>>> switch configuration. See more explains below.
>>>
>>
>> That is irrelevant, if the setup once worked we should not break it unless
>> it's in RFC requirement violation and RFC 4541 is only suggestive, not required.
>
> Thanks for your reply. I just noticed the RFC4541 category is informational.
>
>>> "because this would cause the Queries to be seen as coming from a newly
>>> elected Querier" means other address could be elected as a Querier but
>>> "0.0.0.0" should not.
>>>
>>
>> But this change hasn't been incorporated, has it ? A 0.0.0.0 address currently
>> will always win the election and silence all of the rest. Current bridge state
>> is simply broken for some cases because of that.
>
> Yes. I agree. I realized linus also said
>
> """
> However, one of the two options seems to be necessary. Either
> reverting the patch for the IGMP part, too. Or Ignoring 0.0.0.0
> sources for querier eletcion and presence detection.
> """
>
>>
>> Removing 0.0.0.0 from the election will effectively disable snooping even if there's
>> a configured bridge unless it has an address. You can see that this will end up in
>> people having suddenly their multicast flooded with current setups, right ?
>
> Yes
>
>> Any big behaviour change like that should be optional, but I don't think we need
>> another option as this is not so big of a deal because we're not breaking any
>> required behaviour.
>
> Just a little curious, RFC 3376 said the General Queries are sent from multicast
> routers. I think a router *should* has a IP address, isn't it?
>
> RFC 4541 also suggested:
>
> If the switch is not the Querier, it should use the 'all-zeros' IP
> Source Address in these proxy queries (even though some hosts may
> elect to not process queries with a 0.0.0.0 IP Source Address).
> When such proxy queries are received, they must not be included in
> the Querier election process.
>
> And what I got is most vendors apply this suggestion.
>
>> In case you decide to follow the option path, please use the new boolopt api to avoid
>> adding new fields to the bridge, this should be an on/off thing. I still vote for a
>> revert though.
>
> For consistency with other vendors and rfc, I would prefer to remove zero address election.
> For compatibility with previous users, I'm also OK to revert it.
> > Regards
> Hangbin
>
Hi,
In this case I'd suggest the following course of action:
- For -net/-stable revert the change since backporting new options is a
no-go and we need to restore the bridge state
- After -net is merged in net-next, for net-next if you'd like add it
as an option and also exclude it from elections when the option is
enabled (for example something like multicast_nonzero_src_querier).
Just please use the boolopt api and don't add new fields/attr ids.
Obviously by default this option will be off to be backwards
compatible and avoid surprise mcast flood.
Or just leave it reverted. :)
Thanks,
Nik
^ permalink raw reply
* [PATCH net-next] net: dsa: add missing of_node_put
From: Himadri Pandya @ 2019-02-22 11:18 UTC (permalink / raw)
To: andrew
Cc: vivien.didelot, f.fainelli, davem, netdev, linux-kernel,
Himadri Pandya
Decrement the reference count on port while returning out of the
loop. Issue identified by Coccinelle.
Signed-off-by: Himadri Pandya <himadri18.07@gmail.com>
---
net/dsa/dsa2.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/net/dsa/dsa2.c b/net/dsa/dsa2.c
index a1917025e155..396e7433dd8f 100644
--- a/net/dsa/dsa2.c
+++ b/net/dsa/dsa2.c
@@ -624,19 +624,25 @@ static int dsa_switch_parse_ports_of(struct dsa_switch *ds,
for_each_available_child_of_node(ports, port) {
err = of_property_read_u32(port, "reg", ®);
if (err)
- return err;
+ goto put_port;
- if (reg >= ds->num_ports)
- return -EINVAL;
+ if (reg >= ds->num_ports) {
+ err = -EINVAL;
+ goto put_port;
+ }
dp = &ds->ports[reg];
err = dsa_port_parse_of(dp, port);
if (err)
- return err;
+ goto put_port;
}
return 0;
+
+put_port:
+ of_node_put(port);
+ return err;
}
static int dsa_switch_parse_member_of(struct dsa_switch *ds,
--
2.17.1
^ permalink raw reply related
* Re: [PATCH 2/2] doc: add phylink documentation to the networking book
From: Russell King - ARM Linux admin @ 2019-02-22 11:20 UTC (permalink / raw)
To: Randy Dunlap; +Cc: linux-doc, netdev, David S. Miller, Jonathan Corbet
In-Reply-To: <f002402d-fb27-f697-f07d-de3cdff41f40@infradead.org>
Hi Randy,
Sorry for the delay.
On Wed, Feb 13, 2019 at 08:00:29PM -0800, Randy Dunlap wrote:
> On 2/5/19 7:58 AM, Russell King wrote:
> > +uses in-band mode, where results from the PHYs negotiation are passed
>
> PHY's
...
> > + Manipulation of the PHY's supported/advertised happens within phylink
>
> PHYs
Either one way or the other. "negotiation of the PHY", "supported/
advertised (link modes) of the PHY". Both are posessive, so the first
is a mistake but I don't agree with removing the posessive apostrophy
in the second case.
> > +For information describing the SFP cage in DT, please see the binding
> > +documentation in the kernel source tree
> > +``Documentation/devicetree/bindings/net/sff,sfp.txt``
> oh, so SFP means "Small Form-factor Pluggable".
>
> I see that this source file:
> ./drivers/net/ethernet/intel/i40e/i40e_virtchnl_pf.c:1902:
>
> seems to imply that SFP means "single function per port (SFP) mode":
>
> dev_err(&pf->pdev->dev,
> "VF %d requested polling mode: this feature is supported only when the device is running in single function per port (SFP) mode\n",
> vf->vf_id);
Software engineers endlessly like to create TLAs, so it could mean
both, but in this case it's the official definition from the SFF/
SNIA documents, e.g.:
INF-8074i Specification for
SFP (Small Formfactor Pluggable) Transceiver
note, no hyphen!
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply
* [PATCH net-next v2 1/2] net: phylink: update mac_config() documentation
From: Russell King @ 2019-02-22 11:31 UTC (permalink / raw)
To: linux-doc, netdev
A detail for mac_config() had been missed in the documentation for the
method - it is expected that the method will update the MAC to the
settings, rather than completely reprogram the MAC on each call.
Update the documentation for this method for this detail.
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
include/linux/phylink.h | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index f57059e4353f..6411c624f63a 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -149,6 +149,13 @@ int mac_link_state(struct net_device *ndev,
* configuration word. Nothing is advertised by the MAC. The MAC is
* responsible for reading the configuration word and configuring
* itself accordingly.
+ *
+ * Implementations are expected to update the MAC to reflect the
+ * requested settings - i.o.w., if nothing has changed between two
+ * calls, no action is expected. If only flow control settings have
+ * changed, flow control should be updated *without* taking the link
+ * down. This "update" behaviour is critical to avoid bouncing the
+ * link up status.
*/
void mac_config(struct net_device *ndev, unsigned int mode,
const struct phylink_link_state *state);
--
2.7.4
^ permalink raw reply related
* [PATCH net-next v2 2/2] doc: add phylink documentation to the networking book
From: Russell King @ 2019-02-22 11:31 UTC (permalink / raw)
To: linux-doc, netdev; +Cc: David S. Miller, Jonathan Corbet
Add some phylink documentation to the networking book detailing how
to convert network drivers from phylib to phylink.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
v2: updated with comments from Randy and updated to apply to net-next
Documentation/networking/index.rst | 1 +
Documentation/networking/sfp-phylink.rst | 268 +++++++++++++++++++++++++++++++
2 files changed, 269 insertions(+)
create mode 100644 Documentation/networking/sfp-phylink.rst
diff --git a/Documentation/networking/index.rst b/Documentation/networking/index.rst
index 9a32451cd201..b08cf145d5eb 100644
--- a/Documentation/networking/index.rst
+++ b/Documentation/networking/index.rst
@@ -31,6 +31,7 @@ Linux Networking Documentation
failover
net_failover
phy
+ sfp-phylink
alias
bridge
snmp_counter
diff --git a/Documentation/networking/sfp-phylink.rst b/Documentation/networking/sfp-phylink.rst
new file mode 100644
index 000000000000..5bd26cb07244
--- /dev/null
+++ b/Documentation/networking/sfp-phylink.rst
@@ -0,0 +1,268 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+=======
+phylink
+=======
+
+Overview
+========
+
+phylink is a mechanism to support hot-pluggable networking modules
+without needing to re-initialise the adapter on hot-plug events.
+
+phylink supports conventional phylib-based setups, fixed link setups
+and SFP (Small Formfactor Pluggable) modules at present.
+
+Modes of operation
+==================
+
+phylink has several modes of operation, which depend on the firmware
+settings.
+
+1. PHY mode
+
+ In PHY mode, we use phylib to read the current link settings from
+ the PHY, and pass them to the MAC driver. We expect the MAC driver
+ to configure exactly the modes that are specified without any
+ negotiation being enabled on the link.
+
+2. Fixed mode
+
+ Fixed mode is the same as PHY mode as far as the MAC driver is
+ concerned.
+
+3. In-band mode
+
+ In-band mode is used with 802.3z, SGMII and similar interface modes,
+ and we are expecting to use and honor the in-band negotiation or
+ control word sent across the serdes channel.
+
+By example, what this means is that:
+
+.. code-block:: none
+
+ ð {
+ phy = <&phy>;
+ phy-mode = "sgmii";
+ };
+
+does not use in-band SGMII signalling. The PHY is expected to follow
+exactly the settings given to it in its :c:func:`mac_config` function.
+The link should be forced up or down appropriately in the
+:c:func:`mac_link_up` and :c:func:`mac_link_down` functions.
+
+.. code-block:: none
+
+ ð {
+ managed = "in-band-status";
+ phy = <&phy>;
+ phy-mode = "sgmii";
+ };
+
+uses in-band mode, where results from the PHY's negotiation are passed
+to the MAC through the SGMII control word, and the MAC is expected to
+acknowledge the control word. The :c:func:`mac_link_up` and
+:c:func:`mac_link_down` functions must not force the MAC side link
+up and down.
+
+Rough guide to converting a network driver to sfp/phylink
+=========================================================
+
+This guide briefly describes how to convert a network driver from
+phylib to the sfp/phylink support. Please send patches to improve
+this documentation.
+
+1. Optionally split the network driver's phylib update function into
+ three parts dealing with link-down, link-up and reconfiguring the
+ MAC settings. This can be done as a separate preparation commit.
+
+ An example of this preparation can be found in git commit fc548b991fb0.
+
+2. Replace::
+
+ select FIXED_PHY
+ select PHYLIB
+
+ with::
+
+ select PHYLINK
+
+ in the driver's Kconfig stanza.
+
+3. Add::
+
+ #include <linux/phylink.h>
+
+ to the driver's list of header files.
+
+4. Add::
+
+ struct phylink *phylink;
+
+ to the driver's private data structure. We shall refer to the
+ driver's private data pointer as ``priv`` below, and the driver's
+ private data structure as ``struct foo_priv``.
+
+5. Replace the following functions:
+
+ .. flat-table::
+ :header-rows: 1
+ :widths: 1 1
+ :stub-columns: 0
+
+ * - Original function
+ - Replacement function
+ * - phy_start(phydev)
+ - phylink_start(priv->phylink)
+ * - phy_stop(phydev)
+ - phylink_stop(priv->phylink)
+ * - phy_mii_ioctl(phydev, ifr, cmd)
+ - phylink_mii_ioctl(priv->phylink, ifr, cmd)
+ * - phy_ethtool_get_wol(phydev, wol)
+ - phylink_ethtool_get_wol(priv->phylink, wol)
+ * - phy_ethtool_set_wol(phydev, wol)
+ - phylink_ethtool_set_wol(priv->phylink, wol)
+ * - phy_disconnect(phydev)
+ - phylink_disconnect_phy(priv->phylink)
+
+ Please note that some of these functions must be called under the
+ rtnl lock, and will warn if not. This will normally be the case,
+ except if these are called from the driver suspend/resume paths.
+
+6. Add/replace ksettings get/set methods with:
+
+ .. code-block:: c
+
+ static int foo_ethtool_set_link_ksettings(struct net_device *dev,
+ const struct ethtool_link_ksettings *cmd)
+ {
+ struct foo_priv *priv = netdev_priv(dev);
+
+ return phylink_ethtool_ksettings_set(priv->phylink, cmd);
+ }
+
+ static int foo_ethtool_get_link_ksettings(struct net_device *dev,
+ struct ethtool_link_ksettings *cmd)
+ {
+ struct foo_priv *priv = netdev_priv(dev);
+
+ return phylink_ethtool_ksettings_get(priv->phylink, cmd);
+ }
+
+7. Replace the call to:
+
+ phy_dev = of_phy_connect(dev, node, link_func, flags, phy_interface);
+
+ and associated code with a call to:
+
+ err = phylink_of_phy_connect(priv->phylink, node, flags);
+
+ For the most part, ``flags`` can be zero; these flags are passed to
+ the of_phy_attach() inside this function call if a PHY is specified
+ in the DT node ``node``.
+
+ ``node`` should be the DT node which contains the network phy property,
+ fixed link properties, and will also contain the sfp property.
+
+ The setup of fixed links should also be removed; these are handled
+ internally by phylink.
+
+ of_phy_connect() was also passed a function pointer for link updates.
+ This function is replaced by a different form of MAC updates
+ described below in (8).
+
+ Manipulation of the PHY's supported/advertised happens within phylink
+ based on the validate callback, see below in (8).
+
+ Note that the driver no longer needs to store the ``phy_interface``,
+ and also note that ``phy_interface`` becomes a dynamic property,
+ just like the speed, duplex etc. settings.
+
+ Finally, note that the MAC driver has no direct access to the PHY
+ anymore; that is because in the phylink model, the PHY can be
+ dynamic.
+
+8. Add a :c:type:`struct phylink_mac_ops <phylink_mac_ops>` instance to
+ the driver, which is a table of function pointers, and implement
+ these functions. The old link update function for
+ :c:func:`of_phy_connect` becomes three methods: :c:func:`mac_link_up`,
+ :c:func:`mac_link_down`, and :c:func:`mac_config`. If step 1 was
+ performed, then the functionality will have been split there.
+
+ It is important that if in-band negotiation is used,
+ :c:func:`mac_link_up` and :c:func:`mac_link_down` do not prevent the
+ in-band negotiation from completing, since these functions are called
+ when the in-band link state changes - otherwise the link will never
+ come up.
+
+ The :c:func:`validate` method should mask the supplied supported mask,
+ and ``state->advertising`` with the supported ethtool link modes.
+ These are the new ethtool link modes, so bitmask operations must be
+ used. For an example, see drivers/net/ethernet/marvell/mvneta.c.
+
+ The :c:func:`mac_link_state` method is used to read the link state
+ from the MAC, and report back the settings that the MAC is currently
+ using. This is particularly important for in-band negotiation
+ methods such as 1000base-X and SGMII.
+
+ The :c:func:`mac_config` method is used to update the MAC with the
+ requested state, and must avoid unnecessarily taking the link down
+ when making changes to the MAC configuration. This means the
+ function should modify the state and only take the link down when
+ absolutely necessary to change the MAC configuration. An example
+ of how to do this can be found in :c:func:`mvneta_mac_config` in
+ drivers/net/ethernet/marvell/mvneta.c.
+
+ For further information on these methods, please see the inline
+ documentation in :c:type:`struct phylink_mac_ops <phylink_mac_ops>`.
+
+9. Remove calls to of_parse_phandle() for the PHY,
+ of_phy_register_fixed_link() for fixed links etc. from the probe
+ function, and replace with:
+
+ .. code-block:: c
+
+ struct phylink *phylink;
+
+ phylink = phylink_create(dev, node, phy_mode, &phylink_ops);
+ if (IS_ERR(phylink)) {
+ err = PTR_ERR(phylink);
+ fail probe;
+ }
+
+ priv->phylink = phylink;
+
+ and arrange to destroy the phylink in the probe failure path as
+ appropriate and the removal path too by calling:
+
+ .. code-block:: c
+
+ phylink_destroy(priv->phylink);
+
+10. Arrange for MAC link state interrupts to be forwarded into
+ phylink, via:
+
+ .. code-block:: c
+
+ phylink_mac_change(priv->phylink, link_is_up);
+
+ where ``link_is_up`` is true if the link is currently up or false
+ otherwise.
+
+11. Verify that the driver does not call::
+
+ netif_carrier_on()
+ netif_carrier_off()
+
+ as these will interfere with phylink's tracking of the link state,
+ and cause phylink to omit calls via the :c:func:`mac_link_up` and
+ :c:func:`mac_link_down` methods.
+
+Network drivers should call phylink_stop() and phylink_start() via their
+suspend/resume paths, which ensures that the appropriate
+:c:type:`struct phylink_mac_ops <phylink_mac_ops>` methods are called
+as necessary.
+
+For information describing the SFP cage in DT, please see the binding
+documentation in the kernel source tree
+``Documentation/devicetree/bindings/net/sff,sfp.txt``
--
2.7.4
^ permalink raw reply related
* [PATCH net] net/sched: act_ipt: fix refcount leak when replace fails
From: Davide Caratti @ 2019-02-22 11:33 UTC (permalink / raw)
To: Vlad Buslov, Cong Wang, Jamal Hadi Salim, Jiri Pirko,
David S. Miller
Cc: netdev
After commit 4e8ddd7f1758 ("net: sched: don't release reference on action
overwrite"), the error path of all actions was converted to drop refcount
also when the action was being overwritten. But we forgot act_ipt_init(),
in case allocation of 'tname' was not successful:
# tc action add action xt -j LOG --log-prefix hello index 100
tablename: mangle hook: NF_IP_POST_ROUTING
target: LOG level warning prefix "hello" index 100
# tc action show action xt
total acts 1
action order 0: tablename: mangle hook: NF_IP_POST_ROUTING
target LOG level warning prefix "hello"
index 100 ref 1 bind 0
# tc action replace action xt -j LOG --log-prefix world index 100
tablename: mangle hook: NF_IP_POST_ROUTING
target: LOG level warning prefix "world" index 100
RTNETLINK answers: Cannot allocate memory
We have an error talking to the kernel
# tc action show action xt
total acts 1
action order 0: tablename: mangle hook: NF_IP_POST_ROUTING
target LOG level warning prefix "hello"
index 100 ref 2 bind 0
Ensure we call tcf_idr_release(), in case 'tname' allocation failed, also
when the action is being replaced.
Fixes: 4e8ddd7f1758 ("net: sched: don't release reference on action overwrite")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_ipt.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/sched/act_ipt.c b/net/sched/act_ipt.c
index 8af6c11d2482..faa1addf89b3 100644
--- a/net/sched/act_ipt.c
+++ b/net/sched/act_ipt.c
@@ -199,8 +199,7 @@ static int __tcf_ipt_init(struct net *net, unsigned int id, struct nlattr *nla,
err2:
kfree(tname);
err1:
- if (ret == ACT_P_CREATED)
- tcf_idr_release(*a, bind);
+ tcf_idr_release(*a, bind);
return err;
}
--
2.20.1
^ permalink raw reply related
* [PATCH net] net/sched: act_skbedit: fix refcount leak when replace fails
From: Davide Caratti @ 2019-02-22 11:33 UTC (permalink / raw)
To: Vlad Buslov, Cong Wang, Jamal Hadi Salim, Jiri Pirko,
David S. Miller
Cc: netdev
when act_skbedit was converted to use RCU in the data plane, we added an
error path, but we forgot to drop the action refcount in case of failure
during a 'replace' operation:
# tc actions add action skbedit ptype otherhost pass index 100
# tc action show action skbedit
total acts 1
action order 0: skbedit ptype otherhost pass
index 100 ref 1 bind 0
# tc actions replace action skbedit ptype otherhost drop index 100
RTNETLINK answers: Cannot allocate memory
We have an error talking to the kernel
# tc action show action skbedit
total acts 1
action order 0: skbedit ptype otherhost pass
index 100 ref 2 bind 0
Ensure we call tcf_idr_release(), in case 'params_new' allocation failed,
also when the action is being replaced.
Fixes: c749cdda9089 ("net/sched: act_skbedit: don't use spinlock in the data path")
Signed-off-by: Davide Caratti <dcaratti@redhat.com>
---
net/sched/act_skbedit.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/net/sched/act_skbedit.c b/net/sched/act_skbedit.c
index 64dba3708fce..cfceed28c333 100644
--- a/net/sched/act_skbedit.c
+++ b/net/sched/act_skbedit.c
@@ -189,8 +189,7 @@ static int tcf_skbedit_init(struct net *net, struct nlattr *nla,
params_new = kzalloc(sizeof(*params_new), GFP_KERNEL);
if (unlikely(!params_new)) {
- if (ret == ACT_P_CREATED)
- tcf_idr_release(*a, bind);
+ tcf_idr_release(*a, bind);
return -ENOMEM;
}
--
2.20.1
^ permalink raw reply related
* RE: [PATCH net-next v2 1/7] net: Don't set transport offset to invalid value
From: Maxim Mikityanskiy @ 2019-02-22 12:30 UTC (permalink / raw)
To: Willem de Bruijn
Cc: David S. Miller, Saeed Mahameed, Willem de Bruijn, Jason Wang,
Eric Dumazet, netdev@vger.kernel.org, Eran Ben Elisha,
Tariq Toukan
In-Reply-To: <CAF=yD-KhWPL5JewXMxsikRNYCL88GZqdwrGvNNAm10N-NNaocw@mail.gmail.com>
> -----Original Message-----
> From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
> Sent: 21 February, 2019 19:28
> To: Maxim Mikityanskiy <maximmi@mellanox.com>
> Cc: David S. Miller <davem@davemloft.net>; Saeed Mahameed
> <saeedm@mellanox.com>; Willem de Bruijn <willemb@google.com>; Jason Wang
> <jasowang@redhat.com>; Eric Dumazet <edumazet@google.com>;
> netdev@vger.kernel.org; Eran Ben Elisha <eranbe@mellanox.com>; Tariq Toukan
> <tariqt@mellanox.com>
> Subject: Re: [PATCH net-next v2 1/7] net: Don't set transport offset to
> invalid value
>
> On Thu, Feb 21, 2019 at 7:40 AM Maxim Mikityanskiy <maximmi@mellanox.com>
> wrote:
> >
> > If the socket was created with socket(AF_PACKET, SOCK_RAW, 0),
> > skb->protocol will be unset, __skb_flow_dissect() will fail, and
> > skb_probe_transport_header() will fall back to the offset_hint, making
> > the resulting skb_transport_offset incorrect.
> >
> > If, however, there is no transport header in the packet,
> > transport_header shouldn't be set to an arbitrary value.
> >
> > Fix it by leaving the transport offset unset if it couldn't be found, to
> > be explicit rather than to fill it with some wrong value. It changes the
> > behavior, but if some code relied on the old behavior, it would be
> > broken anyway, as the old one is incorrect.
> >
> > Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
>
> qdisc_pkt_len_init also expects skb_transport_header(skb) to always be
> set for gso packets.
>
> Once net is merged into net-next, commit d5be7f632bad ("net: validate
This commit is already in net-next, isn't it?
> untrusted gso packets without csum offload") will ensure that packets
> that fail flow dissection do not make it into the stack. But we have
> to skip dissection in some cases, like tun [1].
OK, got you. However, is everything OK with patch [1]? It fixes false
positives, when a packet was dropped because network_header had not been
set yet for dissection to succeed, but what about evil packets that have
no network_offset at the moment of calling virtio_net_hdr_to_skb? Why
are all of them considered valid?
> I think we need to add a check in qdisc_pkt_len_init to skip the gso
> size estimation branch if !skb_transport_header_was_set(skb).
>
> Otherwise this patch set looks good to me. To avoid resubmitting
> everything we can fix up the qdisc_pkt_len_init in a follow-up, in
> which case I'm happy to add my Acked-by to this series.
I'll add this check and submit the patch soon. Thanks for reviewing!
> [1] http://patchwork.ozlabs.org/patch/1044429/
^ permalink raw reply
* Re: [Bridge] [PATCH net] net: bridge: remove ipv6 zero address check in mcast queries
From: Hangbin Liu @ 2019-02-22 12:49 UTC (permalink / raw)
To: Nikolay Aleksandrov
Cc: Linus Lüssing, netdev, roopa, bridge, davem, yinxu,
Sebastian Gottschall
In-Reply-To: <2f60dc0a-f6ee-06ad-f60c-15e035574a75@cumulusnetworks.com>
On Fri, Feb 22, 2019 at 01:16:53PM +0200, Nikolay Aleksandrov wrote:
> >
> > For consistency with other vendors and rfc, I would prefer to remove zero address election.
> > For compatibility with previous users, I'm also OK to revert it.
> > > Regards
> > Hangbin
> >
>
> Hi,
> In this case I'd suggest the following course of action:
> - For -net/-stable revert the change since backporting new options is a
> no-go and we need to restore the bridge state
> - After -net is merged in net-next, for net-next if you'd like add it
> as an option and also exclude it from elections when the option is
> enabled (for example something like multicast_nonzero_src_querier).
> Just please use the boolopt api and don't add new fields/attr ids.
> Obviously by default this option will be off to be backwards
> compatible and avoid surprise mcast flood.
>
> Or just leave it reverted. :)
This makes sense to me. Thanks for your suggestions.
Regards
Hangbin
^ permalink raw reply
* Re: [PATCH] sctp: don't compare hb_timer expire date before starting it
From: Marcelo Ricardo Leitner @ 2019-02-22 12:49 UTC (permalink / raw)
To: kwiecienmaciek; +Cc: linux-sctp, netdev, alexander.sverdlin, Maciej Kwiecien
In-Reply-To: <20190222084526.8214-1-maciej.kwiecien@nokia.com>
On Fri, Feb 22, 2019 at 09:45:26AM +0100, kwiecienmaciek@gmail.com wrote:
> From: Maciej Kwiecien <maciej.kwiecien@nokia.com>
>
> hb_timer might not start at all for a particular transport because its
> start is conditional. In a result a node is not sending heartbeats.
>
> Function sctp_transport_reset_hb_timer has two roles:
> - initial start of hb_timer for a given transport,
> - update expire date of hb_timer for a given transport.
> The function is optimized to update timer's expire only if it is before
> a new calculated one but this comparison is invalid for a timer which
> has not yet started. Such a timer has expire == 0 and if a new expire
> value is bigger than (MAX_JIFFIES / 2 + 2) then "time_before" macro will
> fail and timer will not start resulting in no heartbeat packets send by
> the node.
>
> This was found when association was initialized within first 5 mins
> after system boot due to jiffies init value which is near to MAX_JIFFIES.
>
> Test kernel version: 4.9.154 (ARCH=arm)
> hb_timer.expire = 0; //initialized, not started timer
> new_expire = MAX_JIFFIES / 2 + 2; //or more
> time_before(hb_timer.expire, new_expire) == false
>
> Fixes: ba6f5e33bdbb ("sctp: avoid refreshing heartbeat timer too often")
> Reported-by: Marcin Stojek <marcin.stojek@nokia.com>
> Tested-by: Marcin Stojek <marcin.stojek@nokia.com>
> Signed-off-by: Maciej Kwiecien <maciej.kwiecien@nokia.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> net/sctp/transport.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/sctp/transport.c b/net/sctp/transport.c
> index 033696e6f74f..ad158d311ffa 100644
> --- a/net/sctp/transport.c
> +++ b/net/sctp/transport.c
> @@ -207,7 +207,8 @@ void sctp_transport_reset_hb_timer(struct sctp_transport *transport)
>
> /* When a data chunk is sent, reset the heartbeat interval. */
> expires = jiffies + sctp_transport_timeout(transport);
> - if (time_before(transport->hb_timer.expires, expires) &&
> + if ((time_before(transport->hb_timer.expires, expires) ||
> + !timer_pending(&transport->hb_timer)) &&
> !mod_timer(&transport->hb_timer,
> expires + prandom_u32_max(transport->rto)))
> sctp_transport_hold(transport);
> --
> 2.14.1
>
^ permalink raw reply
* Re: [PATCH net-next v2] route: Add a new fib_multipath_hash_policy base on cpu id for tunnel packet
From: wenxu @ 2019-02-22 12:50 UTC (permalink / raw)
To: Nikolay Aleksandrov, davem; +Cc: netdev
In-Reply-To: <fe3c4fd4-1e8a-41a9-aefc-ebf606729524@cumulusnetworks.com>
On 2019/2/22 下午6:26, Nikolay Aleksandrov wrote:
> On 22/02/2019 11:20, wenxu@ucloud.cn wrote:
>> From: wenxu <wenxu@ucloud.cn>
>>
>> Current fib_multipath_hash_policy can make hash based on the L3 or
>> L4. But it only work on the outer IP. So a specific tunnel always
>> has the same hash value. But a specific tunnel may contain so many
>> inner connections. However there is no good way for tunnel packet.
>> A specific tunnel route based on the percpu dst_cache. It will not
>> lookup route table for each packet.
>>
>> This patch provide a based cpu id hash policy. The different
>> connection run on different cpu and there will be different hash
>> value for percpu dst_cache.
>>
>> Signed-off-by: wenxu <wenxu@ucloud.cn>
>> ---
>> net/ipv4/route.c | 6 ++++++
>> net/ipv4/sysctl_net_ipv4.c | 2 +-
>> 2 files changed, 7 insertions(+), 1 deletion(-)
>>
> Hi,
> When we had the same issue in the bonding, we added a new mode which used
> the flow dissector to get the inner headers and hash on them.
> I believe that is even easier nowadays, but I think people wanted to
> use different hash algorithms as well and last we discussed this we
> were talking about yet another bpf use to generate the hash.
> Now we got bpf_set_hash() which can be used to achieve this from various
> places, if you use that with hash_policy=1 (L4) then skb->hash will
> be used and you can achieve any hashing that you desire.
>
> Cheers,
> Nik
>
>
Yes, we can set the skb->hash. But ip tunnel lookup the route table without skb.
The most important for performance issue most tunnel send work with dst_cache.
The dst_cache is percpu cache. So the number of dst_entry of a specific tunnel is the cpu cores .
So It's more better hash based on outer and cpu id.
^ permalink raw reply
* Re: [PATCH v5 0/5] M_CAN Framework re-write
From: Dan Murphy @ 2019-02-22 12:50 UTC (permalink / raw)
To: Wolfgang Grandegger, mkl, davem; +Cc: linux-can, netdev, linux-kernel
In-Reply-To: <50a3a5d6-9b83-f0aa-095c-13b6361236ab@grandegger.com>
Wolfgang
On 2/22/19 3:38 AM, Wolfgang Grandegger wrote:
> Hello Dan,
>
> what kernel version is that patch series for. I have problems to apply it!
>
It is based off of Master
commit 2137397c92aec3713fa10be3c9b830f9a1674e60 (linux_master/master)
And I successfully rebased on top of
commit 8a61716ff2ab23eddd1f7a05a075a374e4d0c3d4 (linux_master/master)
Merge tag 'ceph-for-5.0-rc8' of git://github.com/ceph/ceph-client
Dan
> Wolfgang.
>
> Am 21.02.19 um 17:41 schrieb Wolfgang Grandegger:
>> Hello Dan,
>>
>> I will have a closer look end of this week!
>>
>> Wolfgang.
>>
>> Am 21.02.19 um 17:24 schrieb Dan Murphy:
>>> Bump
>>>
>>> On 2/14/19 12:27 PM, Dan Murphy wrote:
>>>> Hello
>>>>
>>>> OK I did not give up on this patch series just got a little preoccupied with
>>>> some other kernel work. But here is the update per the comments.
>>>>
>>>> It should be understood I broke these out for reviewability.
>>>> For instance the first patch does not compile on its own as including this
>>>> patch should not change the current functionality and it pulls all the io-mapped
>>>> code from the m_can base file to a platfrom file.
>>>>
>>>> The next patch "Migrate the m_can code to use the framework"
>>>> is the change to the kernel for the io-mapped conversion from a flat file to use
>>>> the framework. Finally the rename patch just renames the m_can_priv to
>>>> m_can_classdev. I broke this change out specifically for readability of the
>>>> migration patch per comments on the code.
>>>>
>>>> AFAIC the first 3 patches can all be squashed into a single patch. Or the
>>>> first 2 patches in the series can be re-arranged but then m_can functionality is
>>>> affected in the migration patch.
>>>>
>>>> Again the first 3 patches here are all just for readability and review purposes.
>>>>
>>>> Dan
>>>>
>>>> Dan Murphy (5):
>>>> can: m_can: Create a m_can platform framework
>>>> can: m_can: Migrate the m_can code to use the framework
>>>> can: m_can: Rename m_can_priv to m_can_classdev
>>>> dt-bindings: can: tcan4x5x: Add DT bindings for TCAN4x5X driver
>>>> can: tcan4x5x: Add tcan4x5x driver to the kernel
>>>>
>>>> .../devicetree/bindings/net/can/tcan4x5x.txt | 37 +
>>>> drivers/net/can/m_can/Kconfig | 14 +-
>>>> drivers/net/can/m_can/Makefile | 2 +
>>>> drivers/net/can/m_can/m_can.c | 788 +++++++++---------
>>>> drivers/net/can/m_can/m_can.h | 159 ++++
>>>> drivers/net/can/m_can/m_can_platform.c | 198 +++++
>>>> drivers/net/can/m_can/tcan4x5x.c | 531 ++++++++++++
>>>> 7 files changed, 1320 insertions(+), 409 deletions(-)
>>>> create mode 100644 Documentation/devicetree/bindings/net/can/tcan4x5x.txt
>>>> create mode 100644 drivers/net/can/m_can/m_can.h
>>>> create mode 100644 drivers/net/can/m_can/m_can_platform.c
>>>> create mode 100644 drivers/net/can/m_can/tcan4x5x.c
>>>>
>>>
>>>
--
------------------
Dan Murphy
^ permalink raw reply
* [PATCH net-next] net: Skip GSO length estimation if transport header is not set
From: Maxim Mikityanskiy @ 2019-02-22 12:55 UTC (permalink / raw)
To: David S. Miller, Willem de Bruijn, Saeed Mahameed, Jason Wang,
Eric Dumazet
Cc: netdev@vger.kernel.org, Eran Ben Elisha, Tariq Toukan,
Maxim Mikityanskiy
In-Reply-To: <20190221123908.7196-1-maximmi@mellanox.com>
qdisc_pkt_len_init expects transport_header to be set for GSO packets.
Patch [1] skips transport_header validation for GSO packets that don't
have network_header set at the moment of calling virtio_net_hdr_to_skb,
and allows them to pass into the stack. After patch [2] no placeholder
value is assigned to transport_header if dissection fails, so this patch
adds a check to the place where the value of transport_header is used.
[1] https://patchwork.ozlabs.org/patch/1044429/
[2] https://patchwork.ozlabs.org/patch/1046122/
Signed-off-by: Maxim Mikityanskiy <maximmi@mellanox.com>
---
net/core/dev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index a3d13f5e2bfc..8a0da95ff4cc 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -3421,7 +3421,7 @@ static void qdisc_pkt_len_init(struct sk_buff *skb)
/* To get more precise estimation of bytes sent on wire,
* we add to pkt_len the headers size of all segments
*/
- if (shinfo->gso_size) {
+ if (shinfo->gso_size && skb_transport_header_was_set(skb)) {
unsigned int hdr_len;
u16 gso_segs = shinfo->gso_segs;
--
2.19.1
^ permalink raw reply related
* [PATCH] net: dsa: Inherit dev addr from master
From: Vinod Koul @ 2019-02-22 12:56 UTC (permalink / raw)
To: David S. Miller
Cc: linux-arm-msm, Bjorn Andersson, Xiaofei Shen, Andrew Lunn,
Vivien Didelot, Florian Fainelli, Niklas Cassel, netdev,
Vinod Koul
From: Xiaofei Shen <xiaofeis@codeaurora.org>
When we create slave netdevice, the dev addr is inherited from master
but the master dev addr maybe NULL at that time, so inherit it again
while opening the slave.
Signed-off-by: Xiaofei Shen <xiaofeis@codeaurora.org>
Signed-off-by: Vinod Koul <vkoul@kernel.org>
---
net/dsa/slave.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index 8e64c4e947c6..5f95c538b58c 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -77,7 +77,9 @@ static int dsa_slave_open(struct net_device *dev)
if (!(master->flags & IFF_UP))
return -ENETDOWN;
- if (!ether_addr_equal(dev->dev_addr, master->dev_addr)) {
+ if (!is_valid_ether_addr(dev->dev_addr)) {
+ eth_hw_addr_inherit(dev, master);
+ } else if (!ether_addr_equal(dev->dev_addr, master->dev_addr)) {
err = dev_uc_add(master, dev->dev_addr);
if (err < 0)
goto out;
--
2.20.1
^ permalink raw reply related
* Re: [virtio-dev] Re: net_failover slave udev renaming (was Re: [RFC PATCH net-next v6 4/4] netvsc: refactor notifier/event handling code to use the bypass framework)
From: Rob Miller @ 2019-02-22 12:58 UTC (permalink / raw)
To: si-wei liu
Cc: Samudrala, Sridhar, Michael S. Tsirkin, Siwei Liu, Jiri Pirko,
Stephen Hemminger, David Miller, Netdev, virtualization,
virtio-dev, Brandeburg, Jesse, Alexander Duyck, Jakub Kicinski,
Jason Wang, liran.alon
In-Reply-To: <d9ef40a2-237b-0cce-4401-ecaeac4c602a@oracle.com>
I don’t know enough about how they get named, but is it possible for
user space to suggest its interface name, such that the interface name
would we as unique as the VM name itself. and is limited to scope to
be within the network boundry of an organization?
In other words, as a company, i decided to name my VM co-vm-1 through
co-vm-xxx, i would leave off location of vm b/c that will change. My
interfaces then would be named, co-vm-1.0 through co-vm-1.x.
Just thinking out loud.
Sent from my iPhone
> On Feb 22, 2019, at 2:55 AM, si-wei liu <si-wei.liu@oracle.com> wrote:
>
>
>
>> On 2/21/2019 11:00 PM, Samudrala, Sridhar wrote:
>>
>>
>>> On 2/21/2019 7:33 PM, si-wei liu wrote:
>>>
>>>
>>>> On 2/21/2019 5:39 PM, Michael S. Tsirkin wrote:
>>>>> On Thu, Feb 21, 2019 at 05:14:44PM -0800, Siwei Liu wrote:
>>>>> Sorry for replying to this ancient thread. There was some remaining
>>>>> issue that I don't think the initial net_failover patch got addressed
>>>>> cleanly, see:
>>>>>
>>>>> https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1815268
>>>>>
>>>>> The renaming of 'eth0' to 'ens4' fails because the udev userspace was
>>>>> not specifically writtten for such kernel automatic enslavement.
>>>>> Specifically, if it is a bond or team, the slave would typically get
>>>>> renamed *before* virtual device gets created, that's what udev can
>>>>> control (without getting netdev opened early by the other part of
>>>>> kernel) and other userspace components for e.g. initramfs,
>>>>> init-scripts can coordinate well in between. The in-kernel
>>>>> auto-enslavement of net_failover breaks this userspace convention,
>>>>> which don't provides a solution if user care about consistent naming
>>>>> on the slave netdevs specifically.
>>>>>
>>>>> Previously this issue had been specifically called out when IFF_HIDDEN
>>>>> and the 1-netdev was proposed, but no one gives out a solution to this
>>>>> problem ever since. Please share your mind how to proceed and solve
>>>>> this userspace issue if netdev does not welcome a 1-netdev model.
>>>> Above says:
>>>>
>>>> there's no motivation in the systemd/udevd community at
>>>> this point to refactor the rename logic and make it work well with
>>>> 3-netdev.
>>>>
>>>> What would the fix be? Skip slave devices?
>>>>
>>> There's nothing user can get if just skipping slave devices - the name is still unchanged and unpredictable e.g. eth0, or eth1 the next reboot, while the rest may conform to the naming scheme (ens3 and such). There's no way one can fix this in userspace alone - when the failover is created the enslaved netdev was opened by the kernel earlier than the userspace is made aware of, and there's no negotiation protocol for kernel to know when userspace has done initial renaming of the interface. I would expect netdev list should at least provide the direction in general for how this can be solved...
>>>
>> Is there an issue if slave device names are not predictable? The user/admin scripts are expected
>> to only work with the master failover device.
> Where does this expectation come from?
>
> Admin users may have ethtool or tc configurations that need to deal with predictable interface name. Third-party app which was built upon specifying certain interface name can't be modified to chase dynamic names.
>
> Specifically, we have pre-canned image that uses ethtool to fine tune VF offload settings post boot for specific workload. Those images won't work well if the name is constantly changing just after couple rounds of live migration.
>
>> Moreover, you were suggesting hiding the lower slave devices anyway. There was some discussion
>> about moving them to a hidden network namespace so that they are not visible from the default namespace.
>> I looked into this sometime back, but did not find the right kernel api to create a network namespace within
>> kernel. If so, we could use this mechanism to simulate a 1-netdev model.
> Yes, that's one possible implementation (IMHO the key is to make 1-netdev model as much transparent to a real NIC as possible, while a hidden netns is just the vehicle). However, I recall there was resistance around this discussion that even the concept of hiding itself is a taboo for Linux netdev. I would like to summon potential alternatives before concluding 1-netdev is the only solution too soon.
>
> Thanks,
> -Siwei
>
>>
>>> -Siwei
>>>
>>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: virtio-dev-unsubscribe@lists.oasis-open.org
> For additional commands, e-mail: virtio-dev-help@lists.oasis-open.org
>
^ permalink raw reply
* [PATCH] net: dsa: read mac address from DT for slave device
From: Vinod Koul @ 2019-02-22 12:58 UTC (permalink / raw)
To: David S. Miller
Cc: linux-arm-msm, Bjorn Andersson, Xiaofei Shen, Andrew Lunn,
Vivien Didelot, Florian Fainelli, Niklas Cassel, netdev,
Vinod Koul
From: Xiaofei Shen <xiaofeis@codeaurora.org>
Before creating a slave netdevice, get the mac address from DTS and
apply in case it is valid.
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;
--
2.20.1
^ permalink raw reply related
* Re: [PATCH] cfg80211: reg: Fix use-after-free in call_crda
From: Johannes Berg @ 2019-02-22 13:00 UTC (permalink / raw)
To: Yue Haibing, davem; +Cc: linux-kernel, netdev, linux-wireless
In-Reply-To: <20190222094233.21556-1-yuehaibing@huawei.com>
Hi,
> In function reg_query_database, query_regdb_file call
> request_firmware_nowait to do request_firmware asynchronously,
> which need the caller hold the reference of dev, otherwise it will
> do put_device freeing '®_pdev->dev'. After that, call_crda access
> the dev will trigger use-after-free bug.
So ... OK, but how does that then only fix the firmware file loading,
rather than CRDA calling?
> This patch fix this by holding a reference of dev in regulatory_init
> after platform_device_register_simple registered successly, which
> releasing in platform_device_unregister.
This doesn't make sense? You just add a new reference and don't release
it? If there was a bug then just loading & unloading would trigger an
underflow now?
platform_device_register_full() (to which _simple is a wrapper) will
evidently return the pdev with a reference held, because it does
platform_device_put() in the error path?
johannes
^ permalink raw reply
* pull-request: mac80211 2019-02-22
From: Johannes Berg @ 2019-02-22 13:10 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linux-wireless
Hi Dave,
So ... I thought there weren't going to be any more patches
when I asked you to apply that mac80211 one, but now a few
days later I still ended up with three that are current and
possibly even stable material.
Please pull and let me know if there's any problem.
Thanks,
johannes
The following changes since commit 6321aa197547da397753757bd84c6ce64b3e3d89:
phonet: fix building with clang (2019-02-21 16:23:56 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211.git tags/mac80211-for-davem-2019-02-22
for you to fetch changes up to 51d0af222f6fa43134c6187ab4f374630f6e0d96:
mac80211: allocate tailroom for forwarded mesh packets (2019-02-22 14:00:40 +0100)
----------------------------------------------------------------
Three more fixes:
* mac80211 mesh code wasn't allocating SKB tailroom properly
in some cases
* tx_sk_pacing_shift should be 7 for better performance
* mac80211_hwsim wasn't propagating genlmsg_reply() errors
----------------------------------------------------------------
Felix Fietkau (1):
mac80211: allocate tailroom for forwarded mesh packets
Li RongQing (1):
mac80211_hwsim: propagate genlmsg_reply return code
Toke Høiland-Jørgensen (1):
mac80211: Change default tx_sk_pacing_shift to 7
drivers/net/wireless/mac80211_hwsim.c | 2 +-
net/mac80211/main.c | 4 ++--
net/mac80211/rx.c | 7 ++++++-
3 files changed, 9 insertions(+), 4 deletions(-)
^ permalink raw reply
* Re: [PATCH net-next 2/4] net: dsa: mv88e6xxx: Fix -Wformat-security warnings
From: Andrew Lunn @ 2019-02-22 13:14 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, davem, Vivien Didelot, open list
In-Reply-To: <20190222040929.16657-3-f.fainelli@gmail.com>
On Thu, Feb 21, 2019 at 08:09:27PM -0800, Florian Fainelli wrote:
> We are not specifying an explicit format argument but instead passing a
> string litteral which causes these two warnings to show up:
>
> drivers/net/dsa/mv88e6xxx/chip.c: In function
> 'mv88e6xxx_irq_poll_setup':
> drivers/net/dsa/mv88e6xxx/chip.c:483:2: warning: format not a string
> literal and no format arguments [-Wformat-security]
> chip->kworker = kthread_create_worker(0, dev_name(chip->dev));
> ^~~~
> drivers/net/dsa/mv88e6xxx/ptp.c: In function 'mv88e6xxx_ptp_setup':
> drivers/net/dsa/mv88e6xxx/ptp.c:403:4: warning: format not a string
> literal and no format arguments [-Wformat-security]
> dev_name(chip->dev));
> ^~~~~~~~
> LD [M] drivers/net/dsa/mv88e6xxx/mv88e6xxx.o
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Hi Florian
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH bpf-next v1] bpf, lpm: fix lookup bug in map_delete_elem
From: Alban Crequy @ 2019-02-22 13:14 UTC (permalink / raw)
To: Martin Lau
Cc: Alban Crequy, ast@kernel.org, daniel@iogearbox.net,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
iago@kinvolk.io, Craig Gallek
In-Reply-To: <20190221222350.6u2jbtias2c3mgyy@kafai-mbp.dhcp.thefacebook.com>
On Thu, Feb 21, 2019 at 11:24 PM Martin Lau <kafai@fb.com> wrote:
>
> On Thu, Feb 21, 2019 at 05:39:26PM +0100, Alban Crequy wrote:
> > From: Alban Crequy <alban@kinvolk.io>
> >
> > trie_delete_elem() was deleting an entry even though it was not matching
> > if the prefixlen was correct. This patch adds a check on matchlen.
> >
> > Reproducer:
> >
> > $ sudo bpftool map create /sys/fs/bpf/mylpm type lpm_trie key 8 value 1 entries 128 name mylpm flags 1
> > $ sudo bpftool map update pinned /sys/fs/bpf/mylpm key hex 10 00 00 00 aa bb cc dd value hex 01
> > $ sudo bpftool map dump pinned /sys/fs/bpf/mylpm
> > key: 10 00 00 00 aa bb cc dd value: 01
> > Found 1 element
> > $ sudo bpftool map delete pinned /sys/fs/bpf/mylpm key hex 10 00 00 00 ff ff ff ff
> > $ echo $?
> > 0
> > $ sudo bpftool map dump pinned /sys/fs/bpf/mylpm
> > Found 0 elements
> The change makes sense to me. Can you add this reproducer to
> tools/testing/selftests/bpf/test_lpm_map.c?
>
> Bug fix should be for the "bpf" tree instead of "bpf-next"
> Fixes tag is also required, like
>
> Fixes: e454cf595853 ("bpf: Implement map_delete_elem for BPF_MAP_TYPE_LPM_TRIE")
> Cc: Craig Gallek <kraig@google.com>
Thanks! I'll send a v2 shortly with the selftest and the tags, based
on "bpf" tree.
Cheers,
Alban
> > Signed-off-by: Alban Crequy <alban@kinvolk.io>
> > ---
> > kernel/bpf/lpm_trie.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c
> > index abf1002080df..93a5cbbde421 100644
> > --- a/kernel/bpf/lpm_trie.c
> > +++ b/kernel/bpf/lpm_trie.c
> > @@ -471,6 +471,7 @@ static int trie_delete_elem(struct bpf_map *map, void *_key)
> > }
> >
> > if (!node || node->prefixlen != key->prefixlen ||
> > + node->prefixlen != matchlen ||
> > (node->flags & LPM_TREE_NODE_FLAG_IM)) {
> > ret = -ENOENT;
> > goto out;
> > --
> > 2.20.1
> >
^ permalink raw reply
* [PATCH bpf v2] bpf, lpm: fix lookup bug in map_delete_elem
From: Alban Crequy @ 2019-02-22 13:19 UTC (permalink / raw)
To: ast, daniel; +Cc: kafai, netdev, linux-kernel, alban, iago
From: Alban Crequy <alban@kinvolk.io>
trie_delete_elem() was deleting an entry even though it was not matching
if the prefixlen was correct. This patch adds a check on matchlen.
Reproducer:
$ sudo bpftool map create /sys/fs/bpf/mylpm type lpm_trie key 8 value 1 entries 128 name mylpm flags 1
$ sudo bpftool map update pinned /sys/fs/bpf/mylpm key hex 10 00 00 00 aa bb cc dd value hex 01
$ sudo bpftool map dump pinned /sys/fs/bpf/mylpm
key: 10 00 00 00 aa bb cc dd value: 01
Found 1 element
$ sudo bpftool map delete pinned /sys/fs/bpf/mylpm key hex 10 00 00 00 ff ff ff ff
$ echo $?
0
$ sudo bpftool map dump pinned /sys/fs/bpf/mylpm
Found 0 elements
A similar reproducer is added in the selftests.
Without the patch:
$ sudo ./tools/testing/selftests/bpf/test_lpm_map
test_lpm_map: test_lpm_map.c:485: test_lpm_delete: Assertion `bpf_map_delete_elem(map_fd, key) == -1 && errno == ENOENT' failed.
Aborted
With the patch: test_lpm_map runs without errors.
Fixes: e454cf595853 ("bpf: Implement map_delete_elem for BPF_MAP_TYPE_LPM_TRIE")
Cc: Craig Gallek <kraig@google.com>
Signed-off-by: Alban Crequy <alban@kinvolk.io>
---
Changes v1 to v2:
- add selftest (review from Martin)
- update commitmsg tags (review from Martin)
- rebase on "bpf" tree and test again (review from Martin)
---
kernel/bpf/lpm_trie.c | 1 +
tools/testing/selftests/bpf/test_lpm_map.c | 10 ++++++++++
2 files changed, 11 insertions(+)
diff --git a/kernel/bpf/lpm_trie.c b/kernel/bpf/lpm_trie.c
index abf1002080df..93a5cbbde421 100644
--- a/kernel/bpf/lpm_trie.c
+++ b/kernel/bpf/lpm_trie.c
@@ -471,6 +471,7 @@ static int trie_delete_elem(struct bpf_map *map, void *_key)
}
if (!node || node->prefixlen != key->prefixlen ||
+ node->prefixlen != matchlen ||
(node->flags & LPM_TREE_NODE_FLAG_IM)) {
ret = -ENOENT;
goto out;
diff --git a/tools/testing/selftests/bpf/test_lpm_map.c b/tools/testing/selftests/bpf/test_lpm_map.c
index 147e34cfceb7..02d7c871862a 100644
--- a/tools/testing/selftests/bpf/test_lpm_map.c
+++ b/tools/testing/selftests/bpf/test_lpm_map.c
@@ -474,6 +474,16 @@ static void test_lpm_delete(void)
assert(bpf_map_lookup_elem(map_fd, key, &value) == -1 &&
errno == ENOENT);
+ key->prefixlen = 30; // unused prefix so far
+ inet_pton(AF_INET, "192.255.0.0", key->data);
+ assert(bpf_map_delete_elem(map_fd, key) == -1 &&
+ errno == ENOENT);
+
+ key->prefixlen = 16; // same prefix as the root node
+ inet_pton(AF_INET, "192.255.0.0", key->data);
+ assert(bpf_map_delete_elem(map_fd, key) == -1 &&
+ errno == ENOENT);
+
/* assert initial lookup */
key->prefixlen = 32;
inet_pton(AF_INET, "192.168.0.1", key->data);
--
2.20.1
^ permalink raw reply related
* [PATCH net] Revert "bridge: do not add port to router list when receives query with source 0.0.0.0"
From: Hangbin Liu @ 2019-02-22 13:22 UTC (permalink / raw)
To: netdev
Cc: Nikolay Aleksandrov, roopa, bridge, davem, yinxu,
Greg Kroah-Hartman, stable, Sebastian Gottschall,
Linus Lüssing, Hangbin Liu
This reverts commit 5a2de63fd1a5 ("bridge: do not add port to router list
when receives query with source 0.0.0.0") and commit 0fe5119e267f ("net:
bridge: remove ipv6 zero address check in mcast queries")
The reason is RFC 4541 is not a standard but suggestive. Currently we
will elect 0.0.0.0 as Querier if there is no ip address configured on
bridge. If we do not add the port which recives query with source
0.0.0.0 to router list, the IGMP reports will not be about to forward
to Querier, IGMP data will also not be able to forward to dest.
As Nikolay suggested, revert this change first and add a boolopt api
to disable none-zero election in future if needed.
Reported-by: Linus Lüssing <linus.luessing@c0d3.blue>
Reported-by: Sebastian Gottschall <s.gottschall@newmedia-net.de>
Fixes: 5a2de63fd1a5 ("bridge: do not add port to router list when receives query with source 0.0.0.0")
Fixes: 0fe5119e267f ("net: bridge: remove ipv6 zero address check in mcast queries")
Signed-off-by: Hangbin Liu <liuhangbin@gmail.com>
---
net/bridge/br_multicast.c | 9 +--------
1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index 3aeff0895669..ac92b2eb32b1 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1204,14 +1204,7 @@ static void br_multicast_query_received(struct net_bridge *br,
return;
br_multicast_update_query_timer(br, query, max_delay);
-
- /* Based on RFC4541, section 2.1.1 IGMP Forwarding Rules,
- * the arrival port for IGMP Queries where the source address
- * is 0.0.0.0 should not be added to router port list.
- */
- if ((saddr->proto == htons(ETH_P_IP) && saddr->u.ip4) ||
- saddr->proto == htons(ETH_P_IPV6))
- br_multicast_mark_router(br, port);
+ br_multicast_mark_router(br, port);
}
static void br_ip4_multicast_query(struct net_bridge *br,
--
2.19.2
^ permalink raw reply related
* Re: [PATCH net-next 1/4] mlxsw: spectrum: Avoid -Wformat-truncation warnings
From: Ido Schimmel @ 2019-02-22 13:25 UTC (permalink / raw)
To: Florian Fainelli
Cc: netdev@vger.kernel.org, davem@davemloft.net, Jiri Pirko,
open list
In-Reply-To: <20190222040929.16657-2-f.fainelli@gmail.com>
On Thu, Feb 21, 2019 at 08:09:26PM -0800, Florian Fainelli wrote:
> Give precision identifiers to the two snprintf() formatting the priority
> and TC strings to avoid producing these two warnings:
>
> drivers/net/ethernet/mellanox/mlxsw/spectrum.c: In function
> 'mlxsw_sp_port_get_prio_strings':
> drivers/net/ethernet/mellanox/mlxsw/spectrum.c:2132:37: warning: '%d'
> directive output may be truncated writing between 1 and 3 bytes into a
> region of size between 0 and 31 [-Wformat-truncation=]
> snprintf(*p, ETH_GSTRING_LEN, "%s_%d",
> ^~
> drivers/net/ethernet/mellanox/mlxsw/spectrum.c:2132:3: note: 'snprintf'
> output between 3 and 36 bytes into a destination of size 32
> snprintf(*p, ETH_GSTRING_LEN, "%s_%d",
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> mlxsw_sp_port_hw_prio_stats[i].str, prio);
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/ethernet/mellanox/mlxsw/spectrum.c: In function
> 'mlxsw_sp_port_get_tc_strings':
> drivers/net/ethernet/mellanox/mlxsw/spectrum.c:2143:37: warning: '%d'
> directive output may be truncated writing between 1 and 11 bytes into a
> region of size between 0 and 31 [-Wformat-truncation=]
> snprintf(*p, ETH_GSTRING_LEN, "%s_%d",
> ^~
> drivers/net/ethernet/mellanox/mlxsw/spectrum.c:2143:3: note: 'snprintf'
> output between 3 and 44 bytes into a destination of size 32
> snprintf(*p, ETH_GSTRING_LEN, "%s_%d",
> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> mlxsw_sp_port_hw_tc_stats[i].str, tc);
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Ido Schimmel <idosch@mellanox.com>
^ 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