* [PATCH net 0/4] s390/qeth: fixes 2017-12-13
From: Julian Wiedmann @ 2017-12-13 17:56 UTC (permalink / raw)
To: David Miller
Cc: netdev, linux-s390, Martin Schwidefsky, Heiko Carstens,
Stefan Raspl, Ursula Braun, Julian Wiedmann
Hi Dave,
some more patches for 4.15, that fix multiple issues with IP Takeover
configuration in qeth.
Please queue them up for stable kernels as well (4.9 and newer).
Thanks,
Julian
Julian Wiedmann (4):
s390/qeth: apply takeover changes when mode is toggled
s390/qeth: don't apply takeover changes to RXIP
s390/qeth: lock IP table while applying takeover changes
s390/qeth: update takeover IPs after configuration change
drivers/s390/net/qeth_core.h | 6 ++--
drivers/s390/net/qeth_core_main.c | 6 ++--
drivers/s390/net/qeth_l3.h | 2 +-
drivers/s390/net/qeth_l3_main.c | 36 ++++++++++++++++---
drivers/s390/net/qeth_l3_sys.c | 75 +++++++++++++++++++++------------------
5 files changed, 79 insertions(+), 46 deletions(-)
--
2.13.5
^ permalink raw reply
* Re: [PATCH] ipv6: ip6mr: Recalc UDP checksum before forwarding
From: Eric Dumazet @ 2017-12-13 17:52 UTC (permalink / raw)
To: Brendan McGrath, David S . Miller, netdev, linux-kernel
In-Reply-To: <1513164048-21368-1-git-send-email-redmcg@redmandi.dyndns.org>
On Wed, 2017-12-13 at 22:20 +1100, Brendan McGrath wrote:
> Currently, when forwarding from a Virtual Interface to a Physical
> Interface, ip_summed is set to a value of CHECKSUM_UNNECESSARY and
> the UDP checksum has not been calculated.
>
This seems a bug then ?
CHECKSUM_UNNECESSARY means checksum has been validated.
Not that we want it being computed later in the stack.
If we force a checksum here, what guarantee do we have packet was not
corrupted before we do this ?
^ permalink raw reply
* Re: [RFC][PATCH] new byteorder primitives - ..._{replace,get}_bits()
From: Al Viro @ 2017-12-13 17:45 UTC (permalink / raw)
To: Jakub Kicinski; +Cc: Linus Torvalds, netdev, linux-kernel
In-Reply-To: <20171213142212.GD21978@ZenIV.linux.org.uk>
On Wed, Dec 13, 2017 at 02:22:12PM +0000, Al Viro wrote:
> Next question: where do we put that bunch? I've put it into
> linux/byteorder/generic.h, so that anything picking fixed-endian primitives
> would pick those as well; I hadn't thought of linux/bitfield.h at the time.
> We certainly could put it there instead - it's never pulled by other headers,
> so adding #include <asm/byteorder.h> into linux/bitfield.h is not going to
> cause header order problems. Not sure...
>
> Linus, do you have any preferences in that area?
After looking at some of the callers of bitfield.h stuff: it might be useful
to add
static inline void le64p_replace_bits(__le64 *p, u64 v, u64 mask)
{
__le64 m = cpu_to_le64(mask);
*p = (*p & ~m) | (cpu_to_le64(v * mask_to_multiplier(mask)) & m);
}
and similar for other types. Not sure what would be a good name for
host-endian variants - u64p_replace_bits() sounds a bit clumsy. Suggestions?
^ permalink raw reply
* BUG REPORT: iproute2 seems to have bug with dsfield/tos in ip-rule and ip-route
From: Daniel Lakeland @ 2017-12-13 17:40 UTC (permalink / raw)
To: netdev
This same problem as detailed here
http://lists.openwall.net/netdev/2010/03/26/36
or here:
https://www.spinics.net/lists/lartc/msg22541.html
bit me today
I tried either
ip rule add dsfield CS6 table 100
or
ip rule add dsfield 0xc0 table 100
or replace dsfield with tos, all return:
RTNETLINK answers: Invalid argument
on the other hand, for ip route it will accept the ds/tos values
ip route add default dsfield CS6 dev dummy0
or
ip route add default dsfield 0xc0 dev dummy0
but packets tagged with CS6 don't go to dummy0 they go the regular
default route
^ permalink raw reply
* Re: [patch net-next v3 00/10] net: sched: allow qdiscs to share filter block instances
From: Jiri Pirko @ 2017-12-13 17:39 UTC (permalink / raw)
To: David Ahern
Cc: netdev, davem, jhs, xiyou.wangcong, mlxsw, andrew, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, saeedm, matanb, leonro,
idosch, jakub.kicinski, simon.horman, pieter.jansenvanvuuren,
john.hurley, alexander.h.duyck, ogerlitz, john.fastabend, daniel
In-Reply-To: <90bf2450-a21c-9f70-2dc3-b147d0c40740@gmail.com>
Wed, Dec 13, 2017 at 06:18:04PM CET, dsahern@gmail.com wrote:
>On 12/13/17 10:07 AM, Jiri Pirko wrote:
>> Wed, Dec 13, 2017 at 05:54:35PM CET, dsahern@gmail.com wrote:
>>> On 12/13/17 8:10 AM, Jiri Pirko wrote:
>>>> So back to the example. First, we create 2 qdiscs. Both will share
>>>> block number 22. "22" is just an identification. If we don't pass any
>>>> block number, a new one will be generated by kernel:
>>>>
>>>> $ tc qdisc add dev ens7 ingress block 22
>>>> ^^^^^^^^
>>>> $ tc qdisc add dev ens8 ingress block 22
>>>> ^^^^^^^^
>>>>
>>>> Now if we list the qdiscs, we will see the block index in the output:
>>>>
>>>> $ tc qdisc
>>>> qdisc ingress ffff: dev ens7 parent ffff:fff1 block 22
>>>> qdisc ingress ffff: dev ens8 parent ffff:fff1 block 22
>>>>
>>>> To make is more visual, the situation looks like this:
>>>>
>>>> ens7 ingress qdisc ens7 ingress qdisc
>>>> | |
>>>> | |
>>>> +----------> block 22 <----------+
>>>>
>>>> Unlimited number of qdiscs may share the same block.
>>>>
>>>> Now we can add filter to any of qdiscs sharing the same block:
>>>>
>>>> $ tc filter add dev ens7 ingress protocol ip pref 25 flower dst_ip 192.168.0.0/16 action drop
>>>
>>> I still say this is very odd user semantic - making changes to device M
>>> and the changes magically affect device N. Operating on the shared block
>>> as a separate object makes it is much more direct and clear.
>>
>> I plan to do it as a follow-up patch. But this is how things are done
>> now and have to continue to work.
>
>Why is that? You are introducing the notion of a shared block with this
>patch set. What is the legacy "how things are done now" you are
>referring to?
Well, the filter add/del should just work no matter if the block behind is
shared or not.
>
>> Also changes done on dev block X for dev A has to appear in block X
>> for dev B. Block X is share between A and B.
>>
>
>Certainly - that's the definition of a shared block and you are
>referring to display and datapath. For admin, it is more direct and
>apparent in terms of what is happening to require changes (filter add
>and deletes) to be done by specifying the shared block as the primary
>object.
^ permalink raw reply
* [PATCH 2/3] net: phy: select sensible mode for non-autoneg PHYs on startup
From: Lucas Stach @ 2017-12-13 17:37 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev, kernel, patchwork-lst
In-Reply-To: <20171213173751.12722-1-l.stach@pengutronix.de>
Init speed and duplex to unknown, so phy_lookup_setting() knows that
it should select the mode only based on the PHY allowed link modes.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/net/phy/phy-core.c | 5 +++++
drivers/net/phy/phy_device.c | 2 ++
2 files changed, 7 insertions(+)
diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c
index 21f75ae244b3..149a4bab1e6f 100644
--- a/drivers/net/phy/phy-core.c
+++ b/drivers/net/phy/phy-core.c
@@ -149,6 +149,11 @@ phy_lookup_setting(int speed, int duplex, const unsigned long *mask,
const struct phy_setting *p, *match = NULL, *last = NULL;
int i;
+ if (!exact && speed == SPEED_UNKNOWN)
+ speed = INT_MAX;
+ if (!exact && duplex == DUPLEX_UNKNOWN)
+ duplex = DUPLEX_FULL;
+
for (i = 0, p = settings; i < ARRAY_SIZE(settings); i++, p++) {
if (p->bit < maxbit && test_bit(p->bit, mask)) {
last = p;
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 8ef48b38d97b..35278282259a 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1785,6 +1785,8 @@ static int phy_probe(struct device *dev)
phydev->supported = phydrv->features;
of_set_phy_supported(phydev);
phydev->advertising = phydev->supported;
+ phydev->speed = SPEED_UNKNOWN;
+ phydev->duplex = DUPLEX_UNKNOWN;
/* Get the EEE modes we want to prohibit. We will ask
* the PHY stop advertising these mode later on
--
2.11.0
^ permalink raw reply related
* [PATCH 1/3] net: phy: add support to detect 100BASE-T1 capability
From: Lucas Stach @ 2017-12-13 17:37 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev, kernel, patchwork-lst
100BASE-T1 is the automotive ethernet standard 802.3bw-2015. Currently
we don't detect any valid modes for PHYs, which only support this
standard. Add support to detect the common 100Mbit full-duplex mode.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/net/phy/phy_device.c | 2 ++
include/uapi/linux/mii.h | 1 +
2 files changed, 3 insertions(+)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 67f25ac29025..8ef48b38d97b 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -1607,6 +1607,8 @@ int genphy_config_init(struct phy_device *phydev)
if (val < 0)
return val;
+ if (val & ESTATUS_100T1_FULL)
+ features |= SUPPORTED_100baseT_Full;
if (val & ESTATUS_1000_TFULL)
features |= SUPPORTED_1000baseT_Full;
if (val & ESTATUS_1000_THALF)
diff --git a/include/uapi/linux/mii.h b/include/uapi/linux/mii.h
index b5c2fdcf23fd..eb5cc45d23fb 100644
--- a/include/uapi/linux/mii.h
+++ b/include/uapi/linux/mii.h
@@ -121,6 +121,7 @@
#define EXPANSION_MFAULTS 0x0010 /* Multiple faults detected */
#define EXPANSION_RESV 0xffe0 /* Unused... */
+#define ESTATUS_100T1_FULL 0x0080 /* Can do 100BASE-T1 Full */
#define ESTATUS_1000_TFULL 0x2000 /* Can do 1000BT Full */
#define ESTATUS_1000_THALF 0x1000 /* Can do 1000BT Half */
--
2.11.0
^ permalink raw reply related
* [PATCH 3/3] net: phy: sanitize autoneg in phy_start_aneg_priv
From: Lucas Stach @ 2017-12-13 17:37 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli; +Cc: netdev, kernel, patchwork-lst
In-Reply-To: <20171213173751.12722-1-l.stach@pengutronix.de>
phy_sanitize_settings() is only called when autonegotiation has been
explicitly disabled. This breaks PHYs without any autonegotiation
capability, as the check for the capability happens inside this function.
Move the check out to the caller, so it is properly applied for those
PHYs.
Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
drivers/net/phy/phy.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c
index 2b1e67bc1e73..433d859b6955 100644
--- a/drivers/net/phy/phy.c
+++ b/drivers/net/phy/phy.c
@@ -226,10 +226,6 @@ static void phy_sanitize_settings(struct phy_device *phydev)
const struct phy_setting *setting;
u32 features = phydev->supported;
- /* Sanitize settings based on PHY capabilities */
- if ((features & SUPPORTED_Autoneg) == 0)
- phydev->autoneg = AUTONEG_DISABLE;
-
setting = phy_find_valid(phydev->speed, phydev->duplex, features);
if (setting) {
phydev->speed = setting->speed;
@@ -487,6 +483,10 @@ static int phy_start_aneg_priv(struct phy_device *phydev, bool sync)
mutex_lock(&phydev->lock);
+ /* Sanitize settings based on PHY capabilities */
+ if ((phydev->supported & SUPPORTED_Autoneg) == 0)
+ phydev->autoneg = AUTONEG_DISABLE;
+
if (AUTONEG_DISABLE == phydev->autoneg)
phy_sanitize_settings(phydev);
--
2.11.0
^ permalink raw reply related
* Re: [PATCH net-next] bpf/tracing: fix kernel/events/core.c compilation error
From: Yonghong Song @ 2017-12-13 17:26 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, sfr, netdev; +Cc: kernel-team
In-Reply-To: <475c2645-01fc-7f25-da78-7ff180760b97@fb.com>
On 12/13/17 7:50 AM, Alexei Starovoitov wrote:
> On 12/13/17 7:44 AM, Daniel Borkmann wrote:
>> On 12/13/2017 08:42 AM, Yonghong Song wrote:
>>> Commit f371b304f12e ("bpf/tracing: allow user space to
>>> query prog array on the same tp") introduced a perf
>>> ioctl command to query prog array attached to the
>>> same perf tracepoint. The commit introduced a
>>> compilation error when either CONFIG_BPF_SYSCALL or
>>> CONFIG_EVENT_TRACING is not defined:
>>> kernel/events/core.o: In function `perf_ioctl':
>>> core.c:(.text+0x98c4): undefined reference to
>>> `bpf_event_query_prog_array'
>>>
>>> This patch fixed this error.
>>>
>>> Fixes: f371b304f12e ("bpf/tracing: allow user space to query prog
>>> array on the same tp")
>>> Reported-by: Stephen Rothwell <sfr@canb.auug.org.au>
>>> Signed-off-by: Yonghong Song <yhs@fb.com>
>>
>> Looking at _perf_ioctl(), we also have perf_event_set_bpf_prog()
>> there. It's basically under CONFIG_EVENT_TRACING, which later calls
>> perf_event_attach_bpf_prog() which is under CONFIG_BPF_EVENTS, so
>> where we have the dummy handler returning -EOPNOTSUPP when BPF
>> events is not set. bpf_trace.c is only built when CONFIG_BPF_EVENTS
>> is set and that by itself depends on BPF_SYSCALL already. So it would
>> be more correct to do the same thing here ...
>>
>> #if defined(CONFIG_EVENT_TRACING) && defined(CONFIG_BPF_EVENTS)
>> [...]
>
> +1
> #ifdef CONFIG_BPF_EVENTS
> works, whereas CONFIG_EVENT_TRACING probably not, since kprobe
> can be disabled independently which will turn off BPF_EVENTS
> and body of bpf_event_query_prog_array() will be gone.
I tested to enable/disable uprobe/kprobe/both and my patch works.
But I did not test enable a non uprobe/kprobe tracing event
(e.g., CONFIG_FUNCTION_TRACER) where CONFIG_TRACING and
CONFIG_EVENT_TRACING is on but CONFIG_UPROBES_EVENT/CONFIG_KPROBES_EVENT
is off and then my patch breaks.
Looks like
#ifdef CONFIG_BPF_EVENTS
is suffice.
This config will enable to include bpf_trace.c with the real definition.
It will depend on KPROBE_EVENTS or UPROBE_EVENTS and either
of them will enable CONFIG_TRACING and then CONFIG_EVENT_TRACING.
Will resubmit the patch after testing.
^ permalink raw reply
* Re: [PATCH] ipv6: ip6mr: Recalc UDP checksum before forwarding
From: Marcelo Ricardo Leitner @ 2017-12-13 17:21 UTC (permalink / raw)
To: Brendan McGrath
Cc: David S . Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, netdev,
linux-kernel
In-Reply-To: <1513164048-21368-1-git-send-email-redmcg@redmandi.dyndns.org>
Hi,
On Wed, Dec 13, 2017 at 10:20:48PM +1100, Brendan McGrath wrote:
> Currently, when forwarding from a Virtual Interface to a Physical
> Interface, ip_summed is set to a value of CHECKSUM_UNNECESSARY and
> the UDP checksum has not been calculated.
>
> When the packet is then forwarded by a Multicast Router, the checksum
> value is left as is and therefore rejected by the receiving
> machine(s).
>
> This patch ensures the checksum is recalculated before forwarding.
>
> Signed-off-by: Brendan McGrath <redmcg@redmandi.dyndns.org>
> ---
>
> It's a bit ugly putting UDP specific code in this spot - but I'm not
> aware of any other protocols that are:
> a) multicast;
> b) forwarded; and
> c) checksummed
>
> net/ipv6/ip6mr.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
> index 890f9bda..ee4370a 100644
> --- a/net/ipv6/ip6mr.c
> +++ b/net/ipv6/ip6mr.c
> @@ -2077,6 +2077,13 @@ static int ip6mr_forward2(struct net *net, struct mr6_table *mrt,
> ipv6h = ipv6_hdr(skb);
> ipv6h->hop_limit--;
>
> + if (ipv6h->nexthdr == NEXTHDR_UDP &&
> + skb->ip_summed != CHECKSUM_PARTIAL) {
^
This indentation is wrong. The 2nd line should start right after the (
column in the 1st line, like:
+ if (ipv6h->nexthdr == NEXTHDR_UDP &&
+ skb->ip_summed != CHECKSUM_PARTIAL) {
Adjust with spaces as needed.
Running the patch through scripts/checkpatch.pl before posting will
catch these.
> + struct udphdr *uh = udp_hdr(skb);
> + udp6_set_csum(false, skb, &ipv6_hdr(skb)->saddr,
> + &ipv6_hdr(skb)->daddr, ntohs(uh->len));
^ same deal here.
> + }
> +
> IP6CB(skb)->flags |= IP6SKB_FORWARDED;
>
> return NF_HOOK(NFPROTO_IPV6, NF_INET_FORWARD,
> --
> 2.7.4
>
^ permalink raw reply
* Re: [PATCH net] net: phy: fix resume handling
From: Andrew Lunn @ 2017-12-13 17:19 UTC (permalink / raw)
To: Russell King; +Cc: Florian Fainelli, netdev
In-Reply-To: <E1eOi48-0002N2-QC@rmk-PC.armlinux.org.uk>
On Tue, Dec 12, 2017 at 10:45:36AM +0000, Russell King wrote:
> When a PHY has the BMCR_PDOWN bit set, it may decide to ignore writes
> to other registers, or reset the registers to power-on defaults.
> Micrel PHYs do this for their interrupt registers.
>
> The current structure of phylib tries to enable interrupts before
> resuming (and releasing) the BMCR_PDOWN bit. This fails, causing
> Micrel PHYs to stop working after a suspend/resume sequence if they
> are using interrupts.
>
> Fix this by ensuring that the PHY driver resume methods do not take
> the phydev->lock mutex themselves, but the callers of phy_resume()
> take that lock. This then allows us to move the call to phy_resume()
> before we enable interrupts in phy_start().
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [patch net-next v3 00/10] net: sched: allow qdiscs to share filter block instances
From: David Ahern @ 2017-12-13 17:18 UTC (permalink / raw)
To: Jiri Pirko
Cc: netdev, davem, jhs, xiyou.wangcong, mlxsw, andrew, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, saeedm, matanb, leonro,
idosch, jakub.kicinski, simon.horman, pieter.jansenvanvuuren,
john.hurley, alexander.h.duyck, ogerlitz, john.fastabend, daniel
In-Reply-To: <20171213170757.GJ2031@nanopsycho>
On 12/13/17 10:07 AM, Jiri Pirko wrote:
> Wed, Dec 13, 2017 at 05:54:35PM CET, dsahern@gmail.com wrote:
>> On 12/13/17 8:10 AM, Jiri Pirko wrote:
>>> So back to the example. First, we create 2 qdiscs. Both will share
>>> block number 22. "22" is just an identification. If we don't pass any
>>> block number, a new one will be generated by kernel:
>>>
>>> $ tc qdisc add dev ens7 ingress block 22
>>> ^^^^^^^^
>>> $ tc qdisc add dev ens8 ingress block 22
>>> ^^^^^^^^
>>>
>>> Now if we list the qdiscs, we will see the block index in the output:
>>>
>>> $ tc qdisc
>>> qdisc ingress ffff: dev ens7 parent ffff:fff1 block 22
>>> qdisc ingress ffff: dev ens8 parent ffff:fff1 block 22
>>>
>>> To make is more visual, the situation looks like this:
>>>
>>> ens7 ingress qdisc ens7 ingress qdisc
>>> | |
>>> | |
>>> +----------> block 22 <----------+
>>>
>>> Unlimited number of qdiscs may share the same block.
>>>
>>> Now we can add filter to any of qdiscs sharing the same block:
>>>
>>> $ tc filter add dev ens7 ingress protocol ip pref 25 flower dst_ip 192.168.0.0/16 action drop
>>
>> I still say this is very odd user semantic - making changes to device M
>> and the changes magically affect device N. Operating on the shared block
>> as a separate object makes it is much more direct and clear.
>
> I plan to do it as a follow-up patch. But this is how things are done
> now and have to continue to work.
Why is that? You are introducing the notion of a shared block with this
patch set. What is the legacy "how things are done now" you are
referring to?
> Also changes done on dev block X for dev A has to appear in block X
> for dev B. Block X is share between A and B.
>
Certainly - that's the definition of a shared block and you are
referring to display and datapath. For admin, it is more direct and
apparent in terms of what is happening to require changes (filter add
and deletes) to be done by specifying the shared block as the primary
object.
^ permalink raw reply
* Re: [PATCH iproute2] Show 'external' link mode in output
From: Phil Dibowitz @ 2017-12-13 17:15 UTC (permalink / raw)
To: Sergei Shtylyov, netdev; +Cc: phild
In-Reply-To: <da953234-0678-f0bb-1a73-265b0b506c1c@cogentembedded.com>
[-- Attachment #1.1: Type: text/plain, Size: 556 bytes --]
On 12/13/2017 02:05 AM, Sergei Shtylyov wrote:
> Hello!
Hi!
> The original code is indented with tabs, you're using spaces. Not
> good. :-)
I sent a v2 of the patch later yesterday that fixed this.
Thanks!
--
Phil Dibowitz phil@ipom.com
Open Source software and tech docs Insanity Palace of Metallica
http://www.phildev.net/ http://www.ipom.com/
"Be who you are and say what you feel, because those who mind don't
matter and those who matter don't mind."
- Dr. Seuss
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 854 bytes --]
^ permalink raw reply
* Re: [PATCH] of_mdio / mdiobus: ensure mdio devices have fwnode correctly populated
From: Andrew Lunn @ 2017-12-13 17:15 UTC (permalink / raw)
To: Russell King
Cc: Florian Fainelli, Rob Herring, Frank Rowand, netdev, devicetree
In-Reply-To: <E1eOi7f-0002Rs-K7@rmk-PC.armlinux.org.uk>
On Tue, Dec 12, 2017 at 10:49:15AM +0000, Russell King wrote:
> Ensure that all mdio devices populate the struct device fwnode pointer
> as well as the of_node pointer to allow drivers that wish to use
> fwnode APIs to work.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH] net: igmp: Use correct source address on IGMPv3 reports
From: Andrew Lunn @ 2017-12-13 17:15 UTC (permalink / raw)
To: Kevin Cernekee; +Cc: davem, kuznet, yoshfuji, netdev, linux-kernel
In-Reply-To: <20171211191345.104136-1-cernekee@chromium.org>
On Mon, Dec 11, 2017 at 11:13:45AM -0800, Kevin Cernekee wrote:
> Closing a multicast socket after the final IPv4 address is deleted
> from an interface can generate a membership report that uses the
> source IP from a different interface. The following test script, run
> from an isolated netns, reproduces the issue:
>
> #!/bin/bash
>
> ip link add dummy0 type dummy
> ip link add dummy1 type dummy
> ip link set dummy0 up
> ip link set dummy1 up
> ip addr add 10.1.1.1/24 dev dummy0
> ip addr add 192.168.99.99/24 dev dummy1
>
> tcpdump -U -i dummy0 &
> socat EXEC:"sleep 2" \
> UDP4-DATAGRAM:239.101.1.68:8889,ip-add-membership=239.0.1.68:10.1.1.1 &
>
> sleep 1
> ip addr del 10.1.1.1/24 dev dummy0
> sleep 5
> kill %tcpdump
>
> RFC 3376 specifies that the report must be sent with a valid IP source
> address from the destination subnet, or from address 0.0.0.0. Add an
> extra check to make sure this is the case.
>
> Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [patch net-next v3 00/10] net: sched: allow qdiscs to share filter block instances
From: Jiri Pirko @ 2017-12-13 17:07 UTC (permalink / raw)
To: David Ahern
Cc: netdev, davem, jhs, xiyou.wangcong, mlxsw, andrew, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, saeedm, matanb, leonro,
idosch, jakub.kicinski, simon.horman, pieter.jansenvanvuuren,
john.hurley, alexander.h.duyck, ogerlitz, john.fastabend, daniel
In-Reply-To: <04bcfa37-a74e-9e2f-3ac1-7ed8e63e13df@gmail.com>
Wed, Dec 13, 2017 at 05:54:35PM CET, dsahern@gmail.com wrote:
>On 12/13/17 8:10 AM, Jiri Pirko wrote:
>> So back to the example. First, we create 2 qdiscs. Both will share
>> block number 22. "22" is just an identification. If we don't pass any
>> block number, a new one will be generated by kernel:
>>
>> $ tc qdisc add dev ens7 ingress block 22
>> ^^^^^^^^
>> $ tc qdisc add dev ens8 ingress block 22
>> ^^^^^^^^
>>
>> Now if we list the qdiscs, we will see the block index in the output:
>>
>> $ tc qdisc
>> qdisc ingress ffff: dev ens7 parent ffff:fff1 block 22
>> qdisc ingress ffff: dev ens8 parent ffff:fff1 block 22
>>
>> To make is more visual, the situation looks like this:
>>
>> ens7 ingress qdisc ens7 ingress qdisc
>> | |
>> | |
>> +----------> block 22 <----------+
>>
>> Unlimited number of qdiscs may share the same block.
>>
>> Now we can add filter to any of qdiscs sharing the same block:
>>
>> $ tc filter add dev ens7 ingress protocol ip pref 25 flower dst_ip 192.168.0.0/16 action drop
>
>I still say this is very odd user semantic - making changes to device M
>and the changes magically affect device N. Operating on the shared block
>as a separate object makes it is much more direct and clear.
I plan to do it as a follow-up patch. But this is how things are done
now and have to continue to work.
Also changes done on dev block X for dev A has to appear in block X
for dev B. Block X is share between A and B.
^ permalink raw reply
* Re: [PATCH net-next v2 5/5] net: phy: phylink: Report MoCA as PORT_BNC
From: Andrew Lunn @ 2017-12-13 16:59 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, rmk+kernel, davem, vivien.didelot
In-Reply-To: <20171213000029.8649-6-f.fainelli@gmail.com>
On Tue, Dec 12, 2017 at 04:00:29PM -0800, Florian Fainelli wrote:
> Similarly to what PHYLIB already does, make sure that
> PHY_INTERFACE_MODE_MOCA is reported as PORT_BNC.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next v2 4/5] net: phy: phylink: Allow setting a custom link state callback
From: Andrew Lunn @ 2017-12-13 16:58 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, rmk+kernel, davem, vivien.didelot
In-Reply-To: <20171213000029.8649-5-f.fainelli@gmail.com>
On Tue, Dec 12, 2017 at 04:00:28PM -0800, Florian Fainelli wrote:
> phylink_get_fixed_state() currently consults an optional "link_gpio"
> GPIO descriptor, expand this mechanism to allow specifying a custom
> callback. This is necessary to support out of band link notifcation
> (e.g: from an interrupt within a MMIO register).
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next v2 3/5] net: phy: phylink: Remove error message
From: Andrew Lunn @ 2017-12-13 16:56 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, rmk+kernel, davem, vivien.didelot
In-Reply-To: <20171213000029.8649-4-f.fainelli@gmail.com>
On Tue, Dec 12, 2017 at 04:00:27PM -0800, Florian Fainelli wrote:
> Some subsystems like DSA may be trying to connect to a PHY through OF first,
> and then attempt a connect using a local MDIO bus, remove the error message:
> "unable to find PHY node" so we can let MAC drivers whether to print it or not.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH net-next v2 1/5] net: phy: phylink: Allow specifying PHY device flags
From: Andrew Lunn @ 2017-12-13 16:54 UTC (permalink / raw)
To: Florian Fainelli; +Cc: netdev, rmk+kernel, davem, vivien.didelot
In-Reply-To: <20171213000029.8649-2-f.fainelli@gmail.com>
On Tue, Dec 12, 2017 at 04:00:25PM -0800, Florian Fainelli wrote:
> In order to let subsystems like DSA fully utilize PHYLINK, we need to be able
> to communicate phy_device::flags from of_phy_{connect,attach} even when using
> PHYLINK APIs.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [patch net-next v3 00/10] net: sched: allow qdiscs to share filter block instances
From: David Ahern @ 2017-12-13 16:54 UTC (permalink / raw)
To: Jiri Pirko, netdev
Cc: davem, jhs, xiyou.wangcong, mlxsw, andrew, vivien.didelot,
f.fainelli, michael.chan, ganeshgr, saeedm, matanb, leonro,
idosch, jakub.kicinski, simon.horman, pieter.jansenvanvuuren,
john.hurley, alexander.h.duyck, ogerlitz, john.fastabend, daniel
In-Reply-To: <20171213151038.29665-1-jiri@resnulli.us>
On 12/13/17 8:10 AM, Jiri Pirko wrote:
> So back to the example. First, we create 2 qdiscs. Both will share
> block number 22. "22" is just an identification. If we don't pass any
> block number, a new one will be generated by kernel:
>
> $ tc qdisc add dev ens7 ingress block 22
> ^^^^^^^^
> $ tc qdisc add dev ens8 ingress block 22
> ^^^^^^^^
>
> Now if we list the qdiscs, we will see the block index in the output:
>
> $ tc qdisc
> qdisc ingress ffff: dev ens7 parent ffff:fff1 block 22
> qdisc ingress ffff: dev ens8 parent ffff:fff1 block 22
>
> To make is more visual, the situation looks like this:
>
> ens7 ingress qdisc ens7 ingress qdisc
> | |
> | |
> +----------> block 22 <----------+
>
> Unlimited number of qdiscs may share the same block.
>
> Now we can add filter to any of qdiscs sharing the same block:
>
> $ tc filter add dev ens7 ingress protocol ip pref 25 flower dst_ip 192.168.0.0/16 action drop
I still say this is very odd user semantic - making changes to device M
and the changes magically affect device N. Operating on the shared block
as a separate object makes it is much more direct and clear.
>
>
> We will see the same output if we list filters for ens7 and ens8, including stats:
>
> $ tc -s filter show dev ens7 ingress
> filter protocol ip pref 25 flower chain 0
> filter protocol ip pref 25 flower chain 0 handle 0x1
> eth_type ipv4
> dst_ip 192.168.0.0/16
> not_in_hw
> action order 1: gact action drop
> random type none pass val 0
> index 1 ref 1 bind 1 installed 39 sec used 2 sec
> Action statistics:
> Sent 3108 bytes 37 pkt (dropped 37, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
>
> $ tc -s filter show dev ens8 ingress
> filter protocol ip pref 25 flower chain 0
> filter protocol ip pref 25 flower chain 0 handle 0x1
> eth_type ipv4
> dst_ip 192.168.0.0/16
> not_in_hw
> action order 1: gact action drop
> random type none pass val 0
> index 1 ref 1 bind 1 installed 40 sec used 3 sec
> Action statistics:
> Sent 3108 bytes 37 pkt (dropped 37, overlimits 0 requeues 0)
> backlog 0b 0p requeues 0
>
^ permalink raw reply
* Re: [PATCH v2 net-next 0/3] add VLAN support to DSA MT7530
From: Andrew Lunn @ 2017-12-13 16:50 UTC (permalink / raw)
To: sean.wang
Cc: davem, f.fainelli, vivien.didelot, netdev, linux-kernel,
linux-mediatek
In-Reply-To: <cover.1513136754.git.sean.wang@mediatek.com>
On Wed, Dec 13, 2017 at 12:01:10PM +0800, sean.wang@mediatek.com wrote:
> From: Sean Wang <sean.wang@mediatek.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [BUG] skge: a possible sleep-in-atomic bug in skge_remove
From: Stephen Hemminger @ 2017-12-13 16:50 UTC (permalink / raw)
To: Jia-Ju Bai
Cc: David Miller, mlindner, shemminger, shemminger, netdev,
linux-kernel
In-Reply-To: <f2dee82a-af75-a3af-9899-5dec7950b9e8@gmail.com>
On Wed, 13 Dec 2017 15:42:56 +0800
Jia-Ju Bai <baijiaju1990@gmail.com> wrote:
> On 2017/12/13 13:18, Stephen Hemminger wrote:
> > On Tue, 12 Dec 2017 20:57:01 -0500 (EST)
> > David Miller <davem@davemloft.net> wrote:
> >
> >> From: Stephen Hemminger <stephen@networkplumber.org>
> >> Date: Tue, 12 Dec 2017 10:22:40 -0800
> >>
> >>> On Tue, 12 Dec 2017 08:34:45 -0500 (EST)
> >>> David Miller <davem@davemloft.net> wrote:
> >>>
> >>>> From: Jia-Ju Bai <baijiaju1990@gmail.com>
> >>>> Date: Tue, 12 Dec 2017 16:38:12 +0800
> >>>>
> >>>>> According to drivers/net/ethernet/marvell/skge.c, the driver may sleep
> >>>>> under a spinlock.
> >>>>> The function call path is:
> >>>>> skge_remove (acquire the spinlock)
> >>>>> free_irq --> may sleep
> >>>>>
> >>>>> I do not find a good way to fix it, so I only report.
> >>>>> This possible bug is found by my static analysis tool (DSAC) and
> >>>>> checked by my code review.
> >>>> This was added by:
> >>>>
> >>>> commit a9e9fd7182332d0cf5f3e601df3e71dd431b70d7
> >>>> Author: Stephen Hemminger <shemminger@vyatta.com>
> >>>> Date: Tue Sep 27 13:41:37 2011 -0400
> >>>>
> >>>> skge: handle irq better on single port card
> >>>>
> >>>> I think the free_irq() can be moved below the unlock.
> >>>>
> >>>> Stephen, please take a look.
> >>> The IRQ was being free twice.
> >>> How did you see it, I really doubt any multi-port SKGE cards
> >>> still exist.
> >> He sees it by reading the code, please take a look at this
> >> and move the free_irq() out of the spin locked section since
> >> it can sleep.
> > Thanks, I was hoping for some automated static analysis tool.
>
> This bug was found by an automated static analysis tool named DSAC,
> which is written by myself.
> Then I manually checked driver source code, and finally sent the bug report.
Thanks.
Would it be possible to put tool in tools directory and then have
it automated by kbuild robot?
^ permalink raw reply
* Re: [PATCH] net: phy: marvell: avoid configuring fiber page for SGMII-to-Copper
From: Andrew Lunn @ 2017-12-13 16:46 UTC (permalink / raw)
To: Russell King; +Cc: Florian Fainelli, Jon Nettleton, netdev
In-Reply-To: <E1eP3Ep-0000ZC-UN@rmk-PC.armlinux.org.uk>
On Wed, Dec 13, 2017 at 09:22:03AM +0000, Russell King wrote:
> When in SGMII-to-Copper mode, the fiber page is used for the MAC facing
> link, and does not require configuration of the fiber auto-negotiation
> settings. Avoid trying.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH] net: phy: marvell: avoid pause mode on SGMII-to-Copper for 88e151x
From: Andrew Lunn @ 2017-12-13 16:44 UTC (permalink / raw)
To: Russell King; +Cc: Florian Fainelli, Jon Nettleton, netdev
In-Reply-To: <E1eP3Ev-0000ZJ-1L@rmk-PC.armlinux.org.uk>
On Wed, Dec 13, 2017 at 09:22:09AM +0000, Russell King wrote:
> Observed on the 88e1512 in SGMII-to-Copper mode, negotiating pause
> is unreliable. While the pause bits can be set in the advertisment
> register, they clear shortly after negotiation with a link partner
> commences irrespective of the cause of the negotiation.
>
> While these bits may be correctly conveyed to the link partner on the
> first negotiation, a subsequent negotiation (eg, due to negotiation
> restart by the link partner, or reconnection of the cable) will result
> in the link partner seeing these bits as zero, while the kernel
> believes that it has advertised pause modes.
>
> This leads to the local kernel evaluating (eg) symmetric pause mode,
> while the remote end evaluates that we have no pause mode capability.
>
> Since we can't guarantee the advertisment, disable pause mode support
> with this PHY when used in SGMII-to-Copper mode.
>
> The 88e1510 in RGMII-to-Copper mode appears to behave correctly.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ 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