* Re: [PATCH net 1/2] ipv6: route: enforce RCU protection in rt6_update_exception_stamp_rt()
From: David Miller @ 2019-02-20 19:28 UTC (permalink / raw)
To: pabeni; +Cc: netdev, dsahern
In-Reply-To: <7bf3b6ff2f6d4f876783c3a154fd030805f55fc5.camel@redhat.com>
From: Paolo Abeni <pabeni@redhat.com>
Date: Wed, 20 Feb 2019 18:22:55 +0100
> -ELOWONCOFFEE:
Mind if I make you some? :)
> I'll send a v2.
>
> Again, I'm sorry for the noise,
You are keeping me busy today.
^ permalink raw reply
* Re: [PATCH] net: dsa: add missing phy address offset
From: Florian Fainelli @ 2019-02-20 19:27 UTC (permalink / raw)
To: Marcel Reichmuth, linux-kernel@vger.kernel.org,
netdev@vger.kernel.org
Cc: andrew@lunn.ch, vivien.didelot@gmail.com, davem@davemloft.net
In-Reply-To: <20190220181426.2970-1-marcel.reichmuth@netmodule.com>
On 2/20/19 10:15 AM, Marcel Reichmuth wrote:
> When phys do not start at address 0 like on the mv88e6341 the wrong
> phy address is used and therefore the slave ports can not be
> initialized. This patch adds the proper offset to the phy address.
>
> Signed-off-by: Marcel Reichmuth <marcel.reichmuth@netmodule.com>
You are supposed to describe the port to PHY mapping using the binding,
so for instance:
ports {
port@0 {
reg = <0>;
phy-handle = <&phy1>;
};
};
mdio {
phy1: phy@1 {
reg = <1>;
};
};
etc. is not that working for you?
> ---
> drivers/net/dsa/mv88e6xxx/chip.c | 3 +++
> include/net/dsa.h | 1 +
> net/dsa/slave.c | 3 ++-
> 3 files changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 12fd7ce3f1ff..0ca649f784d2 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -2198,12 +2198,15 @@ static int mv88e6xxx_setup_upstream_port(struct mv88e6xxx_chip *chip, int port)
> static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
> {
> struct dsa_switch *ds = chip->ds;
> + struct dsa_port *dp = &ds->ports[port];
> int err;
> u16 reg;
>
> chip->ports[port].chip = chip;
> chip->ports[port].port = port;
>
> + dp->phy_base_addr = chip->info->phy_base_addr;
> +
> /* MAC Forcing register: don't force link, speed, duplex or flow control
> * state to any particular values on physical ports, but force the CPU
> * port and all DSA ports to their maximum bandwidth and full duplex.
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index b3eefe8e18fd..f9c9dc1f6d21 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -196,6 +196,7 @@ struct dsa_port {
>
> struct dsa_switch *ds;
> unsigned int index;
> + unsigned int phy_base_addr;
> const char *name;
> const struct dsa_port *cpu_dp;
> struct device_node *dn;
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index a1c9fe155057..4f67dff34a3b 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -1221,7 +1221,8 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev)
> /* We could not connect to a designated PHY or SFP, so use the
> * switch internal MDIO bus instead
> */
> - ret = dsa_slave_phy_connect(slave_dev, dp->index);
> + ret = dsa_slave_phy_connect(slave_dev, dp->phy_base_addr +
> + dp->index);
> if (ret) {
> netdev_err(slave_dev,
> "failed to connect to port %d: %d\n",
>
--
Florian
^ permalink raw reply
* Re: [PATCH net-next] gso: validate gso_type on ipip style tunnels
From: David Miller @ 2019-02-20 19:25 UTC (permalink / raw)
To: willemdebruijn.kernel; +Cc: netdev, willemb
In-Reply-To: <20190220155212.47248-1-willemdebruijn.kernel@gmail.com>
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Wed, 20 Feb 2019 10:52:12 -0500
> From: Willem de Bruijn <willemb@google.com>
>
> Commit 121d57af308d ("gso: validate gso_type in GSO handlers") added
> gso_type validation to existing gso_segment callback functions, to
> filter out illegal and potentially dangerous SKB_GSO_DODGY packets.
>
> Convert tunnels that now call inet_gso_segment and ipv6_gso_segment
> directly to have their own callbacks and extend validation to these.
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
Looks good, applied, thanks Willem.
^ permalink raw reply
* Re: [PATCH 6/6] net: ethernet: ti: cpsw: deprecate cpsw-phy-sel driver
From: David Miller @ 2019-02-20 19:22 UTC (permalink / raw)
To: grygorii.strashko
Cc: kishon, robh+dt, tony, netdev, nsekhar, linux-kernel, linux-omap,
devicetree, linux-arm-kernel
In-Reply-To: <1550676319-6440-7-git-send-email-grygorii.strashko@ti.com>
From: Grygorii Strashko <grygorii.strashko@ti.com>
Date: Wed, 20 Feb 2019 17:25:19 +0200
> Deprecate cpsw-phy-sel driver as it's been replaced with new
> TI phy-gmii-sel PHY driver.
>
> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
Acked-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* Re: [PATCH net] net: dsa: fix unintended change of bridge interface STP state
From: David Miller @ 2019-02-20 19:09 UTC (permalink / raw)
To: rmk+kernel; +Cc: andrew, f.fainelli, vivien.didelot, netdev
In-Reply-To: <E1gwPBM-0008SQ-CM@rmk-PC.armlinux.org.uk>
From: Russell King <rmk+kernel@armlinux.org.uk>
Date: Wed, 20 Feb 2019 10:32:52 +0000
> When a DSA port is added to a bridge and brought up, the resulting STP
> state programmed into the hardware depends on the order that these
> operations are performed. However, the Linux bridge code believes that
> the port is in disabled mode.
>
> If the DSA port is first added to a bridge and then brought up, it will
> be in blocking mode. If it is brought up and then added to the bridge,
> it will be in disabled mode.
>
> This difference is caused by DSA always setting the STP mode in
> dsa_port_enable() whether or not this port is part of a bridge. Since
> bridge always sets the STP state when the port is added, brought up or
> taken down, it is unnecessary for us to manipulate the STP state.
>
> Apparently, this code was copied from Rocker, and the very next day a
> similar fix for Rocker was merged but was not propagated to DSA. See
> e47172ab7e41 ("rocker: put port in FORWADING state after leaving bridge")
>
> Fixes: b73adef67765 ("net: dsa: integrate with SWITCHDEV for HW bridging")
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH net-next] selftest/tls: Add test to verify received 'type' of non-data record
From: David Miller @ 2019-02-20 19:06 UTC (permalink / raw)
To: vakul.garg; +Cc: netdev, borisp, aviadye, davejwatson, doronrk
In-Reply-To: <20190220084033.25141-1-vakul.garg@nxp.com>
From: Vakul Garg <vakul.garg@nxp.com>
Date: Wed, 20 Feb 2019 08:43:00 +0000
> Test case 'control_msg' has been updated to peek non-data record and
> then verify the type of record received. Subsequently, the same record
> is retrieved without MSG_PEEK flag in recvmsg().
>
> Signed-off-by: Vakul Garg <vakul.garg@nxp.com>
Applied.
^ permalink raw reply
* Re: linux-next: Fixes tag needs some work in the net-next tree
From: David Miller @ 2019-02-20 19:02 UTC (permalink / raw)
To: jiri; +Cc: vkoul, sfr, netdev, linux-next, linux-kernel, andrew
In-Reply-To: <20190220083611.GJ3080@nanopsycho>
From: Jiri Pirko <jiri@resnulli.us>
Date: Wed, 20 Feb 2019 09:36:11 +0100
> Would be good to have some robot checking "Fixes" sanity...
I want to add a script to my trees that locally do it for me but the
backlog for patch review for me is so huge that I never get to "fun"
tasks like that....
This has been discussed before too.
^ permalink raw reply
* Re: [PATCH bpf-next 6/9] bpf: Sample program to load cg skb BPF programs
From: Jakub Kicinski @ 2019-02-20 18:43 UTC (permalink / raw)
To: Stanislav Fomichev; +Cc: brakmo, netdev, Martin Lau, Alexei Starovoitov, daniel
In-Reply-To: <20190220050129.GC1928@mini-arch>
On Tue, 19 Feb 2019 21:01:29 -0800, Stanislav Fomichev wrote:
> On 02/19, Jakub Kicinski wrote:
> > On Mon, 18 Feb 2019 21:38:36 -0800, brakmo wrote:
> > > +#include "bpf_load.h"
> >
> > nit: please use libbpf
> My suggestion was to just use bpftool, no need for another helper in the
> samples/ directory. WDYT?
Oh yes, you're right, bpftool fully covers what this samples does.
^ permalink raw reply
* Re: [PATCH net] mlxsw: __mlxsw_sp_port_headroom_set(): Fix a use of local variable
From: David Miller @ 2019-02-20 18:41 UTC (permalink / raw)
To: idosch; +Cc: netdev, jiri, petrm, mlxsw
In-Reply-To: <20190220081215.GA2684@splinter>
From: Ido Schimmel <idosch@mellanox.com>
Date: Wed, 20 Feb 2019 08:12:18 +0000
> On Tue, Feb 19, 2019 at 10:33:38AM -0800, David Miller wrote:
>> From: Ido Schimmel <idosch@mellanox.com>
>> Date: Tue, 19 Feb 2019 09:30:31 +0000
>>
>> > We have a series for net-next that adds support for Spectrum-2 shared
>> > buffers and it depends on this patch. I was wondering if you could merge
>> > net into net-next today or later this week, so that we could submit it.
>> >
>> > Normally I would just wait for the merge to happen, but it looks like
>> > this is going to be the last week to submit changes and I prefer not to
>> > miss the window while waiting for the inevitable merge.
>> >
>> > Thanks and sorry about the noise.
>>
>> Ido, I will send Linus a pull request today and he should pull it in and
>> I should therefore be able to get net-next sync'd with it by the end of
>> tomorrow.
>
> Great, thanks!
This is now done.
^ permalink raw reply
* Re: [PATCH net] ipvlan: disallow userns cap_net_admin to change global mode/flags
From: Mahesh Bandewar (महेश बंडेवार) @ 2019-02-20 18:40 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: David Miller, m, linux-netdev
In-Reply-To: <20190219231530.11306-1-daniel@iogearbox.net>
On Tue, Feb 19, 2019 at 3:38 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> When running Docker with userns isolation e.g. --userns-remap="default"
> and spawning up some containers with CAP_NET_ADMIN under this realm, I
> noticed that link changes on ipvlan slave device inside that container
> can affect all devices from this ipvlan group which are in other net
> namespaces where the container should have no permission to make changes
> to, such as the init netns, for example.
>
> This effectively allows to undo ipvlan private mode and switch globally to
> bridge mode where slaves can communicate directly without going through
> hostns, or it allows to switch between global operation mode (l2/l3/l3s)
> for everyone bound to the given ipvlan master device. libnetwork plugin
> here is creating an ipvlan master and ipvlan slave in hostns and a slave
> each that is moved into the container's netns upon creation event.
>
> * In hostns:
>
> # ip -d a
> [...]
> 8: cilium_host@bond0: <BROADCAST,MULTICAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
> link/ether 0c:c4:7a:e1:3d:cc brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65535
> ipvlan mode l3 bridge numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
> inet 10.41.0.1/32 scope link cilium_host
> valid_lft forever preferred_lft forever
> [...]
>
> * Spawn container & change ipvlan mode setting inside of it:
>
> # docker run -dt --cap-add=NET_ADMIN --network cilium-net --name client -l app=test cilium/netperf
> 9fff485d69dcb5ce37c9e33ca20a11ccafc236d690105aadbfb77e4f4170879c
>
> # docker exec -ti client ip -d a
> [...]
> 10: cilium0@if4: <BROADCAST,MULTICAST,NOARP,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
> link/ether 0c:c4:7a:e1:3d:cc brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65535
> ipvlan mode l3 bridge numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
> inet 10.41.197.43/32 brd 10.41.197.43 scope global cilium0
> valid_lft forever preferred_lft forever
>
> # docker exec -ti client ip link change link cilium0 name cilium0 type ipvlan mode l2
>
> # docker exec -ti client ip -d a
> [...]
> 10: cilium0@if4: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
> link/ether 0c:c4:7a:e1:3d:cc brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65535
> ipvlan mode l2 bridge numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
> inet 10.41.197.43/32 brd 10.41.197.43 scope global cilium0
> valid_lft forever preferred_lft forever
>
> * In hostns (mode switched to l2):
>
> # ip -d a
> [...]
> 8: cilium_host@bond0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
> link/ether 0c:c4:7a:e1:3d:cc brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65535
> ipvlan mode l2 bridge numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
> inet 10.41.0.1/32 scope link cilium_host
> valid_lft forever preferred_lft forever
> [...]
>
> Same l3 -> l2 switch would also happen by creating another slave inside
> the container's network namespace when specifying the existing cilium0
> link to derive the actual (bond0) master:
>
> # docker exec -ti client ip link add link cilium0 name cilium1 type ipvlan mode l2
>
> # docker exec -ti client ip -d a
> [...]
> 2: cilium1@if4: <BROADCAST,MULTICAST> mtu 1500 qdisc noop state DOWN group default qlen 1000
> link/ether 0c:c4:7a:e1:3d:cc brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65535
> ipvlan mode l2 bridge numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
> 10: cilium0@if4: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
> link/ether 0c:c4:7a:e1:3d:cc brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65535
> ipvlan mode l2 bridge numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
> inet 10.41.197.43/32 brd 10.41.197.43 scope global cilium0
> valid_lft forever preferred_lft forever
>
> * In hostns:
>
> # ip -d a
> [...]
> 8: cilium_host@bond0: <BROADCAST,MULTICAST,UP,LOWER_UP> mtu 1500 qdisc noqueue state UNKNOWN group default qlen 1000
> link/ether 0c:c4:7a:e1:3d:cc brd ff:ff:ff:ff:ff:ff promiscuity 0 minmtu 68 maxmtu 65535
> ipvlan mode l2 bridge numtxqueues 1 numrxqueues 1 gso_max_size 65536 gso_max_segs 65535
> inet 10.41.0.1/32 scope link cilium_host
> valid_lft forever preferred_lft forever
> [...]
>
> One way to mitigate it is to check CAP_NET_ADMIN permissions of
> the ipvlan master device's ns, and only then allow to change
> mode or flags for all devices bound to it. Above two cases are
> then disallowed after the patch.
thanks for the fix Daniel.
>
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Mahesh Bandewar <maheshb@google.com>
> ---
> drivers/net/ipvlan/ipvlan_main.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/net/ipvlan/ipvlan_main.c b/drivers/net/ipvlan/ipvlan_main.c
> index 7cdac77..07e41c4 100644
> --- a/drivers/net/ipvlan/ipvlan_main.c
> +++ b/drivers/net/ipvlan/ipvlan_main.c
> @@ -499,6 +499,8 @@ static int ipvlan_nl_changelink(struct net_device *dev,
>
> if (!data)
> return 0;
> + if (!ns_capable(dev_net(ipvlan->phy_dev)->user_ns, CAP_NET_ADMIN))
> + return -EPERM;
>
> if (data[IFLA_IPVLAN_MODE]) {
> u16 nmode = nla_get_u16(data[IFLA_IPVLAN_MODE]);
> @@ -601,6 +603,8 @@ int ipvlan_link_new(struct net *src_net, struct net_device *dev,
> struct ipvl_dev *tmp = netdev_priv(phy_dev);
>
> phy_dev = tmp->phy_dev;
> + if (!ns_capable(dev_net(phy_dev)->user_ns, CAP_NET_ADMIN))
> + return -EPERM;
> } else if (!netif_is_ipvlan_port(phy_dev)) {
> /* Exit early if the underlying link is invalid or busy */
> if (phy_dev->type != ARPHRD_ETHER ||
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH net v2] af_packet: fix raw sockets over 6in4 tunnel
From: Willem de Bruijn @ 2019-02-20 18:39 UTC (permalink / raw)
To: Sasha Levin
Cc: David Miller, Nicolas Dichtel, Network Development,
Willem de Bruijn, Maxim Mikityanskiy
In-Reply-To: <20190218155745.GU10616@sasha-vm>
On Mon, Feb 18, 2019 at 1:50 PM Sasha Levin <sashal@kernel.org> wrote:
>
> On Thu, Jan 17, 2019 at 03:55:27PM -0800, David Miller wrote:
> >From: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> >Date: Thu, 17 Jan 2019 11:27:22 +0100
> >
> >> Since commit cb9f1b783850, scapy (which uses an AF_PACKET socket in
> >> SOCK_RAW mode) is unable to send a basic icmp packet over a sit tunnel:
> >>
> >> Here is a example of the setup:
> >> $ ip link set ntfp2 up
> >> $ ip addr add 10.125.0.1/24 dev ntfp2
> >> $ ip tunnel add tun1 mode sit ttl 64 local 10.125.0.1 remote 10.125.0.2 dev ntfp2
> >> $ ip addr add fd00:cafe:cafe::1/128 dev tun1
> >> $ ip link set dev tun1 up
> >> $ ip route add fd00:200::/64 dev tun1
> >> $ scapy
> >>>>> p = []
> >>>>> p += IPv6(src='fd00:100::1', dst='fd00:200::1')/ICMPv6EchoRequest()
> >>>>> send(p, count=1, inter=0.1)
> >>>>> quit()
> >> $ ip -s link ls dev tun1 | grep -A1 "TX.*errors"
> >> TX: bytes packets errors dropped carrier collsns
> >> 0 0 1 0 0 0
> >>
> >> The problem is that the network offset is set to the hard_header_len of the
> >> output device (tun1, ie 14 + 20) and in our case, because the packet is
> >> small (48 bytes) the pskb_inet_may_pull() fails (it tries to pull 40 bytes
> >> (ipv6 header) starting from the network offset).
> >>
> >> This problem is more generally related to device with variable hard header
> >> length. To avoid a too intrusive patch in the current release, a (ugly)
> >> workaround is proposed in this patch. It has to be cleaned up in net-next.
> >>
> >> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=993675a3100b1
> >> Link: http://patchwork.ozlabs.org/patch/1024489/
> >> Fixes: cb9f1b783850 ("ip: validate header length on virtual device xmit")
> >> CC: Willem de Bruijn <willemb@google.com>
> >> CC: Maxim Mikityanskiy <maximmi@mellanox.com>
> >> Signed-off-by: Nicolas Dichtel <nicolas.dichtel@6wind.com>
> >> ---
> >>
> >> v1 -> v2:
> >> reset nh offset only for small packets sent on a variable hard hdr len device
> >
> >Applied.
>
> Should this go to -stable as well? The patch it fixes is in 4.20.
I believe so. It was also backported to 4.19 stable.
^ permalink raw reply
* Re: [PATCH net] net: dsa: fix unintended change of bridge interface STP state
From: Florian Fainelli @ 2019-02-20 18:38 UTC (permalink / raw)
To: Andrew Lunn; +Cc: Russell King, Vivien Didelot, David S. Miller, netdev
In-Reply-To: <20190220172715.GL13075@lunn.ch>
On 2/20/19 9:27 AM, Andrew Lunn wrote:
> On Wed, Feb 20, 2019 at 09:22:30AM -0800, Florian Fainelli wrote:
>> On 2/20/19 2:32 AM, Russell King wrote:
>>> When a DSA port is added to a bridge and brought up, the resulting STP
>>> state programmed into the hardware depends on the order that these
>>> operations are performed. However, the Linux bridge code believes that
>>> the port is in disabled mode.
>>>
>>> If the DSA port is first added to a bridge and then brought up, it will
>>> be in blocking mode. If it is brought up and then added to the bridge,
>>> it will be in disabled mode.
>>>
>>> This difference is caused by DSA always setting the STP mode in
>>> dsa_port_enable() whether or not this port is part of a bridge. Since
>>> bridge always sets the STP state when the port is added, brought up or
>>> taken down, it is unnecessary for us to manipulate the STP state.
>>>
>>> Apparently, this code was copied from Rocker, and the very next day a
>>> similar fix for Rocker was merged but was not propagated to DSA. See
>>> e47172ab7e41 ("rocker: put port in FORWADING state after leaving bridge")
>>>
>>> Fixes: b73adef67765 ("net: dsa: integrate with SWITCHDEV for HW bridging")
>>> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>>
>> Nice example of cargo cult programming, thanks for fixing this!
>
> Maybe now would be a good time to look at other drivers. Does the
> Microsemi Ocelot driver have the same issue?
Checked fsl-dpaa2/ethsw and ocelot and neither of those seem to be
affected by this problem.
--
Florian
^ permalink raw reply
* Re: [RFC] net: dsa: qca8k: CPU port broken with commit 5502b218e001 ("net: phy: use phy_resolve_aneg_linkmode in genphy_read_status")
From: Heiner Kallweit @ 2019-02-20 18:37 UTC (permalink / raw)
To: Michal Vokáč, Andrew Lunn, Vinod Koul
Cc: David S. Miller, Florian Fainelli, netdev
In-Reply-To: <d127618b-873e-e4aa-ea75-f747da5574ee@ysoft.com>
On 20.02.2019 16:02, Michal Vokáč wrote:
> Hi,
>
> Another issue in a row with networking on imx6dl-yapp4 platform [1]
> that uses QCA8334 Ethernet switch.
>
> Very recently, with Vinod and Andrew, we solved an issue with
> RGMII_ID mode by patch[2][3]. I tested those with next-20190215
> and it worked just fine.
>
> The patch[2] was merged into next-20190220 so I tested the latest version.
> Now the cpu port does not work again. I tracked it down to this commit
> 5502b218e001 ("net: phy: use phy_resolve_aneg_linkmode in
> genphy_read_status") [4]
>
> If I revert the offending commit, cpu port works fine. I suspect the
> problem is on the qca8k driver side but I am not really sure.
> AFAICT autonegotiation is not available on the QCA833x cpu port (MAC0).
>
Patch[4] only affects a code path with AUTONEG_ENABLE == phydev->autoneg.
IMO this shouldn't be the case for the cpu port (like you state).
Seems like somewhere a phydev->autoneg = AUTONEG_DISABLE is missing.
Not clear to me is where the difference with this patch comes from.
It would be helpful to know (w/o patch[4]):
- which values the register reads in genphy_read_status() return
- which bits are set in phydev->advertising
> Any ideas what may be the root cause of the problem?
>
> Thank you,
> Michal
>
Heiner
> [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=87489ec3a77f3e01bcf0d46e353ae7112ec8c4f0
> [2] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=a968b5e9d5879f9535d6099505f9e14abcafb623
> [3] https://lore.kernel.org/patchwork/patch/1043817/
> [4] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=5502b218e001
>
^ permalink raw reply
* Re: [RFC PATCH net-next v3 13/21] ethtool: provide timestamping information in GET_INFO request
From: Jakub Kicinski @ 2019-02-20 18:37 UTC (permalink / raw)
To: Michal Kubecek
Cc: netdev, David Miller, Andrew Lunn, Jiri Pirko, linux-kernel
In-Reply-To: <20190220130007.GI23151@unicorn.suse.cz>
On Wed, 20 Feb 2019 14:00:07 +0100, Michal Kubecek wrote:
> On Tue, Feb 19, 2019 at 07:00:48PM -0800, Jakub Kicinski wrote:
> > On Mon, 18 Feb 2019 19:22:29 +0100 (CET), Michal Kubecek wrote:
> > > Add timestamping information as provided by ETHTOOL_GET_TS_INFO ioctl
> > > command in GET_INFO reply if ETH_INFO_IM_TSINFO flag is set in the request.
> > >
> > > Add constants for counts of HWTSTAMP_TX_* and HWTSTAM_FILTER_* constants
> > > and provide symbolic names for timestamping related values so that they can
> > > be retrieved in GET_STRSET and GET_INFO requests.
> >
> > What's the reason for providing the symbolic names?
>
> One of the the goals I had was to reduce the need to keep the lists of
> possible values in sync between kernel and userspace ethtool and other
> users of the interface so that when a new value is added, we don't have
> to update all userspace tools to be able to use or present it.
>
> This already works in ethtool for some newer commands (e.g. features)
> and obviously for those where the list of available options depends on
> the device (e.g. private flags or statistics). I would like to extend
> the principle also to older commands and new ones which do not work like
> this (e.g. device reset).
Let me try to argue that's the wrong direction. People should learn to
update their user space tooling if they want access to new features.
In my (limited) experience trying to solve forward compatibility leads
to short term gains, and long term warts in the APIs and increased
maintenance burden in the kernel.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, seccomp: fix false positive preemption splat for cbpf->ebpf progs
From: Alexei Starovoitov @ 2019-02-20 18:34 UTC (permalink / raw)
To: Daniel Borkmann; +Cc: Alexei Starovoitov, Network Development
In-Reply-To: <15b7c010-0635-35e4-dac8-0d811a496cd7@iogearbox.net>
On Wed, Feb 20, 2019 at 10:27 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 02/20/2019 06:07 PM, Alexei Starovoitov wrote:
> > On Wed, Feb 20, 2019 at 12:06:29PM +0100, Daniel Borkmann wrote:
> >> In 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
> >> a check was added for BPF_PROG_RUN() that for every invocation preemption has
> >> to be disabled to not break eBPF assumptions (e.g. per-cpu map). Of course this
> >> does not count for seccomp because only cBPF -> eBPF is loaded here and it does
> >> not make use of any functionality that would require this assertion. Fix this
> >> false positive by adding and using __BPF_PROG_RUN() variant that does not have
> >> the cant_sleep(); check.
> >>
> >> Fixes: 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
> >> Reported-by: syzbot+8bf19ee2aa580de7a2a7@syzkaller.appspotmail.com
> >> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
> >> ---
> >> include/linux/filter.h | 9 ++++++++-
> >> kernel/seccomp.c | 2 +-
> >> 2 files changed, 9 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/include/linux/filter.h b/include/linux/filter.h
> >> index f32b3ec..2648fd7 100644
> >> --- a/include/linux/filter.h
> >> +++ b/include/linux/filter.h
> >> @@ -533,7 +533,14 @@ struct sk_filter {
> >> struct bpf_prog *prog;
> >> };
> >>
> >> -#define BPF_PROG_RUN(filter, ctx) ({ cant_sleep(); (*(filter)->bpf_func)(ctx, (filter)->insnsi); })
> >> +#define bpf_prog_run__non_preempt(prog, ctx) \
> >> + ({ cant_sleep(); __BPF_PROG_RUN(prog, ctx); })
> >> +/* Native eBPF or cBPF -> eBPF transitions. Preemption must be disabled. */
> >> +#define BPF_PROG_RUN(prog, ctx) \
> >> + bpf_prog_run__non_preempt(prog, ctx)
> >> +/* Direct use for cBPF -> eBPF only, but not for native eBPF. */
> >
> > I think the comment is too abstract.
> > May be it should say that this is seccomp cBPF only ?
> > And macro name should be explicit as well ?
>
> I think macro naming is probably okay imho as used internally as
> well from BPF_PROG_RUN(), but I'll improve the comment to state
> seccomp specifically as an example there and providing some more
> background.
I'm worried about misuse of the macro.
If there was a word seccomp in it it would made people
think much harder before calling it.
^ permalink raw reply
* Re: [PATCH bpf-next] bpf, seccomp: fix false positive preemption splat for cbpf->ebpf progs
From: Daniel Borkmann @ 2019-02-20 18:27 UTC (permalink / raw)
To: Alexei Starovoitov; +Cc: ast, netdev
In-Reply-To: <20190220170723.bbcj7bipsa6r7oy6@ast-mbp>
On 02/20/2019 06:07 PM, Alexei Starovoitov wrote:
> On Wed, Feb 20, 2019 at 12:06:29PM +0100, Daniel Borkmann wrote:
>> In 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
>> a check was added for BPF_PROG_RUN() that for every invocation preemption has
>> to be disabled to not break eBPF assumptions (e.g. per-cpu map). Of course this
>> does not count for seccomp because only cBPF -> eBPF is loaded here and it does
>> not make use of any functionality that would require this assertion. Fix this
>> false positive by adding and using __BPF_PROG_RUN() variant that does not have
>> the cant_sleep(); check.
>>
>> Fixes: 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
>> Reported-by: syzbot+8bf19ee2aa580de7a2a7@syzkaller.appspotmail.com
>> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
>> ---
>> include/linux/filter.h | 9 ++++++++-
>> kernel/seccomp.c | 2 +-
>> 2 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/filter.h b/include/linux/filter.h
>> index f32b3ec..2648fd7 100644
>> --- a/include/linux/filter.h
>> +++ b/include/linux/filter.h
>> @@ -533,7 +533,14 @@ struct sk_filter {
>> struct bpf_prog *prog;
>> };
>>
>> -#define BPF_PROG_RUN(filter, ctx) ({ cant_sleep(); (*(filter)->bpf_func)(ctx, (filter)->insnsi); })
>> +#define bpf_prog_run__non_preempt(prog, ctx) \
>> + ({ cant_sleep(); __BPF_PROG_RUN(prog, ctx); })
>> +/* Native eBPF or cBPF -> eBPF transitions. Preemption must be disabled. */
>> +#define BPF_PROG_RUN(prog, ctx) \
>> + bpf_prog_run__non_preempt(prog, ctx)
>> +/* Direct use for cBPF -> eBPF only, but not for native eBPF. */
>
> I think the comment is too abstract.
> May be it should say that this is seccomp cBPF only ?
> And macro name should be explicit as well ?
I think macro naming is probably okay imho as used internally as
well from BPF_PROG_RUN(), but I'll improve the comment to state
seccomp specifically as an example there and providing some more
background.
>> +#define __BPF_PROG_RUN(prog, ctx) \
>> + (*(prog)->bpf_func)(ctx, (prog)->insnsi)
>>
>> #define BPF_SKB_CB_LEN QDISC_CB_PRIV_LEN
>>
>> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
>> index e815781..826d4e4 100644
>> --- a/kernel/seccomp.c
>> +++ b/kernel/seccomp.c
>> @@ -268,7 +268,7 @@ static u32 seccomp_run_filters(const struct seccomp_data *sd,
>> * value always takes priority (ignoring the DATA).
>> */
>> for (; f; f = f->prev) {
>> - u32 cur_ret = BPF_PROG_RUN(f->prog, sd);
>> + u32 cur_ret = __BPF_PROG_RUN(f->prog, sd);
>>
>> if (ACTION_ONLY(cur_ret) < ACTION_ONLY(ret)) {
>> ret = cur_ret;
>> --
>> 2.9.5
>>
^ permalink raw reply
* Re: BUG: assuming atomic context at kernel/seccomp.c:LINE
From: Kees Cook @ 2019-02-20 18:23 UTC (permalink / raw)
To: Daniel Borkmann
Cc: syzbot, Alexei Starovoitov, kafai, LKML, Andy Lutomirski,
Network Development, Song Liu, syzkaller-bugs, Will Drewry,
Yonghong Song
In-Reply-To: <69ff36f9-8729-9b58-5595-1b35aa4a7825@iogearbox.net>
On Wed, Feb 20, 2019 at 2:00 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 02/20/2019 10:32 AM, syzbot wrote:
> > Hello,
> >
> > syzbot found the following crash on:
> >
> > HEAD commit: abf446c90405 Add linux-next specific files for 20190220
> > git tree: linux-next
> > console output: https://syzkaller.appspot.com/x/log.txt?x=17f250d8c00000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=463cb576ac40e350
> > dashboard link: https://syzkaller.appspot.com/bug?extid=8bf19ee2aa580de7a2a7
> > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> >
> > Unfortunately, I don't have any reproducer for this crash yet.
> >
> > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > Reported-by: syzbot+8bf19ee2aa580de7a2a7@syzkaller.appspotmail.com
> >
> > BUG: assuming atomic context at kernel/seccomp.c:271
> > in_atomic(): 0, irqs_disabled(): 0, pid: 12803, name: syz-executor.5
> > no locks held by syz-executor.5/12803.
> > CPU: 1 PID: 12803 Comm: syz-executor.5 Not tainted 5.0.0-rc7-next-20190220 #39
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> > Call Trace:
> > __dump_stack lib/dump_stack.c:77 [inline]
> > dump_stack+0x172/0x1f0 lib/dump_stack.c:113
> > __cant_sleep kernel/sched/core.c:6218 [inline]
> > __cant_sleep.cold+0xa3/0xbb kernel/sched/core.c:6195
> > seccomp_run_filters kernel/seccomp.c:271 [inline]
> > __seccomp_filter+0x12b/0x12b0 kernel/seccomp.c:801
> > __secure_computing+0x101/0x360 kernel/seccomp.c:932
> > syscall_trace_enter+0x5bf/0xe10 arch/x86/entry/common.c:120
> > do_syscall_64+0x479/0x610 arch/x86/entry/common.c:280
> > entry_SYSCALL_64_after_hwframe+0x49/0xbe
>
> False positive; bpf-next only. Pushing this out in a bit:
>
> From d56547070162a105ff666f3324e558fa6492aedd Mon Sep 17 00:00:00 2001
> From: Daniel Borkmann <daniel@iogearbox.net>
> Date: Wed, 20 Feb 2019 10:51:17 +0100
> Subject: [PATCH bpf-next] bpf, seccomp: fix false positive preemption splat for
> cbpf->ebpf progs
>
> In 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
> a check was added for BPF_PROG_RUN() that for every invocation preemption is
> disabled to not break eBPF assumptions (e.g. per-cpu map). Of course this does
> not count for seccomp because only cBPF -> eBPF is loaded here and it does not
> make use of any functionality that would require this assertion. Fix this false
> positive by adding and using __BPF_PROG_RUN() variant that does not have the
> cant_sleep(); check.
>
> Fixes: 568f196756ad ("bpf: check that BPF programs run with preemption disabled")
> Reported-by: syzbot+8bf19ee2aa580de7a2a7@syzkaller.appspotmail.com
> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net>
Acked-by: Kees Cook <keescook@chromium.org>
-Kees
> ---
> include/linux/filter.h | 9 ++++++++-
> kernel/seccomp.c | 2 +-
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index f32b3ec..2f3e29a 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -533,7 +533,14 @@ struct sk_filter {
> struct bpf_prog *prog;
> };
>
> -#define BPF_PROG_RUN(filter, ctx) ({ cant_sleep(); (*(filter)->bpf_func)(ctx, (filter)->insnsi); })
> +#define bpf_prog_run__non_preempt(prog, ctx) \
> + ({ cant_sleep(); __BPF_PROG_RUN(prog, ctx); })
> +/* Native eBPF or cBPF -> eBPF transitions. Preemption must be disabled. */
> +#define BPF_PROG_RUN(prog, ctx) \
> + bpf_prog_run__non_preempt(prog, ctx)
> +/* cBPF -> eBPF only, but not for native eBPF. */
> +#define __BPF_PROG_RUN(prog, ctx) \
> + (*(prog)->bpf_func)(ctx, (prog)->insnsi)
>
> #define BPF_SKB_CB_LEN QDISC_CB_PRIV_LEN
>
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index e815781..826d4e4 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -268,7 +268,7 @@ static u32 seccomp_run_filters(const struct seccomp_data *sd,
> * value always takes priority (ignoring the DATA).
> */
> for (; f; f = f->prev) {
> - u32 cur_ret = BPF_PROG_RUN(f->prog, sd);
> + u32 cur_ret = __BPF_PROG_RUN(f->prog, sd);
>
> if (ACTION_ONLY(cur_ret) < ACTION_ONLY(ret)) {
> ret = cur_ret;
> --
> 2.9.5
--
Kees Cook
^ permalink raw reply
* [PATCH] net: dsa: add missing phy address offset
From: Marcel Reichmuth @ 2019-02-20 18:15 UTC (permalink / raw)
To: linux-kernel@vger.kernel.org, netdev@vger.kernel.org
Cc: andrew@lunn.ch, vivien.didelot@gmail.com, f.fainelli@gmail.com,
davem@davemloft.net, Marcel Reichmuth
When phys do not start at address 0 like on the mv88e6341 the wrong
phy address is used and therefore the slave ports can not be
initialized. This patch adds the proper offset to the phy address.
Signed-off-by: Marcel Reichmuth <marcel.reichmuth@netmodule.com>
---
drivers/net/dsa/mv88e6xxx/chip.c | 3 +++
include/net/dsa.h | 1 +
net/dsa/slave.c | 3 ++-
3 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 12fd7ce3f1ff..0ca649f784d2 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -2198,12 +2198,15 @@ static int mv88e6xxx_setup_upstream_port(struct mv88e6xxx_chip *chip, int port)
static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
{
struct dsa_switch *ds = chip->ds;
+ struct dsa_port *dp = &ds->ports[port];
int err;
u16 reg;
chip->ports[port].chip = chip;
chip->ports[port].port = port;
+ dp->phy_base_addr = chip->info->phy_base_addr;
+
/* MAC Forcing register: don't force link, speed, duplex or flow control
* state to any particular values on physical ports, but force the CPU
* port and all DSA ports to their maximum bandwidth and full duplex.
diff --git a/include/net/dsa.h b/include/net/dsa.h
index b3eefe8e18fd..f9c9dc1f6d21 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -196,6 +196,7 @@ struct dsa_port {
struct dsa_switch *ds;
unsigned int index;
+ unsigned int phy_base_addr;
const char *name;
const struct dsa_port *cpu_dp;
struct device_node *dn;
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index a1c9fe155057..4f67dff34a3b 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -1221,7 +1221,8 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev)
/* We could not connect to a designated PHY or SFP, so use the
* switch internal MDIO bus instead
*/
- ret = dsa_slave_phy_connect(slave_dev, dp->index);
+ ret = dsa_slave_phy_connect(slave_dev, dp->phy_base_addr +
+ dp->index);
if (ret) {
netdev_err(slave_dev,
"failed to connect to port %d: %d\n",
--
2.11.0
^ permalink raw reply related
* Re: [RESEND PATCH 0/7] Add FOLL_LONGTERM to GUP fast and use it
From: Ira Weiny @ 2019-02-20 18:02 UTC (permalink / raw)
To: Christoph Hellwig
Cc: John Hubbard, Andrew Morton, Michal Hocko, Kirill A. Shutemov,
Peter Zijlstra, Jason Gunthorpe, Benjamin Herrenschmidt,
Paul Mackerras, David S. Miller, Martin Schwidefsky,
Heiko Carstens, Rich Felker, Yoshinori Sato, Thomas Gleixner,
Ingo Molnar, Borislav Petkov, Ralf Baechle, Paul Burton,
James Hogan, linux-kernel, linux-mm, linux-mips, linuxppc-dev,
linux-s390, linux-sh, sparclinux, kvm-ppc, kvm, linux-fpga,
dri-devel, linux-rdma, linux-media, linux-scsi, devel,
virtualization, netdev, linux-fbdev, xen-devel, devel, ceph-devel,
rds-devel
In-Reply-To: <20190220151930.GB11695@infradead.org>
On Wed, Feb 20, 2019 at 07:19:30AM -0800, Christoph Hellwig wrote:
> On Tue, Feb 19, 2019 at 09:30:33PM -0800, ira.weiny@intel.com wrote:
> > From: Ira Weiny <ira.weiny@intel.com>
> >
> > Resending these as I had only 1 minor comment which I believe we have covered
> > in this series. I was anticipating these going through the mm tree as they
> > depend on a cleanup patch there and the IB changes are very minor. But they
> > could just as well go through the IB tree.
> >
> > NOTE: This series depends on my clean up patch to remove the write parameter
> > from gup_fast_permitted()[1]
> >
> > HFI1, qib, and mthca, use get_user_pages_fast() due to it performance
> > advantages. These pages can be held for a significant time. But
> > get_user_pages_fast() does not protect against mapping of FS DAX pages.
>
> This I don't get - if you do lock down long term mappings performance
> of the actual get_user_pages call shouldn't matter to start with.
>
> What do I miss?
A couple of points.
First "longterm" is a relative thing and at this point is probably a misnomer.
This is really flagging a pin which is going to be given to hardware and can't
move. I've thought of a couple of alternative names but I think we have to
settle on if we are going to use FL_LAYOUT or something else to solve the
"longterm" problem. Then I think we can change the flag to a better name.
Second, It depends on how often you are registering memory. I have spoken with
some RDMA users who consider MR in the performance path... For the overall
application performance. I don't have the numbers as the tests for HFI1 were
done a long time ago. But there was a significant advantage. Some of which is
probably due to the fact that you don't have to hold mmap_sem.
Finally, architecturally I think it would be good for everyone to use *_fast.
There are patches submitted to the RDMA list which would allow the use of
*_fast (they reworking the use of mmap_sem) and as soon as they are accepted
I'll submit a patch to convert the RDMA core as well. Also to this point
others are looking to use *_fast.[2]
As an asside, Jasons pointed out in my previous submission that *_fast and
*_unlocked look very much the same. I agree and I think further cleanup will
be coming. But I'm focused on getting the final solution for DAX at the
moment.
Ira
^ permalink raw reply
* Re: [PATCH] iwlwifi: mvm: Use div64_s64 instead of do_div in iwl_mvm_debug_range_resp
From: Nathan Chancellor @ 2019-02-20 17:56 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Johannes Berg, Emmanuel Grumbach, Luca Coelho,
Intel Linux Wireless, Kalle Valo, linux-wireless, Networking,
Linux Kernel Mailing List, Nick Desaulniers
In-Reply-To: <CAK8P3a1d6Zksdv-Or+paawyn4N0u4EpVw9QgyDGHYvX_1s3JrQ@mail.gmail.com>
On Wed, Feb 20, 2019 at 11:51:34AM +0100, Arnd Bergmann wrote:
> On Tue, Feb 19, 2019 at 7:22 PM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
>
> > diff --git a/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c b/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c
> > index e9822a3ec373..92b22250eb7d 100644
> > --- a/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c
> > +++ b/drivers/net/wireless/intel/iwlwifi/mvm/ftm-initiator.c
> > @@ -462,7 +462,7 @@ static void iwl_mvm_debug_range_resp(struct iwl_mvm *mvm, u8 index,
> > {
> > s64 rtt_avg = res->ftm.rtt_avg * 100;
> >
> > - do_div(rtt_avg, 6666);
> > + div64_s64(rtt_avg, 6666);
>
> This is wrong: div64_s64 does not modify its argument like do_div(), but
> it returns the result instead. You also don't want to divide by a 64-bit
> value when the second argument is a small constant.
>
> I think the correct way should be
>
> s64 rtt_avg = div_s64(res->ftm.rtt_avg * 100, 6666);
>
> If you know that the value is positive, using unsigned types
> and div_u64() would be slightly faster.
>
> Arnd
Thanks for the review and explanation, Arnd.
Luca, could you drop this version so I can resend it?
Nathan
^ permalink raw reply
* Re: [PATCH][next] rtlwifi: rtl8192ce: fix typo, "PairwiseENcAlgorithm" -> "PairwiseEncAlgorithm"
From: Kalle Valo @ 2019-02-20 17:55 UTC (permalink / raw)
To: Colin King
Cc: Ping-Ke Shih, David S . Miller, Larry Finger, linux-wireless,
netdev, kernel-janitors, linux-kernel
In-Reply-To: <20190220092727.17119-1-colin.king@canonical.com>
Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> There is an uppercase 'N' that should be a lowercase 'n', fix this.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Patch applied to wireless-drivers-next.git, thanks.
0421dd4167ec rtlwifi: rtl8192ce: fix typo, "PairwiseENcAlgorithm" -> "PairwiseEncAlgorithm"
--
https://patchwork.kernel.org/patch/10821751/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH net-next v3 3/3] net: dsa: enable flooding for bridge ports
From: Russell King - ARM Linux admin @ 2019-02-20 17:33 UTC (permalink / raw)
To: Vivien Didelot
Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller,
netdev
In-Reply-To: <20190220122355.GB32192@t480s.localdomain>
On Wed, Feb 20, 2019 at 12:23:55PM -0500, Vivien Didelot wrote:
> Hi Russell,
>
> On Wed, 20 Feb 2019 12:36:59 +0000, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> > Switches work by learning the MAC address for each attached station by
> > monitoring traffic from each station. When a station sends a packet,
> > the switch records which port the MAC address is connected to.
> >
> > With IPv4 networking, before communication commences with a neighbour,
> > an ARP packet is broadcasted to all stations asking for the MAC address
> > corresponding with the IPv4. The desired station responds with an ARP
> > reply, and the ARP reply causes the switch to learn which port the
> > station is connected to.
> >
> > With IPv6 networking, the situation is rather different. Rather than
> > broadcasting ARP packets, a "neighbour solicitation" is multicasted
> > rather than broadcasted. This multicast needs to reach the intended
> > station in order for the neighbour to be discovered.
> >
> > Once a neighbour has been discovered, and entered into the sending
> > stations neighbour cache, communication can restart at a point later
> > without sending a new neighbour solicitation, even if the entry in
> > the neighbour cache is marked as stale. This can be after the MAC
> > address has expired from the forwarding cache of the DSA switch -
> > when that occurs, there is a long pause in communication.
> >
> > Our DSA implementation for mv88e6xxx switches disables flooding of
> > multicast and unicast frames for bridged ports. As per the above
> > description, this is fine for IPv4 networking, since the broadcasted
> > ARP queries will be sent to and received by all stations on the same
> > network. However, this breaks IPv6 very badly - blocking neighbour
> > solicitations and later causing connections to stall.
> >
> > The defaults that the Linux bridge code expect from bridges are for
> > unknown unicast and unknown multicast frames to be flooded to all ports
> > on the bridge, which is at odds to the defaults adopted by our DSA
> > implementation for mv88e6xxx switches.
> >
> > This commit enables by default flooding of both unknown unicast and
> > unknown multicast frames whenever a port is added to a bridge, and
> > disables the flooding when a port leaves the bridge. This means that
> > mv88e6xxx DSA switches now behave as per the bridge(8) man page, and
> > IPv6 works flawlessly through such a switch.
> >
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> > ---
> > net/dsa/port.c | 12 +++++++++++-
> > 1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/dsa/port.c b/net/dsa/port.c
> > index b84d010fb165..9e7aab13957e 100644
> > --- a/net/dsa/port.c
> > +++ b/net/dsa/port.c
> > @@ -105,6 +105,11 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br)
> > };
> > int err;
> >
> > + /* Set the flooding mode before joining */
>
> Note that as stated by the comment just below, the port has already joined
> the bridge here.
The software interface has joined at this point, but not the physical
port itself. I actually find that the statement in the comment below
this code is the confusing statement.
>
> > + err = dsa_port_bridge_flags(dp, BR_FLOOD | BR_MCAST_FLOOD, NULL);
> > + if (err)
> > + return err;
> > +
> > /* Here the port is already bridged. Reflect the current configuration
> > * so that drivers can program their chips accordingly.
This is the confusing statement: the switch port is not bridged at
this point since we haven't programmed the hardware to make that happen.
The Linux interface corresponding to the switch port is what has been
bridged.
> > */
> > @@ -113,8 +118,10 @@ int dsa_port_bridge_join(struct dsa_port *dp, struct net_device *br)
> > err = dsa_port_notify(dp, DSA_NOTIFIER_BRIDGE_JOIN, &info);
> >
> > /* The bridging is rolled back on error */
> > - if (err)
> > + if (err) {
> > + dsa_port_bridge_flags(dp, 0, NULL);
> > dp->bridge_dev = NULL;
> > + }
> >
> > return err;
> > }
> > @@ -137,6 +144,9 @@ void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
> > if (err)
> > pr_err("DSA: failed to notify DSA_NOTIFIER_BRIDGE_LEAVE\n");
> >
> > + /* Port is leaving the bridge, disable flooding */
> > + dsa_port_bridge_flags(dp, BR_LEARNING, NULL);
Hmm, that should've been 0 not BR_LEARNING since we are not dealing
with that flag yet. I'll send a v4 later this evening.
> > +
> > /* Port left the bridge, put in BR_STATE_DISABLED by the bridge layer,
> > * so allow it to be in BR_STATE_FORWARDING to be kept functional
> > */
>
>
> This makes it clear that we must add this logic which sets the expected
> default flags into the bridge code itself. But this can be done later.
Note that the above is carefully chosen to ensure that the port is
configured for flooding whenever traffic may pass in bridge mode,
which I suggest is something that is kept in future.
>
>
> Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>
>
--
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
* Re: [PATCH net-next v3 1/3] net: dsa: add support for bridge flags
From: Vivien Didelot @ 2019-02-20 17:30 UTC (permalink / raw)
To: Russell King
Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller,
netdev
In-Reply-To: <E1gwR7I-0003iS-Kg@rmk-PC.armlinux.org.uk>
Hi Russell,
On Wed, 20 Feb 2019 12:36:48 +0000, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> The Linux bridge implementation allows various properties of the bridge
> to be controlled, such as flooding unknown unicast and multicast frames.
> This patch adds the necessary DSA infrastructure to allow the Linux
> bridge support to control these properties for DSA switches.
>
> We implement this by providing two new methods: one to get the switch-
> wide support bitmask, and another to set the properties.
This is not true anymore ;-)
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> include/net/dsa.h | 2 ++
> net/dsa/dsa_priv.h | 2 ++
> net/dsa/port.c | 16 ++++++++++++++++
> net/dsa/slave.c | 6 ++++++
> 4 files changed, 26 insertions(+)
>
> diff --git a/include/net/dsa.h b/include/net/dsa.h
> index 7f2a668ef2cc..2c2c10812814 100644
> --- a/include/net/dsa.h
> +++ b/include/net/dsa.h
> @@ -400,6 +400,8 @@ struct dsa_switch_ops {
> void (*port_stp_state_set)(struct dsa_switch *ds, int port,
> u8 state);
> void (*port_fast_age)(struct dsa_switch *ds, int port);
> + int (*port_egress_floods)(struct dsa_switch *ds, int port,
> + bool unicast, bool multicast);
>
> /*
> * VLAN support
> diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
> index 1f4972dab9f2..f4f99ec29f5d 100644
> --- a/net/dsa/dsa_priv.h
> +++ b/net/dsa/dsa_priv.h
> @@ -160,6 +160,8 @@ int dsa_port_mdb_add(const struct dsa_port *dp,
> struct switchdev_trans *trans);
> int dsa_port_mdb_del(const struct dsa_port *dp,
> const struct switchdev_obj_port_mdb *mdb);
> +int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags,
> + struct switchdev_trans *trans);
> int dsa_port_vlan_add(struct dsa_port *dp,
> const struct switchdev_obj_port_vlan *vlan,
> struct switchdev_trans *trans);
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index 2d7e01b23572..b84d010fb165 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -177,6 +177,22 @@ int dsa_port_ageing_time(struct dsa_port *dp, clock_t ageing_clock,
> return dsa_port_notify(dp, DSA_NOTIFIER_AGEING_TIME, &info);
> }
>
> +int dsa_port_bridge_flags(const struct dsa_port *dp, unsigned long flags,
> + struct switchdev_trans *trans)
> +{
> + struct dsa_switch *ds = dp->ds;
> + int port = dp->index;
> +
> + if (switchdev_trans_ph_prepare(trans))
> + return 0;
> +
> + if (ds->ops->port_egress_floods)
> + ds->ops->port_egress_floods(ds, port, flags & BR_FLOOD,
> + flags & BR_MCAST_FLOOD);
Even though we're not supposed to fail in the switchdev commit phase, I'd
prefer not to ignore the return code.
> +
> + return 0;
> +}
> +
> int dsa_port_fdb_add(struct dsa_port *dp, const unsigned char *addr,
> u16 vid)
> {
> diff --git a/net/dsa/slave.c b/net/dsa/slave.c
> index 2e5e7c04821b..f99161c3b1ea 100644
> --- a/net/dsa/slave.c
> +++ b/net/dsa/slave.c
> @@ -295,6 +295,9 @@ static int dsa_slave_port_attr_set(struct net_device *dev,
> case SWITCHDEV_ATTR_ID_BRIDGE_AGEING_TIME:
> ret = dsa_port_ageing_time(dp, attr->u.ageing_time, trans);
> break;
> + case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS:
> + ret = dsa_port_bridge_flags(dp, attr->u.brport_flags, trans);
> + break;
> default:
> ret = -EOPNOTSUPP;
> break;
> @@ -384,6 +387,9 @@ static int dsa_slave_port_attr_get(struct net_device *dev,
> switch (attr->id) {
> case SWITCHDEV_ATTR_ID_PORT_BRIDGE_FLAGS_SUPPORT:
> attr->u.brport_flags_support = 0;
> + if (ds->ops->port_egress_floods)
> + attr->u.brport_flags_support |= BR_FLOOD |
> + BR_MCAST_FLOOD;
> break;
> default:
> return -EOPNOTSUPP;
Otherwise, LGTM:
Reviewed-by: Vivien Didelot <vivien.didelot@gmail.com>
^ permalink raw reply
* Re: [PATCH net] net: dsa: fix unintended change of bridge interface STP state
From: Andrew Lunn @ 2019-02-20 17:27 UTC (permalink / raw)
To: Florian Fainelli; +Cc: Russell King, Vivien Didelot, David S. Miller, netdev
In-Reply-To: <b7f73796-8545-c953-d71b-c0b0adb85965@gmail.com>
On Wed, Feb 20, 2019 at 09:22:30AM -0800, Florian Fainelli wrote:
> On 2/20/19 2:32 AM, Russell King wrote:
> > When a DSA port is added to a bridge and brought up, the resulting STP
> > state programmed into the hardware depends on the order that these
> > operations are performed. However, the Linux bridge code believes that
> > the port is in disabled mode.
> >
> > If the DSA port is first added to a bridge and then brought up, it will
> > be in blocking mode. If it is brought up and then added to the bridge,
> > it will be in disabled mode.
> >
> > This difference is caused by DSA always setting the STP mode in
> > dsa_port_enable() whether or not this port is part of a bridge. Since
> > bridge always sets the STP state when the port is added, brought up or
> > taken down, it is unnecessary for us to manipulate the STP state.
> >
> > Apparently, this code was copied from Rocker, and the very next day a
> > similar fix for Rocker was merged but was not propagated to DSA. See
> > e47172ab7e41 ("rocker: put port in FORWADING state after leaving bridge")
> >
> > Fixes: b73adef67765 ("net: dsa: integrate with SWITCHDEV for HW bridging")
> > Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
>
> Nice example of cargo cult programming, thanks for fixing this!
Maybe now would be a good time to look at other drivers. Does the
Microsemi Ocelot driver have the same issue?
Andrew
^ permalink raw reply
* Re: [PATCH net-next v3 2/3] net: dsa: mv88e6xxx: add support for bridge flags
From: Vivien Didelot @ 2019-02-20 17:26 UTC (permalink / raw)
To: Russell King
Cc: Andrew Lunn, Florian Fainelli, Heiner Kallweit, David S. Miller,
netdev
In-Reply-To: <E1gwR7O-0003ig-0P@rmk-PC.armlinux.org.uk>
Hi Russell,
On Wed, 20 Feb 2019 12:36:54 +0000, Russell King <rmk+kernel@armlinux.org.uk> wrote:
> Add support for the bridge flags to Marvell 88e6xxx bridges, allowing
> the multicast and unicast flood properties to be controlled. These
> can be controlled on a per-port basis via commands such as:
>
> bridge link set dev lan1 flood on|off
> bridge link set dev lan1 mcast_flood on|off
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> drivers/net/dsa/mv88e6xxx/chip.c | 19 +++++++++++++++++++
> 1 file changed, 19 insertions(+)
>
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 32e7af5caa69..937370639fe4 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -4692,6 +4692,24 @@ static int mv88e6xxx_port_mdb_del(struct dsa_switch *ds, int port,
> return err;
> }
>
> +static int mv88e6xxx_port_egress_floods(struct dsa_switch *ds, int port,
> + bool unicast, bool multicast)
> +{
> + struct mv88e6xxx_chip *chip = ds->priv;
> + int ret = -EOPNOTSUPP;
I'd prefer "err" like in the rest of the driver.
> +
> + WARN_ON_ONCE(dsa_is_cpu_port(ds, port) || dsa_is_dsa_port(ds, port));
No need to warn, the driver does not need to care about which port is targeted
by a call.
> +
> + mutex_lock(&chip->reg_lock);
> + if (chip->info->ops->port_set_egress_floods)
> + ret = chip->info->ops->port_set_egress_floods(chip, port,
> + unicast,
> + multicast);
> + mutex_unlock(&chip->reg_lock);
> +
> + return ret;
> +}
> +
> static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
> #if IS_ENABLED(CONFIG_NET_DSA_LEGACY)
> .probe = mv88e6xxx_drv_probe,
> @@ -4719,6 +4737,7 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
> .set_ageing_time = mv88e6xxx_set_ageing_time,
> .port_bridge_join = mv88e6xxx_port_bridge_join,
> .port_bridge_leave = mv88e6xxx_port_bridge_leave,
> + .port_egress_floods = mv88e6xxx_port_egress_floods,
> .port_stp_state_set = mv88e6xxx_port_stp_state_set,
> .port_fast_age = mv88e6xxx_port_fast_age,
> .port_vlan_filtering = mv88e6xxx_port_vlan_filtering,
Otherwise it looks good to me.
^ 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