* Re: [PATCH v2 net-next 1/9] lan743x: Add support for ethtool get_drvinfo
From: Andrew Lunn @ 2018-07-12 22:28 UTC (permalink / raw)
To: Bryan Whitehead; +Cc: davem, netdev, UNGLinuxDriver, richardcochran
In-Reply-To: <1531422306-6547-2-git-send-email-Bryan.Whitehead@microchip.com>
On Thu, Jul 12, 2018 at 03:04:58PM -0400, Bryan Whitehead wrote:
> Implement ethtool get_drvinfo
>
> Signed-off-by: Bryan Whitehead <Bryan.Whitehead@microchip.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH v2 net-next 2/9] lan743x: Add support for ethtool link settings
From: Andrew Lunn @ 2018-07-12 22:29 UTC (permalink / raw)
To: Bryan Whitehead; +Cc: davem, netdev, UNGLinuxDriver, richardcochran
In-Reply-To: <1531422306-6547-3-git-send-email-Bryan.Whitehead@microchip.com>
On Thu, Jul 12, 2018 at 03:04:59PM -0400, Bryan Whitehead wrote:
> Use default link setting functions
>
> Signed-off-by: Bryan Whitehead <Bryan.Whitehead@microchip.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH v2 net-next 3/9] lan743x: Add support for ethtool statistics
From: Andrew Lunn @ 2018-07-12 22:31 UTC (permalink / raw)
To: Bryan Whitehead; +Cc: davem, netdev, UNGLinuxDriver, richardcochran
In-Reply-To: <1531422306-6547-4-git-send-email-Bryan.Whitehead@microchip.com>
On Thu, Jul 12, 2018 at 03:05:00PM -0400, Bryan Whitehead wrote:
> Implement ethtool statistics
>
> Signed-off-by: Bryan Whitehead <Bryan.Whitehead@microchip.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH v2 net-next 4/9] lan743x: Add support for ethtool message level
From: Andrew Lunn @ 2018-07-12 22:31 UTC (permalink / raw)
To: Bryan Whitehead; +Cc: davem, netdev, UNGLinuxDriver, richardcochran
In-Reply-To: <1531422306-6547-5-git-send-email-Bryan.Whitehead@microchip.com>
On Thu, Jul 12, 2018 at 03:05:01PM -0400, Bryan Whitehead wrote:
> Implement ethtool message level
>
> Signed-off-by: Bryan Whitehead <Bryan.Whitehead@microchip.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH v2 net-next 5/9] lan743x: Add support for ethtool eeprom access
From: Andrew Lunn @ 2018-07-12 22:35 UTC (permalink / raw)
To: Bryan Whitehead; +Cc: davem, netdev, UNGLinuxDriver, richardcochran
In-Reply-To: <1531422306-6547-6-git-send-email-Bryan.Whitehead@microchip.com>
On Thu, Jul 12, 2018 at 03:05:02PM -0400, Bryan Whitehead wrote:
> Implement ethtool eeprom access
> Also provides access to OTP (One Time Programming)
>
> Signed-off-by: Bryan Whitehead <Bryan.Whitehead@microchip.com>
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Andrew
^ permalink raw reply
* Re: [PATCH v2 net-next 6/9] lan743x: Add power management support
From: Andrew Lunn @ 2018-07-12 22:42 UTC (permalink / raw)
To: Bryan Whitehead; +Cc: davem, netdev, UNGLinuxDriver, richardcochran
In-Reply-To: <1531422306-6547-7-git-send-email-Bryan.Whitehead@microchip.com>
> +#ifdef CONFIG_PM
> +static void lan743x_ethtool_get_wol(struct net_device *netdev,
> + struct ethtool_wolinfo *wol)
> +{
> + struct lan743x_adapter *adapter = netdev_priv(netdev);
> +
> + wol->supported = WAKE_BCAST | WAKE_UCAST | WAKE_MCAST |
> + WAKE_MAGIC | WAKE_PHY | WAKE_ARP;
> +
> + wol->wolopts = adapter->wolopts;
> +}
> +#endif /* CONFIG_PM */
> +
> +#ifdef CONFIG_PM
> +static int lan743x_ethtool_set_wol(struct net_device *netdev,
> + struct ethtool_wolinfo *wol)
> +{
> + struct lan743x_adapter *adapter = netdev_priv(netdev);
> +
> + if (wol->wolopts & WAKE_MAGICSECURE)
> + return -EOPNOTSUPP;
> +
> + adapter->wolopts = 0;
> + if (wol->wolopts & WAKE_UCAST)
> + adapter->wolopts |= WAKE_UCAST;
> + if (wol->wolopts & WAKE_MCAST)
> + adapter->wolopts |= WAKE_MCAST;
> + if (wol->wolopts & WAKE_BCAST)
> + adapter->wolopts |= WAKE_BCAST;
> + if (wol->wolopts & WAKE_MAGIC)
> + adapter->wolopts |= WAKE_MAGIC;
> + if (wol->wolopts & WAKE_PHY)
> + adapter->wolopts |= WAKE_PHY;
> + if (wol->wolopts & WAKE_ARP)
> + adapter->wolopts |= WAKE_ARP;
> +
> + device_set_wakeup_enable(&adapter->pdev->dev, (bool)wol->wolopts);
> +
> + phy_ethtool_set_wol(netdev->phydev, wol);
Hi Bryan
This seems asymmetric. set_wol you call into the phylib to enable wol
in the PHY. But get_wol does not call into phylib. So the phy has no
chance to set what it supports.
Andrew
^ permalink raw reply
* Re: [PATCH v2 net-next 7/9] lan743x: Add EEE support
From: Andrew Lunn @ 2018-07-12 22:49 UTC (permalink / raw)
To: Bryan Whitehead; +Cc: davem, netdev, UNGLinuxDriver, richardcochran
In-Reply-To: <1531422306-6547-8-git-send-email-Bryan.Whitehead@microchip.com>
> +static int lan743x_ethtool_set_eee(struct net_device *netdev,
> + struct ethtool_eee *eee)
> +{
> + struct lan743x_adapter *adapter = netdev_priv(netdev);
> + struct phy_device *phydev = NULL;
> + u32 buf = 0;
> + int ret = 0;
> +
> + if (!netdev)
> + return -EINVAL;
> + adapter = netdev_priv(netdev);
> + if (!adapter)
> + return -EINVAL;
> + phydev = netdev->phydev;
> + if (!phydev)
> + return -EIO;
> + if (!phydev->drv) {
> + netif_err(adapter, drv, adapter->netdev,
> + "Missing PHY Driver\n");
> + return -EIO;
> + }
> +
> + if (eee->eee_enabled) {
> + ret = phy_init_eee(phydev, 0);
> + if (ret) {
> + netif_err(adapter, drv, adapter->netdev,
> + "EEE initialization failed\n");
> + return ret;
> + }
> +
> + buf = lan743x_csr_read(adapter, MAC_CR);
> + buf |= MAC_CR_EEE_EN_;
> + lan743x_csr_write(adapter, MAC_CR, buf);
> +
> + phy_ethtool_set_eee(phydev, eee);
> +
> + buf = (u32)eee->tx_lpi_timer;
> + lan743x_csr_write(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT, buf);
> + netif_info(adapter, drv, adapter->netdev, "Enabled EEE\n");
> + } else {
> + buf = lan743x_csr_read(adapter, MAC_CR);
> + buf &= ~MAC_CR_EEE_EN_;
> + lan743x_csr_write(adapter, MAC_CR, buf);
> + netif_info(adapter, drv, adapter->netdev, "Disabled EEE\n");
> + }
> +
Hi Bryan
You should call phy_ethtool_set_eee() in both cases, so that it gets
disabled in the PHY as well. It needs to stop advertising it.
Andrew
^ permalink raw reply
* Proposal
From: Miss Victoria Mehmet @ 2018-07-12 22:46 UTC (permalink / raw)
To: Recipients
Hello
I have a business proposal of mutual benefits i would like to discuss with
you.
^ permalink raw reply
* Re: [BUG] bonded interfaces drop bpdu (stp) frames
From: Michal Soltys @ 2018-07-12 23:14 UTC (permalink / raw)
To: Jay Vosburgh,
Mahesh Bandewar (महेश बंडेवार)
Cc: Chonggang Li, linux-netdev
In-Reply-To: <30874.1531433032@nyx>
On 2018-07-13 00:03, Jay Vosburgh wrote:
> Mahesh Bandewar (महेश बंडेवार) wrote:
>
>>On Thu, Jul 12, 2018 at 11:03 AM, Jay Vosburgh
>><jay.vosburgh@canonical.com> wrote:
>>> Michal Soltys <soltys@ziu.info> wrote:
>>>
>>>>On 07/12/2018 04:51 PM, Jay Vosburgh wrote:
>>>>> Mahesh Bandewar (महेश बंडेवार) wrote:
>>>>>
>>>>>> On Wed, Jul 11, 2018 at 3:23 PM, Michal Soltys <soltys@ziu.info> wrote:
>>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> As weird as that sounds, this is what I observed today after bumping
>>>>>>> kernel version. I have a setup where 2 bonds are attached to linux
>>>>>>> bridge and physically are connected to two switches doing MSTP (and
>>>>>>> linux bridge is just passing them).
>>>>>>>
>>>>>>> Initially I suspected some changes related to bridge code - but quick
>>>>>>> peek at the code showed nothing suspicious - and the part of it that
>>>>>>> explicitly passes stp frames if stp is not enabled has seen little
>>>>>>> changes (e.g. per-port group_fwd_mask added recently). Furthermore - if
>>>>>>> regular non-bonded interfaces are attached everything works fine.
>>>>>>>
>>>>>>> Just to be sure I detached the bond (802.3ad mode) and checked it with
>>>>>>> simple tcpdump (ether proto \\stp) - and indeed no hello packets were
>>>>>>> there (with them being present just fine on active enslaved interface,
>>>>>>> or on the bond device in earlier kernels).
>>>>>>>
>>>>>>> If time permits I'll bisect tommorow to pinpoint the commit, but from
>>>>>>> quick todays test - 4.9.x is working fine, while 4.16.16 (tested on
>>>>>>> debian) and 4.17.3 (tested on archlinux) are failing.
>>>>>>>
>>>>>>> Unless this is already a known issue (or you have any suggestions what
>>>>>>> could be responsible).
>>>>>>>
>>>>>> I believe these are link-local-multicast messages and sometime back a
>>>>>> change went into to not pass those frames to the bonding master. This
>>>>>> could be the side effect of that.
>>>>>
>>>>> Mahesh, I suspect you're thinking of:
>>>>>
>>>>> commit b89f04c61efe3b7756434d693b9203cc0cce002e
>>>>> Author: Chonggang Li <chonggangli@google.com>
>>>>> Date: Sun Apr 16 12:02:18 2017 -0700
>>>>>
>>>>> bonding: deliver link-local packets with skb->dev set to link that packets arrived on
>>>>>
>>>>> Michal, are you able to revert this patch and test?
>>>>>
>>>>> -J
>>>>>
>>>>> ---
>>>>> -Jay Vosburgh, jay.vosburgh@canonical.com
>>>>>
>>>>
>>>>
>>>>Just tested - yes, reverting that patch solves the issues.
>>>
>>> Chonggang,
>>>
>>> Reading the changelog in your commit referenced above, I'm not
>>> entirely sure what actual problem it is fixing. Could you elaborate?
>>>
>>> As the patch appears to cause a regression, it needs to be
>>> either fixed or reverted.
>>>
>>> Mahesh, you signed-off on it as well, perhaps you also have some
>>> context?
>>>
>>
>>I think the original idea behind it was to pass the LLDPDUs to the
>>stack on the interface that they came on since this is considered to
>>be link-local traffic and passing to bond-master would loose it's
>>"linklocal-ness". This is true for LLDP and if you change the skb->dev
>>of the packet, then you don't know which slave link it came on in
>>(from LLDP consumer's perspective).
>>
>>I don't know much about STP but trunking two links and aggregating
>>this link info through bond-master seems wrong. Just like LLDP, you
>>are losing info specific to a link and the decision derived from that
>>info could be wrong.
>>
>>Having said that, we determine "linklocal-ness" by looking at L2 and
>>bondmaster shares this with lts slaves. So it does seem fair to pass
>>those frames to the bonding-master but at the same time link-local
>>traffic is supposed to be limited to the physical link (LLDP/STP/LACP
>>etc). Your thoughts?
>
> I agree the whole thing sounds kind of weird, but I'm curious as
> to what Michal's actual use case is; he presumably has some practical
> use for this, since he noticed that the behavior changed.
>
The whole "link-local" term is a bit I don't know - at this point it
feels like too many things were thrown into single bag and it got
somewhat confusing (bpdu, lldp, pause frames, lacp, pae, qinq mulitcast
that afaik has its own address) - I added some examples in another reply
I did at the same time as you were typing this one =)
> Michal, you mentioned MSTP and using 802.3ad (LACP) mode; how
> does that combination work rationally given that the bond might send and
> receive traffic across multiple slaves? Or does the switch side bundle
> the ports together into a single logical interface for MSTP purposes?
> On the TX side, I think the bond will likely balance all STP frames to
> just one slave.
>
The basic concept - two "main" switches with "important" machines
connected to those. One switch dies and everything keeps working. With
no unused ports and so on.
In more details:
Originally I was trying MSTP daemon (on "important" machines) which
seems quite well and completely coded, but cannot really work correctly
- as afaik you can't put port (in linux bridge conext) in different
forwarding/blocking/etc. state per-region - itow per group of vlans (or
mstpd didn't know how to do that, or it wasn't implemented - I didn't
look too deep back then, though my interest resurfaced in recent days).
So that option was out of the question. But any switch, real or not,
/must/ pass bpdu frames if it doesn't interpret them. So instead of
having active mstp participant, we have passive linux bridge that passes
the frames and the two real switches around that care of mstp, treating
the linux as a shared segment. The costs/priorities/etc. on the real
swtiches are set so one bond handles two regions, and the other bond
handles other two regions. If any of the real switches dies or is taken
down for e.g. firmware update - the bond going to the other switch
handles all four regions (failover is of course not as fast as with
active rstp/mstp participation, but works quite well none the less -
around 10s after some tuning).
We could have used RSTP for that purpose as well - but that being all or
nothing in context of per-port blocking/forwarding, would leave half of
the ports unused - and we wanted to avoid that (that's why MSTP was
created after all).
Instead of using 2 bonds (2 interfaces each) we could just use 4
interfaces directly, one per region. But two of those regions see very
little traffic, so we put more and less active regions in pairs.
> As for a resolution, presuming that Michal has some reasonable
> use case, I'm thinking along the lines of reverting the new (leave frame
> attached to slave) behavior for the general case and adding a special
> case for LLDP and friends to get the new behavior. I'd like to avoid
> adding any new options to bonding.
>
My use case aside, this will cause issues for anyone attaching bond
(instead of direct interface or veth) to a bridge and doing something
more complex with it - whether related to stp or to selectively passing
e.g. lldp using group_fwd_mask sysfs. Or having LLDP daemon (e.g.
systemd-resolvd to not look far away) told to do LLDP on bond device
(even most basic active-backup case) and remaining blind. Or anything
else that expects to see/pass those multicasts on/via bonded device
(which is just a convenient way to create virtual interface out of real
interfaces after all - ITOW shouldn't probably make any calls in this
regard).
^ permalink raw reply
* Re: [net-next PATCH] net: ipv4: fix listify ip_rcv_finish in case of forwarding
From: David Miller @ 2018-07-12 23:41 UTC (permalink / raw)
To: brouer; +Cc: ecree, netdev
In-Reply-To: <153132125549.13161.16380200872856218805.stgit@firesoul>
From: Jesper Dangaard Brouer <brouer@redhat.com>
Date: Wed, 11 Jul 2018 17:01:20 +0200
> In commit 5fa12739a53d ("net: ipv4: listify ip_rcv_finish") calling
> dst_input(skb) was split-out. The ip_sublist_rcv_finish() just calls
> dst_input(skb) in a loop.
>
> The problem is that ip_sublist_rcv_finish() forgot to remove the SKB
> from the list before invoking dst_input(). Further more we need to
> clear skb->next as other parts of the network stack use another kind
> of SKB lists for xmit_more (see dev_hard_start_xmit).
>
> A crash occurs if e.g. dst_input() invoke ip_forward(), which calls
> dst_output()/ip_output() that eventually calls __dev_queue_xmit() +
> sch_direct_xmit(), and a crash occurs in validate_xmit_skb_list().
>
> This patch only fixes the crash, but there is a huge potential for
> a performance boost if we can pass an SKB-list through to ip_forward.
>
> Fixes: 5fa12739a53d ("net: ipv4: listify ip_rcv_finish")
> Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> ---
> Only driver sfc actually uses this, but I don't have this NIC, so I
> tested this on mlx5, with my own changes to make it use netif_receive_skb_list(),
> but I'm not ready to upstream the mlx5 driver change yet.
Applied, thanks Jesper.
This whole:
list_del();
skb->next = NULL;
business is exactly the kind of dragons I was worried about when starting
to use list_head with SKBs.
There is a similar fix wrt. the GRO stuff that I'm about to apply as well.
It definitely is better if we don't have to forcefully hand off NULL
->next next pointers like this in the long term.
^ permalink raw reply
* Re: [PATCH net-next 00/10] s390/qeth: updates 2018-07-11
From: David Miller @ 2018-07-12 23:43 UTC (permalink / raw)
To: jwi; +Cc: netdev, linux-s390, schwidefsky, heiko.carstens, raspl, ubraun
In-Reply-To: <20180711154247.5523-1-jwi@linux.ibm.com>
From: Julian Wiedmann <jwi@linux.ibm.com>
Date: Wed, 11 Jul 2018 17:42:37 +0200
> please apply this first batch of qeth patches for net-next. It brings the
> usual cleanups, and some performance improvements to the transmit paths.
Series applied, thank you.
^ permalink raw reply
* [net-next, 2/3] tcp: convert icsk_user_timeout from jiffies to msecs
From: Jon Maxwell @ 2018-07-13 0:02 UTC (permalink / raw)
To: davem
Cc: edumazet, ncardwell, David.Laight, kuznet, yoshfuji, netdev,
linux-kernel, jmaxwell
Create a seperate helper routine called tcp_retransmit_stamp() as per Neal
Cardwells suggestion. To be used by the final commit in this series and
retransmits_timed_out().
Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
---
net/ipv4/tcp_timer.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index fa34984d0b12..8ab8c9645294 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -22,6 +22,20 @@
#include <linux/gfp.h>
#include <net/tcp.h>
+u32 tcp_retransmit_stamp(const struct sock *sk)
+{
+ u32 start_ts = tcp_sk(sk)->retrans_stamp;
+
+ if (unlikely(!start_ts)) {
+ struct sk_buff *head = tcp_rtx_queue_head(sk);
+
+ if (!head)
+ return 0;
+ start_ts = tcp_skb_timestamp(head);
+ }
+ return start_ts;
+}
+
/**
* tcp_write_err() - close socket and save error info
* @sk: The socket the error has appeared on.
@@ -166,14 +180,9 @@ static bool retransmits_timed_out(struct sock *sk,
if (!inet_csk(sk)->icsk_retransmits)
return false;
- start_ts = tcp_sk(sk)->retrans_stamp;
- if (unlikely(!start_ts)) {
- struct sk_buff *head = tcp_rtx_queue_head(sk);
-
- if (!head)
- return false;
- start_ts = tcp_skb_timestamp(head);
- }
+ start_ts = tcp_retransmit_stamp(sk);
+ if (!start_ts)
+ return false;
if (likely(timeout == 0)) {
linear_backoff_thresh = ilog2(TCP_RTO_MAX/rto_base);
--
2.13.6
^ permalink raw reply related
* Re: [PATCH net] nsh: set mac len based on inner packet
From: David Miller @ 2018-07-12 23:57 UTC (permalink / raw)
To: willemdebruijn.kernel; +Cc: netdev, willemb
In-Reply-To: <20180711160046.198091-1-willemdebruijn.kernel@gmail.com>
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Wed, 11 Jul 2018 12:00:44 -0400
> From: Willem de Bruijn <willemb@google.com>
>
> When pulling the NSH header in nsh_gso_segment, set the mac length
> based on the encapsulated packet type.
>
> skb_reset_mac_len computes an offset to the network header, which
> here still points to the outer packet:
>
> > skb_reset_network_header(skb);
> > [...]
> > __skb_pull(skb, nsh_len);
> > skb_reset_mac_header(skb); // now mac hdr starts nsh_len == 8B after net hdr
> > skb_reset_mac_len(skb); // mac len = net hdr - mac hdr == (u16) -8 == 65528
> > [..]
> > skb_mac_gso_segment(skb, ..)
>
> Link: http://lkml.kernel.org/r/CAF=yD-KeAcTSOn4AxirAxL8m7QAS8GBBe1w09eziYwvPbbUeYA@mail.gmail.com
> Reported-by: syzbot+7b9ed9872dab8c32305d@syzkaller.appspotmail.com
> Fixes: c411ed854584 ("nsh: add GSO support")
> Signed-off-by: Willem de Bruijn <willemb@google.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net] packet: reset network header if packet shorter than ll reserved space
From: David Miller @ 2018-07-12 23:57 UTC (permalink / raw)
To: willemdebruijn.kernel; +Cc: netdev, willemb
In-Reply-To: <20180711160046.198091-2-willemdebruijn.kernel@gmail.com>
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Wed, 11 Jul 2018 12:00:45 -0400
> From: Willem de Bruijn <willemb@google.com>
>
> If variable length link layer headers result in a packet shorter
> than dev->hard_header_len, reset the network header offset. Else
> skb->mac_len may exceed skb->len after skb_mac_reset_len.
>
> packet_sendmsg_spkt already has similar logic.
>
> Fixes: b84bbaf7a6c8 ("packet: in packet_snd start writing at link layer allocation")
> Signed-off-by: Willem de Bruijn <willemb@google.com>
Applied and queued up for -stable.
^ permalink raw reply
* Re: [PATCH net] selftests: in udpgso_bench do not test udp zerocopy
From: David Miller @ 2018-07-12 23:57 UTC (permalink / raw)
To: willemdebruijn.kernel; +Cc: netdev, willemb
In-Reply-To: <20180711160046.198091-3-willemdebruijn.kernel@gmail.com>
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Wed, 11 Jul 2018 12:00:46 -0400
> From: Willem de Bruijn <willemb@google.com>
>
> The udpgso benchmark compares various configurations of UDP and TCP.
> Including one that is not upstream, udp zerocopy. This is a leftover
> from the earlier RFC patchset.
>
> The test is part of kselftests and run in continuous spinners. Remove
> the failing case to make the test start passing.
>
> Fixes: 3a687bef148d ("selftests: udp gso benchmark")
> Reported-by: Naresh Kamboju <naresh.kamboju@linaro.org>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: gro: properly remove skb from list
From: David Miller @ 2018-07-13 0:02 UTC (permalink / raw)
To: bhole_prashant_q7; +Cc: brouer, tyhicks, netdev
In-Reply-To: <20180712072459.8800-1-bhole_prashant_q7@lab.ntt.co.jp>
From: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
Date: Thu, 12 Jul 2018 16:24:59 +0900
> Following crash occurs in validate_xmit_skb_list() when same skb is
> iterated multiple times in the loop and consume_skb() is called.
>
> The root cause is calling list_del_init(&skb->list) and not clearing
> skb->next in d4546c2509b1. list_del_init(&skb->list) sets skb->next
> to point to skb itself. skb->next needs to be cleared because other
> parts of network stack uses another kind of SKB lists.
> validate_xmit_skb_list() uses such list.
>
> A similar type of bugfix was reported by Jesper Dangaard Brouer.
> https://patchwork.ozlabs.org/patch/942541/
>
> This patch clears skb->next and changes list_del_init() to list_del()
> so that list->prev will maintain the list poison.
...
> Fixes: d4546c2509b1 ("net: Convert GRO SKB handling to list_head.")
> Signed-off-by: Prashant Bhole <bhole_prashant_q7@lab.ntt.co.jp>
> Reported-by: Tyler Hicks <tyhicks@canonical.com>
Applied, thank you.
Hopefully we can convert more layers to list_head SKB usage, and
thus no longer need hacks like this.
Thanks.
^ permalink raw reply
* Re: [PATCH] liquidio: Use %pad printk format for dma_addr_t values
From: Felix Manlunas @ 2018-07-13 0:06 UTC (permalink / raw)
To: Helge Deller
Cc: Derek Chickles, Satanand Burla, Felix Manlunas, Raghu Vatsavayi,
netdev, linux-parisc
In-Reply-To: <20180712203629.GA32537@ls3530.fritz.box>
On Thu, Jul 12, 2018 at 10:36:29PM +0200, Helge Deller wrote:
> Use the existing %pad printk format to print dma_addr_t values.
> This avoids the following warnings when compiling on the parisc platform:
>
> warning: format '%llx' expects argument of type 'long long unsigned int', but argument 2 has type 'dma_addr_t {aka unsigned int}' [-Wformat=]
>
> Signed-off-by: Helge Deller <deller@gmx.de>
>
> diff --git a/drivers/net/ethernet/cavium/liquidio/request_manager.c b/drivers/net/ethernet/cavium/liquidio/request_manager.c
> index 1f2e75da28f8..d5d9e47daa4b 100644
> --- a/drivers/net/ethernet/cavium/liquidio/request_manager.c
> +++ b/drivers/net/ethernet/cavium/liquidio/request_manager.c
> @@ -110,8 +110,8 @@ int octeon_init_instr_queue(struct octeon_device *oct,
>
> memset(iq->request_list, 0, sizeof(*iq->request_list) * num_descs);
>
> - dev_dbg(&oct->pci_dev->dev, "IQ[%d]: base: %p basedma: %llx count: %d\n",
> - iq_no, iq->base_addr, iq->base_addr_dma, iq->max_count);
> + dev_dbg(&oct->pci_dev->dev, "IQ[%d]: base: %p basedma: %pad count: %d\n",
> + iq_no, iq->base_addr, &iq->base_addr_dma, iq->max_count);
>
> iq->txpciq.u64 = txpciq.u64;
> iq->fill_threshold = (u32)conf->db_min;
Acked-by: Felix Manlunas <felix.manlunas@cavium.com>
^ permalink raw reply
* Proposal
From: Miss Victoria Mehmet @ 2018-07-12 23:32 UTC (permalink / raw)
To: Recipients
Hello
I have a business proposal of mutual benefits i would like to discuss with
you i asked before and i still await your positive response thanks
^ permalink raw reply
* Re: [BUG] bonded interfaces drop bpdu (stp) frames
From: Mahesh Bandewar (महेश बंडेवार) @ 2018-07-13 0:15 UTC (permalink / raw)
To: Michal Soltys; +Cc: Jay Vosburgh, Chonggang Li, linux-netdev
In-Reply-To: <836819fc-1a70-1b30-d90c-8c00953b8b6c@ziu.info>
On Thu, Jul 12, 2018 at 4:14 PM, Michal Soltys <soltys@ziu.info> wrote:
> On 2018-07-13 00:03, Jay Vosburgh wrote:
>> Mahesh Bandewar (महेश बंडेवार) wrote:
>>
>>>On Thu, Jul 12, 2018 at 11:03 AM, Jay Vosburgh
>>><jay.vosburgh@canonical.com> wrote:
>>>> Michal Soltys <soltys@ziu.info> wrote:
>>>>
>>>>>On 07/12/2018 04:51 PM, Jay Vosburgh wrote:
>>>>>> Mahesh Bandewar (महेश बंडेवार) wrote:
>>>>>>
>>>>>>> On Wed, Jul 11, 2018 at 3:23 PM, Michal Soltys <soltys@ziu.info> wrote:
>>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> As weird as that sounds, this is what I observed today after bumping
>>>>>>>> kernel version. I have a setup where 2 bonds are attached to linux
>>>>>>>> bridge and physically are connected to two switches doing MSTP (and
>>>>>>>> linux bridge is just passing them).
>>>>>>>>
>>>>>>>> Initially I suspected some changes related to bridge code - but quick
>>>>>>>> peek at the code showed nothing suspicious - and the part of it that
>>>>>>>> explicitly passes stp frames if stp is not enabled has seen little
>>>>>>>> changes (e.g. per-port group_fwd_mask added recently). Furthermore - if
>>>>>>>> regular non-bonded interfaces are attached everything works fine.
>>>>>>>>
>>>>>>>> Just to be sure I detached the bond (802.3ad mode) and checked it with
>>>>>>>> simple tcpdump (ether proto \\stp) - and indeed no hello packets were
>>>>>>>> there (with them being present just fine on active enslaved interface,
>>>>>>>> or on the bond device in earlier kernels).
>>>>>>>>
>>>>>>>> If time permits I'll bisect tommorow to pinpoint the commit, but from
>>>>>>>> quick todays test - 4.9.x is working fine, while 4.16.16 (tested on
>>>>>>>> debian) and 4.17.3 (tested on archlinux) are failing.
>>>>>>>>
>>>>>>>> Unless this is already a known issue (or you have any suggestions what
>>>>>>>> could be responsible).
>>>>>>>>
>>>>>>> I believe these are link-local-multicast messages and sometime back a
>>>>>>> change went into to not pass those frames to the bonding master. This
>>>>>>> could be the side effect of that.
>>>>>>
>>>>>> Mahesh, I suspect you're thinking of:
>>>>>>
>>>>>> commit b89f04c61efe3b7756434d693b9203cc0cce002e
>>>>>> Author: Chonggang Li <chonggangli@google.com>
>>>>>> Date: Sun Apr 16 12:02:18 2017 -0700
>>>>>>
>>>>>> bonding: deliver link-local packets with skb->dev set to link that packets arrived on
>>>>>>
>>>>>> Michal, are you able to revert this patch and test?
>>>>>>
>>>>>> -J
>>>>>>
>>>>>> ---
>>>>>> -Jay Vosburgh, jay.vosburgh@canonical.com
>>>>>>
>>>>>
>>>>>
>>>>>Just tested - yes, reverting that patch solves the issues.
>>>>
>>>> Chonggang,
>>>>
>>>> Reading the changelog in your commit referenced above, I'm not
>>>> entirely sure what actual problem it is fixing. Could you elaborate?
>>>>
>>>> As the patch appears to cause a regression, it needs to be
>>>> either fixed or reverted.
>>>>
>>>> Mahesh, you signed-off on it as well, perhaps you also have some
>>>> context?
>>>>
>>>
>>>I think the original idea behind it was to pass the LLDPDUs to the
>>>stack on the interface that they came on since this is considered to
>>>be link-local traffic and passing to bond-master would loose it's
>>>"linklocal-ness". This is true for LLDP and if you change the skb->dev
>>>of the packet, then you don't know which slave link it came on in
>>>(from LLDP consumer's perspective).
>>>
>>>I don't know much about STP but trunking two links and aggregating
>>>this link info through bond-master seems wrong. Just like LLDP, you
>>>are losing info specific to a link and the decision derived from that
>>>info could be wrong.
>>>
>>>Having said that, we determine "linklocal-ness" by looking at L2 and
>>>bondmaster shares this with lts slaves. So it does seem fair to pass
>>>those frames to the bonding-master but at the same time link-local
>>>traffic is supposed to be limited to the physical link (LLDP/STP/LACP
>>>etc). Your thoughts?
>>
>> I agree the whole thing sounds kind of weird, but I'm curious as
>> to what Michal's actual use case is; he presumably has some practical
>> use for this, since he noticed that the behavior changed.
>>
>
> The whole "link-local" term is a bit I don't know - at this point it
> feels like too many things were thrown into single bag and it got
> somewhat confusing (bpdu, lldp, pause frames, lacp, pae, qinq mulitcast
> that afaik has its own address) - I added some examples in another reply
> I did at the same time as you were typing this one =)
>
>> Michal, you mentioned MSTP and using 802.3ad (LACP) mode; how
>> does that combination work rationally given that the bond might send and
>> receive traffic across multiple slaves? Or does the switch side bundle
>> the ports together into a single logical interface for MSTP purposes?
>> On the TX side, I think the bond will likely balance all STP frames to
>> just one slave.
>>
>
> The basic concept - two "main" switches with "important" machines
> connected to those. One switch dies and everything keeps working. With
> no unused ports and so on.
>
> In more details:
>
> Originally I was trying MSTP daemon (on "important" machines) which
> seems quite well and completely coded, but cannot really work correctly
> - as afaik you can't put port (in linux bridge conext) in different
> forwarding/blocking/etc. state per-region - itow per group of vlans (or
> mstpd didn't know how to do that, or it wasn't implemented - I didn't
> look too deep back then, though my interest resurfaced in recent days).
>
> So that option was out of the question. But any switch, real or not,
> /must/ pass bpdu frames if it doesn't interpret them.
I think it's contrary! These (STP) frames are link-local-multicast
frames i.e. their dest-mac is multicast-mac (unlike the one that you
would see on an interface). So a *real* switch would never pass those
frames (because there wont be an entry in the CAM table) and would
have to consume those frames. Just imagine a userspace LACP-daemon
receiving LACPDUs on bonding master, it wouldn't make right decisions
about slaves and hence the health of the aggregator would be
questionable. Also imagine LACP frames being forwards by *real*
switch, things wont work as intended.
> So instead of
> having active mstp participant, we have passive linux bridge that passes
> the frames and the two real switches around that care of mstp, treating
> the linux as a shared segment. The costs/priorities/etc. on the real
> swtiches are set so one bond handles two regions, and the other bond
> handles other two regions. If any of the real switches dies or is taken
> down for e.g. firmware update - the bond going to the other switch
> handles all four regions (failover is of course not as fast as with
> active rstp/mstp participation, but works quite well none the less -
> around 10s after some tuning).
>
> We could have used RSTP for that purpose as well - but that being all or
> nothing in context of per-port blocking/forwarding, would leave half of
> the ports unused - and we wanted to avoid that (that's why MSTP was
> created after all).
>
> Instead of using 2 bonds (2 interfaces each) we could just use 4
> interfaces directly, one per region. But two of those regions see very
> little traffic, so we put more and less active regions in pairs.
>
>> As for a resolution, presuming that Michal has some reasonable
>> use case, I'm thinking along the lines of reverting the new (leave frame
>> attached to slave) behavior for the general case and adding a special
>> case for LLDP and friends to get the new behavior. I'd like to avoid
>> adding any new options to bonding.
>>
>
> My use case aside, this will cause issues for anyone attaching bond
> (instead of direct interface or veth) to a bridge and doing something
> more complex with it - whether related to stp or to selectively passing
> e.g. lldp using group_fwd_mask sysfs. Or having LLDP daemon (e.g.
> systemd-resolvd to not look far away) told to do LLDP on bond device
> (even most basic active-backup case) and remaining blind. Or anything
> else that expects to see/pass those multicasts on/via bonded device
> (which is just a convenient way to create virtual interface out of real
> interfaces after all - ITOW shouldn't probably make any calls in this
> regard).
A real switch cannot forward link-local-multicast and if it does, then
I would consider it as broken. Having said that we are taking about
the bonding-master and as I mentioned earlier, it seems fair to pass
those frames to bonding master because we share L2 with slaves but we
should pass the frames to the stack as they are (current behavior!) to
maintain the "linklocal-ness" so both master and the current-slave get
a copy (it's multicast after all).
Thanks,
--mahesh..
^ permalink raw reply
* Re: [PATCH net-next 00/18] net: mvpp2: add RSS support
From: David Miller @ 2018-07-13 0:31 UTC (permalink / raw)
To: maxime.chevallier
Cc: netdev, linux-kernel, antoine.tenart, thomas.petazzoni,
gregory.clement, miquel.raynal, nadavh, stefanc, ymarkman, mw
In-Reply-To: <20180712115427.27375-1-maxime.chevallier@bootlin.com>
From: Maxime Chevallier <maxime.chevallier@bootlin.com>
Date: Thu, 12 Jul 2018 13:54:09 +0200
> This series adds support for RSS on PPv2. There already was some code to
> handle the RSS tables, but the driver was missing all the classification
> steps required to actually use these tables.
>
> RSS is used through the classifier, using at least 2 lookups :
> - One using the C2 engine, a TCAM engine that match the packet based on
> some header extracted fields, assigns the default rx queue for that
> packet and tag it for RSS
> - One using the C3Hx engine, which computes the hash that's used to perform
> the lookup in the RSS table.
>
> Since RSS spreads the load across CPUs, we need to make sure that packets
> from the same flow are always assigned the same rx queue, to prevent
> re-ordering.
>
> This series therefore adds a classification step based on the Header Parser,
> that separate ingress traffic into 52 flows, based on some L2, L3 and L4
> parameters.
>
> Patches 1 and 2 fix some header issues, from the driver splitting
>
> Patches 3 to 7 make sure the correct receive queue setup is used for RSS
>
> Patches 8 to 14 deal with the way we handle the RSS tables
>
> Patch 15 implement basic classifier configuration, by using it to assign the
> default receive queue
>
> Patch 16 implement the ingress traffic splitting into multiple flows
>
> Patch 17 adds RSS support, by using the needed classification steps
>
> Patch 18 adds the required ethtool ops to configure the flow hash parameters
>
> This was tested on MacchiatoBin, giving some nice performance improvements
> using ip forwarding (going from 5Gbps to 9.6Gbps total throughput).
>
> RSS is disabled by default.
Series applied, thank you.
^ permalink raw reply
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2018-07-13 0:47 UTC (permalink / raw)
To: David Miller, Networking
Cc: Linux-Next Mailing List, Linux Kernel Mailing List,
Stefan Baranoff, Arnd Bergmann, Eric Dumazet
[-- Attachment #1: Type: text/plain, Size: 2299 bytes --]
Hi all,
Today's linux-next merge of the net-next tree got a conflict in:
net/ipv4/tcp_ipv4.c
between commit:
21684dc46c59 ("tcp: fix sequence numbers for repaired sockets re-using TIME-WAIT sockets")
from the net tree and commit:
cca9bab1b72c ("tcp: use monotonic timestamps for PAWS")
from the net-next tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc net/ipv4/tcp_ipv4.c
index 3b2711e33e4c,dc415c66a33a..000000000000
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@@ -155,25 -155,13 +155,26 @@@ int tcp_twsk_unique(struct sock *sk, st
and use initial timestamp retrieved from peer table.
*/
if (tcptw->tw_ts_recent_stamp &&
- (!twp || (reuse && get_seconds() - tcptw->tw_ts_recent_stamp > 1))) {
+ (!twp || (reuse && time_after32(ktime_get_seconds(),
+ tcptw->tw_ts_recent_stamp)))) {
- tp->write_seq = tcptw->tw_snd_nxt + 65535 + 2;
- if (tp->write_seq == 0)
- tp->write_seq = 1;
- tp->rx_opt.ts_recent = tcptw->tw_ts_recent;
- tp->rx_opt.ts_recent_stamp = tcptw->tw_ts_recent_stamp;
+ /* In case of repair and re-using TIME-WAIT sockets we still
+ * want to be sure that it is safe as above but honor the
+ * sequence numbers and time stamps set as part of the repair
+ * process.
+ *
+ * Without this check re-using a TIME-WAIT socket with TCP
+ * repair would accumulate a -1 on the repair assigned
+ * sequence number. The first time it is reused the sequence
+ * is -1, the second time -2, etc. This fixes that issue
+ * without appearing to create any others.
+ */
+ if (likely(!tp->repair)) {
+ tp->write_seq = tcptw->tw_snd_nxt + 65535 + 2;
+ if (tp->write_seq == 0)
+ tp->write_seq = 1;
+ tp->rx_opt.ts_recent = tcptw->tw_ts_recent;
+ tp->rx_opt.ts_recent_stamp = tcptw->tw_ts_recent_stamp;
+ }
sock_hold(sktw);
return 1;
}
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* [net-next, 3/3] tcp: convert icsk_user_timeout from jiffies to msecs
From: Jon Maxwell @ 2018-07-13 0:49 UTC (permalink / raw)
To: davem
Cc: edumazet, ncardwell, David.Laight, kuznet, yoshfuji, netdev,
linux-kernel, jmaxwell
The final patch in this series. Create the tcp_clamp_rto_to_user_timeout()
helper routine. To calculate the correct rto, so that the TCP_USER_TIMEOUT
socket option is more accurate. Taking suggestions and feedback into account
from Eric Dumazet, Neal Cardwell and David Laight. Due to the 1st commit we
can avoid the msecs_to_jiffies() and jiffies_to_msecs() dance.
Signed-off-by: Jon Maxwell <jmaxwell37@gmail.com>
---
net/ipv4/tcp_timer.c | 19 ++++++++++++++++++-
1 file changed, 18 insertions(+), 1 deletion(-)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index 8ab8c9645294..d4d4b30a6bec 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -36,6 +36,22 @@ u32 tcp_retransmit_stamp(const struct sock *sk)
return start_ts;
}
+static u32 tcp_clamp_rto_to_user_timeout(const struct sock *sk)
+{
+ struct inet_connection_sock *icsk = inet_csk(sk);
+ u32 elapsed, start_ts;
+
+ start_ts = tcp_retransmit_stamp(sk);
+ if (!icsk->icsk_user_timeout || !start_ts)
+ return icsk->icsk_rto;
+ elapsed = tcp_time_stamp(tcp_sk(sk)) - start_ts;
+ if (elapsed >= icsk->icsk_user_timeout)
+ return 1; /* user timeout has passed; fire ASAP */
+ else
+ return min_t(u32, icsk->icsk_rto, msecs_to_jiffies(icsk->icsk_user_timeout -
+ elapsed));
+}
+
/**
* tcp_write_err() - close socket and save error info
* @sk: The socket the error has appeared on.
@@ -544,7 +560,8 @@ void tcp_retransmit_timer(struct sock *sk)
/* Use normal (exponential) backoff */
icsk->icsk_rto = min(icsk->icsk_rto << 1, TCP_RTO_MAX);
}
- inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS, icsk->icsk_rto, TCP_RTO_MAX);
+ inet_csk_reset_xmit_timer(sk, ICSK_TIME_RETRANS,
+ tcp_clamp_rto_to_user_timeout(sk), TCP_RTO_MAX);
if (retransmits_timed_out(sk, net->ipv4.sysctl_tcp_retries1 + 1, 0))
__sk_dst_reset(sk);
--
2.13.6
^ permalink raw reply related
* Re: [PATCH net-next v3 00/11] devlink: Add support for region access
From: David Miller @ 2018-07-13 0:37 UTC (permalink / raw)
To: valex; +Cc: netdev, jiri, dsahern, andrew, rahul.lakkireddy, jakub.kicinski
In-Reply-To: <1531397598-11207-1-git-send-email-valex@mellanox.com>
From: Alex Vesker <valex@mellanox.com>
Date: Thu, 12 Jul 2018 15:13:07 +0300
> This is a proposal which will allow access to driver defined address
> regions using devlink. Each device can create its supported address
> regions and register them. A device which exposes a region will allow
> access to it using devlink.
>
> The suggested implementation will allow exposing regions to the user,
> reading and dumping snapshots taken from different regions.
> A snapshot represents a memory image of a region taken by the driver.
>
> If a device collects a snapshot of an address region it can be later
> exposed using devlink region read or dump commands.
> This functionality allows for future analyses on the snapshots to be
> done.
>
> The major benefit of this support is not only to provide access to
> internal address regions which were inaccessible to the user but also
> to provide an additional way to debug complex error states using the
> region snapshots.
...
Series applied, thanks!
^ permalink raw reply
* Re: [PATCH net-next 1/5 v3] net: gemini: Look up L3 maxlen from table
From: David Miller @ 2018-07-13 0:39 UTC (permalink / raw)
To: linus.walleij; +Cc: netdev, ulli.kroll, f.fainelli, mirq-linux, andrew
In-Reply-To: <20180711193245.21980-1-linus.walleij@linaro.org>
From: Linus Walleij <linus.walleij@linaro.org>
Date: Wed, 11 Jul 2018 21:32:41 +0200
> The code to calculate the hardware register enumerator
> for the maximum L3 length isn't entirely simple to read.
> Use the existing defines and rewrite the function into a
> table look-up.
>
> Acked-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Applied.
^ permalink raw reply
* Re: [PATCH net-next 2/5 v3] net: gemini: Improve connection prints
From: David Miller @ 2018-07-13 0:39 UTC (permalink / raw)
To: linus.walleij; +Cc: netdev, ulli.kroll, f.fainelli, mirq-linux, andrew
In-Reply-To: <20180711193245.21980-2-linus.walleij@linaro.org>
From: Linus Walleij <linus.walleij@linaro.org>
Date: Wed, 11 Jul 2018 21:32:42 +0200
> Switch over to using a module parameter and debug prints
> that can be controlled by this or ethtool like everyone
> else. Depromote all other prints to debug messages.
>
> The phy_print_status() was already in place, albeit never
> really used because the debuglevel hiding it had to be
> set up using ethtool.
>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
Applied.
^ 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