Netdev List
 help / color / mirror / Atom feed
* Re: TC-pedit man page examples error
From: Tyler Bautista @ 2017-07-26 16:47 UTC (permalink / raw)
  To: Phil Sutter, Tyler Bautista, netdev
In-Reply-To: <20170726084338.GH2235@orbyte.nwl.cc>

Hello Phil,

Thank you for taking my message. I have gotten a solution working so
feel free to ignore the rest of this email. By vanilla iproute2 do you
mean the latest package through the yum or apt repository? All I can
tell you is that in the example I gave the iproute2 package was the
latest one through the RHEL 7 yum repository. Otherwise I will need
clarification on what vanilla means. I attempted the above example
code on two other Ubuntu machines, one of which I can confirm is
Ubuntu 16.04 LTS. Unfortunately I am not at that machine right now so
I cannot give you the output. Both machines gave similar error
messages. The reason I emailed you via netdev was because the example
code I used was taken verbatim from the tc-pedit man page on my
machine. The man page also gave me email addresses to contact in case
an error was found with the man page, which is what I did. I did get
the code to work when using the latest version of iproute2, so I
assume the only practical solution is to manually install the latest
version of iproute2 or wait for the various distros of Linux to use
updated iproute2. I appreciate your help. Have a nice day.

Tyler Bautista

On Wed, Jul 26, 2017 at 1:43 AM, Phil Sutter <phil@nwl.cc> wrote:
> Hi Tyler,
>
> On Tue, Jul 25, 2017 at 08:33:40AM -0700, Tyler Bautista wrote:
>> To whom it may concern,
>> I recently attempted to use simple tc action pedit commands on the man
>> page and I ran into some errors. The following is some information
>> about my version of iproute and my machine:
>> ----------------------------
>> the following is my iproute package information
>>
>> Loaded plugins: fastestmirror, langpacks
>> iproute-3.10.0-74.el7.x86_64
>
> This mailing list is for upstream issues only. Can you reproduce the
> problem with vanilla iproute2?
>
> You are using RHEL7's iproute package, which has a number of known
> issues in pedit action:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=1322406
>
> These are planned to be resolved for RHEL7.5, not sure when it will land
> in CentOS.
>
> Cheers, Phil

^ permalink raw reply

* Re: [PATCH net-next] TLP: Don't reschedule PTO when there's one outstanding TLP retransmission.
From: Yuchung Cheng @ 2017-07-26 16:45 UTC (permalink / raw)
  To: maowenan
  Cc: Neal Cardwell, Netdev, David Miller, weiyongjun (A), Chenweilong,
	Nandita Dukkipati, Wangkefeng (Kevin)
In-Reply-To: <F95AC9340317A84688A5F0DF0246F3F202929B17@DGGEMI512-MBX.china.huawei.com>

On Tue, Jul 25, 2017 at 7:12 PM, maowenan <maowenan@huawei.com> wrote:
>
>
>
> > -----Original Message-----
> > From: Neal Cardwell [mailto:ncardwell@google.com]
> > Sent: Tuesday, July 25, 2017 9:30 PM
> > To: maowenan
> > Cc: Netdev; David Miller; weiyongjun (A); Chenweilong; Yuchung Cheng;
> > Nandita Dukkipati
> > Subject: Re: [PATCH net-next] TLP: Don't reschedule PTO when there's one
> > outstanding TLP retransmission.
> >
> > On Tue, Jul 25, 2017 at 4:35 AM, Mao Wenan <maowenan@huawei.com>
> > wrote:
> > > If there is one TLP probe went out(TLP use the write_queue_tail packet
> > > as TLP probe, we assume this first TLP probe named A), and this TLP
> > > probe was not acked by receive side.
> > >
> > > Then the transmit side sent the next two packetes out(named B,C), but
> > > unfortunately these two packets are also not acked by receive side.
> > >
> > > And then there is one data packet with ack_seq A arrive, in tcp_ack()
> > > will call tcp_schedule_loss_probe() to rearm PTO, the handler
> > > tcp_send_loss_probe() pass if(tp->tlp_high_seq)(because there is one
> > > outstanding TLP named A,tp->tlp_high_seq is not zero), so the new TLP
> > > probe can't be went out and need to rearm the RTO timer(timeout is
> > > relative to the transmit time of the write queue head).
> > >
> > > After this, another data packet with ack_seq A is received, if the
> > > tlp_time_stamp is after rto_time_stamp, it will reset the TLP timeout
> > > with delta value, which is before previous RTO timeout, so PTO is
> > > rearm and previous RTO is cleared. This TLP probe also can't be sent
> > > out because of tp->tlp_high_seq != 0, so there is no way(or need very
> > > long time)to retransmit the packet because of TLP A is lost.
> > >
> > > This fix is not to pass the if(tp->tlp_high_seq) in
> > > tcp_schedule_loss_probe() when TLP PTO is after RTO, It is no need to
> > > reschedule PTO when there is one outstanding TLP retransmission, so if
> > > the TLP A is lost then RTO can retransmit that packet, and
> > > tp->tlp_high_seq will be set to 0. After this TLP will go to the normal work
> > process.
> > >
> > > Signed-off-by: Mao Wenan <maowenan@huawei.com>
> >
> > Thanks for posting this. This is a pretty involved scenario. To help
> > document/test precisely what the behavior is before and after your patch,
> > would you be able to post a packetdrill ( https://github.com/google/packetdrill )
> > test case for this scenario?
> >
> > Can I ask if you saw this scenario in an actual trace, or noticed this by
> > inspection?
> [Mao Wenan] It is my actual product scenario, from some debug trace info
> I found that PTO is always rescheduled before RTO, this is PTO is trigged
> by receiving one tcp_ack packets. After this patch, the product works well and
> there is no previous issue happen. The packet can retransmit with RTO if TLP probe
> is lost.
> I will say sorry that my local test can't reproduce because i can't build successfully the same
> complex scenario, I don't have mature test case to reproduce now but I will do my best to try
> in local test environment.
tcpdump output of an example trace also helps. Feel free to remove IP
address, ports, or payload information. We only need the timing and
TCP header info.


> Thank you.
> >
> > thanks,
> > neal

^ permalink raw reply

* Re: [PATCH RFC 00/13] phylink and sfp support
From: Andrew Lunn @ 2017-07-26 16:44 UTC (permalink / raw)
  To: Russell King - ARM Linux; +Cc: Florian Fainelli, netdev
In-Reply-To: <20170725140138.GK31807@n2100.armlinux.org.uk>

On Tue, Jul 25, 2017 at 03:01:39PM +0100, Russell King - ARM Linux wrote:
> Hi,
> 
> This patch series introduces generic support for SFP sockets found on
> various Marvell based platforms.  The idea here is to provide common
> SFP socket support which can be re-used by network drivers as
> appropriate, rather than each network driver having to re-implement
> SFP socket support.

Hi Russell

Thanks for posting these patches. I will review them once i catch up
with my backlog.

...

> I currently (but not included in this series) have code to convert
> mvneta to use phylink

Do you have a git branch we can look at to see these changes. It helps
understand the API when you can see both sides of it.

Thanks
	Andrew

^ permalink raw reply

* Re: TCP fast retransmit issues
From: Neal Cardwell @ 2017-07-26 16:43 UTC (permalink / raw)
  To: Willy Tarreau
  Cc: Eric Dumazet, Klavs Klavsen, Netdev, Yuchung Cheng,
	Nandita Dukkipati
In-Reply-To: <20170726145059.GE1737@1wt.eu>

[-- Attachment #1: Type: text/plain, Size: 1412 bytes --]

On Wed, Jul 26, 2017 at 04:08:19PM +0200, Klavs Klavsen wrote:
> Grabbed on both ends.
>
> http://blog.klavsen.info/fast-retransmit-problem-junos-linux (updated to new
> dump - from client scp'ing)
> http://blog.klavsen.info/fast-retransmit-problem-junos-linux-receiving-side
> (receiving host)

Looking at some time-sequence plots of the sender trace (attached),
and thinking about the Linux TCP sender code, it like there are at
least two interesting things going on:

(1) Because the connection negotiated SACK, the Linux TCP sender does
not get to its tcp_add_reno_sack() code to count dupacks and enter
fast recovery on the 3rd dupack. The sender keeps waiting for specific
packets to be SACKed that would signal that something has probably
been lost. We could probably mitigate this by having the sender turn
off SACK once it sees SACKed ranges that are completely invalid (way
out of window). Then it should use the old non-SACK "Recovery on 3rd
dupack" path.

(2) It looks like there is a bug in the sender code where it seems to
be repeatedly using a TLP timer firing 211ms after every ACK is
received to transmit another TLP probe (a new packet in this case).
Somehow these weird invalid SACKs seem to be triggering a code path
that makes us think we can send another TLP, when we probably should
be firing an RTO. That's my interpretation, anyway. I will try to
reproduce this with packetdrill.

neal

[-- Attachment #2: linux-tcp-fr-issues-2017-07-26-zoomed-out.png --]
[-- Type: image/png, Size: 37196 bytes --]

[-- Attachment #3: linux-tcp-fr-issues-2017-07-26-zoomed-in-1.png --]
[-- Type: image/png, Size: 39867 bytes --]

[-- Attachment #4: linux-tcp-fr-issues-2017-07-26-zoomed-in-2.png --]
[-- Type: image/png, Size: 37358 bytes --]

^ permalink raw reply

* Re: [PATCH net] udp: unbreak build lacking CONFIG_XFRM
From: David Miller @ 2017-07-26 16:43 UTC (permalink / raw)
  To: pabeni; +Cc: netdev, paul, kbuild-all
In-Reply-To: <b8448ef348e62558d3ab92468e1e5ed076d66afe.1501061474.git.pabeni@redhat.com>

From: Paolo Abeni <pabeni@redhat.com>
Date: Wed, 26 Jul 2017 11:33:49 +0200

> We must use pre-processor conditional block or suitable accessors to
> manipulate skb->sp elsewhere builds lacking the CONFIG_XFRM will break.
> 
> Fixes: dce4551cb2ad ("udp: preserve head state for IP_CMSG_PASSSEC")
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Applied, thank you.

^ permalink raw reply

* Re: [PATCH] hamradio: dmascc: avoid -Wformat-overflow warning
From: David Miller @ 2017-07-26 16:41 UTC (permalink / raw)
  To: arnd; +Cc: jreuter, dhowells, linux-hams, netdev, linux-kernel
In-Reply-To: <20170726075548.2736340-1-arnd@arndb.de>

From: Arnd Bergmann <arnd@arndb.de>
Date: Wed, 26 Jul 2017 09:55:33 +0200

> gcc warns that the device name might overflow:
> 
> drivers/net/hamradio/dmascc.c: In function 'dmascc_init':
> drivers/net/hamradio/dmascc.c:584:22: error: 'sprintf' may write a terminating nul past the end of the destination [-Werror=format-overflow=]
>    sprintf(dev->name, "dmascc%i", 2 * n + i);
> drivers/net/hamradio/dmascc.c:584:3: note: 'sprintf' output between 8 and 17 bytes into a destination of size 16
>    sprintf(dev->name, "dmascc%i", 2 * n + i);
> 
> From the static data in this file, I can tell that the index is
> strictly limited to 16, so it won't overflow. This simply changes
> the sprintf() to snprintf(), which is a good idea in general,
> and shuts up this warning.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied to net-next.

^ permalink raw reply

* Re: [PATCH v2 2/4] can: fixed-transceiver: Add documentation for CAN fixed transceiver bindings
From: Andrew Lunn @ 2017-07-26 16:41 UTC (permalink / raw)
  To: Franklin S Cooper Jr
  Cc: linux-kernel, devicetree, netdev, linux-can, wg, mkl, robh+dt,
	quentin.schulz, dev.kurt, sergei.shtylyov, socketcan
In-Reply-To: <20170724230521.1436-3-fcooper@ti.com>

On Mon, Jul 24, 2017 at 06:05:19PM -0500, Franklin S Cooper Jr wrote:
> Add documentation to describe usage of the new fixed transceiver binding.
> This new binding is applicable for any CAN device therefore it exists as
> its own document.
> 
> Signed-off-by: Franklin S Cooper Jr <fcooper@ti.com>
> ---
> Version 2:
> Tweak commit message working
> Tweak wording for properties
> Drop unit addressing
> Give example if CAN-FD isn't supported
> 
>  .../bindings/net/can/fixed-transceiver.txt         | 42 ++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt b/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
> new file mode 100644
> index 0000000..dc7e3eb
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/can/fixed-transceiver.txt
> @@ -0,0 +1,42 @@
> +Fixed transceiver Device Tree binding
> +------------------------------
> +
> +CAN transceiver typically limits the max speed in standard CAN and CAN FD
> +modes. Typically these limitations are static and the transceivers themselves
> +provide no way to detect this limitation at runtime. For this situation,
> +the "fixed-transceiver" node can be used.
> +
> +Properties:
> +
> +Optional:
> + max-arbitration-speed: a positive non 0 value that determines the max
> +			speed that CAN can run in non CAN-FD mode or during the
> +			arbitration phase in CAN-FD mode.

Hi Franklin

Since this and the next property are optional, it is good to document
what happens when they are not in the DT blob. Also document what 0
means.

> +
> + max-data-speed:	a positive non 0 value that determines the max data rate
> +			that can be used in CAN-FD mode. A value of -1 implies
> +			CAN-FD is not supported by the transceiver.

-1 is ugly. I think it would be better to have a missing
max-data-speed property indicate that CAN-FD is not supported. And
maybe put 'fd' into the property name.

      Andrew

^ permalink raw reply

* Re: [PATCH net-next 10/18] net: mvpp2: use the GoP interrupt for link status changes
From: Andrew Lunn @ 2017-07-26 16:26 UTC (permalink / raw)
  To: Antoine Tenart
  Cc: davem, jason, gregory.clement, sebastian.hesselbarth,
	thomas.petazzoni, nadavh, linux, mw, stefanc, netdev,
	linux-arm-kernel
In-Reply-To: <20170724134848.19330-11-antoine.tenart@free-electrons.com>

On Mon, Jul 24, 2017 at 03:48:40PM +0200, Antoine Tenart wrote:
> This patch adds the GoP link interrupt support for when a port isn't
> connected to a PHY. Because of this the phylib callback is never called
> and the link status management isn't done. This patch use the GoP link
> interrupt in such cases to still have a minimal link management. Without
> this patch ports not connected to a PHY cannot work.

Hi Antoine

When is a GoP link interrupt signalled? When is a port without a PHY
actually up/down?

With SFF/SFP ports, you generally need a gpio line the fibre module
can use to indicate if it has link. Fixed-phy has such support, and
your link_change function will get called when the link changes.

And this is another bit of code you probably need to change in a while
with phylink lands.

     Andrew

^ permalink raw reply

* Re: [PATCH iproute2] bridge: Assume master at FDB modification
From: Arkadi Sharshevsky @ 2017-07-26 16:21 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, davem, nikolay, vivien.didelot, mlxsw
In-Reply-To: <20170726090820.0252da7e@xeon-e3>



On 07/26/2017 07:08 PM, Stephen Hemminger wrote:
> On Wed, 26 Jul 2017 18:36:34 +0300
> Arkadi Sharshevsky <arkadis@mellanox.com> wrote:
> 
>> According to the man page the master flag should be the default, yet, the
>> current code assumes otherwise.
>>
>> Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
> 
> Agree that the documentation and code don't match.
> But your change could break users with existing scripts by changing behavior.
> 
> It would be safer to change the man page not the code.
> 

Can we maybe set master and self by default. It doesn't make
sense by default to not include the bridge, it will not cause
regression in this case.

^ permalink raw reply

* Re: [PATCH net-next 08/18] net: mvpp2: make the phy optional
From: Andrew Lunn @ 2017-07-26 16:20 UTC (permalink / raw)
  To: Antoine Tenart
  Cc: davem, jason, gregory.clement, sebastian.hesselbarth,
	thomas.petazzoni, nadavh, linux, mw, stefanc, netdev,
	linux-arm-kernel
In-Reply-To: <20170724134848.19330-9-antoine.tenart@free-electrons.com>

On Mon, Jul 24, 2017 at 03:48:38PM +0200, Antoine Tenart wrote:
> SFP ports do not necessarily need to have an Ethernet PHY between the
> SoC and the actual physical port. However, the driver currently makes
> the "phy" property mandatory, contrary to what is stated in the Device
> Tree binding.
> 
> To allow handling the PPv2 controller on those boards, this patch makes
> the PHY optional, and aligns the PPv2 driver on its device tree
> documentation.

It is an architectural question...

but with the boards i have with an SFF port, i actually use a
fixed-phy to represent the SFF. Then nothing special is needed.

Also, Russell King posted his phylink patches. Once accepted, you are
going to want to re-write some of this to make use of that code.

      Andrew

^ permalink raw reply

* Re: [PATCH net-next 07/18] net: mvpp2: initialize the GoP
From: Andrew Lunn @ 2017-07-26 16:16 UTC (permalink / raw)
  To: Antoine Tenart
  Cc: davem, jason, gregory.clement, sebastian.hesselbarth,
	thomas.petazzoni, nadavh, linux, mw, stefanc, netdev,
	linux-arm-kernel
In-Reply-To: <20170724134848.19330-8-antoine.tenart@free-electrons.com>

> +	switch (port->phy_interface) {
> +	case PHY_INTERFACE_MODE_RGMII:

Here as well,

     Andrew

^ permalink raw reply

* Re: [PATCH net-next 05/18] net: mvpp2: initialize the GMAC when using a port
From: Andrew Lunn @ 2017-07-26 16:14 UTC (permalink / raw)
  To: Antoine Tenart
  Cc: davem, jason, gregory.clement, sebastian.hesselbarth,
	thomas.petazzoni, nadavh, linux, mw, stefanc, netdev,
	linux-arm-kernel
In-Reply-To: <20170724134848.19330-6-antoine.tenart@free-electrons.com>

> +	} else if (port->phy_interface == PHY_INTERFACE_MODE_RGMII) {

Don't forget:
   PHY_INTERFACE_MODE_RGMII_ID,
   PHY_INTERFACE_MODE_RGMII_RXID,
   PHY_INTERFACE_MODE_RGMII_TXID,

> +		val = readl(port->base + MVPP22_GMAC_CTRL_4_REG);
> +		val |= MVPP22_CTRL4_EXT_PIN_GMII_SEL |
> +		       MVPP22_CTRL4_SYNC_BYPASS_DIS |
> +		       MVPP22_CTRL4_QSGMII_BYPASS_ACTIVE;
> +		val &= ~MVPP22_CTRL4_DP_CLK_SEL;
> +		writel(val, port->base + MVPP22_GMAC_CTRL_4_REG);
> +
> +		val = readl(port->base + MVPP2_GMAC_CTRL_2_REG);
> +		val &= ~MVPP2_GMAC_DISABLE_PADDING;
> +		writel(val, port->base + MVPP2_GMAC_CTRL_2_REG);
> +	}
> +
> +	/* The port is connected to a copper PHY */
> +	val = readl(port->base + MVPP2_GMAC_CTRL_0_REG);
> +	val &= ~MVPP2_GMAC_PORT_TYPE_MASK;
> +	writel(val, port->base + MVPP2_GMAC_CTRL_0_REG);
> +
> +	val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> +	val |= MVPP2_GMAC_IN_BAND_AUTONEG |
> +	       MVPP2_GMAC_IN_BAND_AUTONEG_BYPASS |
> +	       MVPP2_GMAC_AN_SPEED_EN | MVPP2_GMAC_FLOW_CTRL_AUTONEG |
> +	       MVPP2_GMAC_AN_DUPLEX_EN;
> +	writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> +}
> +
> +static void mvpp2_port_mii_gmac_configure(struct mvpp2_port *port)
> +{
> +	u32 val;
> +
> +	/* Force link down */
> +	val = readl(port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> +	val |= MVPP2_GMAC_FORCE_LINK_DOWN;
> +	writel(val, port->base + MVPP2_GMAC_AUTONEG_CONFIG);
> +
> +	/* Set the GMAC in a reset state */
> +	val = readl(port->base + MVPP2_GMAC_CTRL_2_REG);
> +	val |= MVPP2_GMAC_PORT_RESET_MASK;
> +	writel(val, port->base + MVPP2_GMAC_CTRL_2_REG);
> +
> +	/* Configure the PCS and in-band AN */
> +	val = readl(port->base + MVPP2_GMAC_CTRL_2_REG);
> +	if (port->phy_interface == PHY_INTERFACE_MODE_SGMII) {
> +	        val |= MVPP2_GMAC_INBAND_AN_MASK | MVPP2_GMAC_PCS_ENABLE_MASK;
> +	} else if (port->phy_interface == PHY_INTERFACE_MODE_RGMII) {

phy_interface_is_rgmii()

	Andrew

^ permalink raw reply

* [PULL] vhost: fixes, cleanups
From: Michael S. Tsirkin @ 2017-07-26 16:11 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: kvm, virtualization, netdev, linux-kernel, andriy.shevchenko,
	bigeasy, drjones, jasowang, liang.z.li, mst, wei.w.wang

All's quiet here this cycle, just some minor fixes.  A block printk
message bugfix is still pending for the next pull, didn't want to delay
this one as people are getting impatient.

The following changes since commit 520eccdfe187591a51ea9ab4c1a024ae4d0f68d9:

  Linux 4.13-rc2 (2017-07-23 16:15:17 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git tags/for_linus

for you to fetch changes up to cfa0ebc9d6d6308564f5174ecb655b9d504b2be5:

  virtio-net: fix module unloading (2017-07-25 16:37:36 +0300)

----------------------------------------------------------------
virtio: fixes, cleanups

Fixes some minor issues all over the codebase.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

----------------------------------------------------------------
Andrew Jones (1):
      virtio-net: fix module unloading

Andy Shevchenko (1):
      virtio_blk: Use sysfs_match_string() helper

Liang Li (1):
      virtio-balloon: deflate via a page list

Wei Wang (1):
      virtio-balloon: coding format cleanup

 drivers/block/virtio_blk.c      |  7 ++-----
 drivers/net/virtio_net.c        |  2 +-
 drivers/virtio/virtio_balloon.c | 28 ++++++++++++----------------
 3 files changed, 15 insertions(+), 22 deletions(-)

^ permalink raw reply

* Re: [PATCH net-next 04/18] net: mvpp2: move the mii configuration in the ndo_open path
From: Andrew Lunn @ 2017-07-26 16:11 UTC (permalink / raw)
  To: Antoine Tenart
  Cc: davem, jason, gregory.clement, sebastian.hesselbarth,
	thomas.petazzoni, nadavh, linux, mw, stefanc, netdev,
	linux-arm-kernel
In-Reply-To: <20170724134848.19330-5-antoine.tenart@free-electrons.com>

On Mon, Jul 24, 2017 at 03:48:34PM +0200, Antoine Tenart wrote:
> This moves the mii configuration in the ndo_open path, to allow handling
> different mii configurations later and to switch between these
> configurations at runtime.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> ---
>  drivers/net/ethernet/marvell/mvpp2.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
> index 6ffff929b22a..9d204ffb9b89 100644
> --- a/drivers/net/ethernet/marvell/mvpp2.c
> +++ b/drivers/net/ethernet/marvell/mvpp2.c
> @@ -5862,6 +5862,7 @@ static void mvpp2_start_dev(struct mvpp2_port *port)
>  	/* Enable interrupts on all CPUs */
>  	mvpp2_interrupts_enable(port);
>  
> +	mvpp2_port_mii_set(port);

Hi Antoine

You probably should take a look at mvpp2_port_mii_set() and have it
handle all PHY_INTERFACE_MODE_RGMII variants.

       Andrew

^ permalink raw reply

* Re: [PATCH net] Revert "vhost: cache used event for better performance"
From: Michael S. Tsirkin @ 2017-07-26 16:08 UTC (permalink / raw)
  To: Jason Wang; +Cc: netdev, linux-kernel, kvm, virtualization
In-Reply-To: <6b3a9a98-c095-1729-3528-dd521f136797@redhat.com>

On Wed, Jul 26, 2017 at 09:37:15PM +0800, Jason Wang wrote:
> 
> 
> On 2017年07月26日 21:18, Jason Wang wrote:
> > 
> > 
> > On 2017年07月26日 20:57, Michael S. Tsirkin wrote:
> > > On Wed, Jul 26, 2017 at 04:03:17PM +0800, Jason Wang wrote:
> > > > This reverts commit 809ecb9bca6a9424ccd392d67e368160f8b76c92. Since it
> > > > was reported to break vhost_net. We want to cache used event and use
> > > > it to check for notification. We try to valid cached used event by
> > > > checking whether or not it was ahead of new, but this is not correct
> > > > all the time, it could be stale and there's no way to know about this.
> > > > 
> > > > Signed-off-by: Jason Wang<jasowang@redhat.com>
> > > Could you supply a bit more data here please?  How does it get stale?
> > > What does guest need to do to make it stale?  This will be helpful if
> > > anyone wants to bring it back, or if we want to extend the protocol.
> > > 
> > 
> > The problem we don't know whether or not guest has published a new used
> > event. The check vring_need_event(vq->last_used_event, new + vq->num,
> > new) is not sufficient to check for this.
> > 
> > Thanks
> 
> More notes, the previous assumption is that we don't move used event back,
> but this could happen in fact if idx is wrapper around.

You mean if the 16 bit index wraps around after 64K entries.
Makes sense.

> Will repost and add
> this into commit log.
> 
> Thanks
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

^ permalink raw reply

* Re: [PATCH iproute2] bridge: Assume master at FDB modification
From: Stephen Hemminger @ 2017-07-26 16:08 UTC (permalink / raw)
  To: Arkadi Sharshevsky; +Cc: netdev, davem, nikolay, vivien.didelot, mlxsw
In-Reply-To: <1501083394-26180-1-git-send-email-arkadis@mellanox.com>

On Wed, 26 Jul 2017 18:36:34 +0300
Arkadi Sharshevsky <arkadis@mellanox.com> wrote:

> According to the man page the master flag should be the default, yet, the
> current code assumes otherwise.
> 
> Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>

Agree that the documentation and code don't match.
But your change could break users with existing scripts by changing behavior.

It would be safer to change the man page not the code.

^ permalink raw reply

* Re: [PATCH net-next 03/18] net: mvpp2: set the SMI PHY address when connecting to the PHY
From: Andrew Lunn @ 2017-07-26 16:08 UTC (permalink / raw)
  To: Antoine Tenart
  Cc: davem, jason, gregory.clement, sebastian.hesselbarth,
	thomas.petazzoni, nadavh, linux, mw, stefanc, netdev,
	linux-arm-kernel
In-Reply-To: <20170724134848.19330-4-antoine.tenart@free-electrons.com>

On Mon, Jul 24, 2017 at 03:48:33PM +0200, Antoine Tenart wrote:
> When connecting to the PHY, explicitly set the SMI PHY address in the
> controller registers to configure a given port to be connected to the
> selected PHY.
> 
> Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> ---
>  drivers/net/ethernet/marvell/mvpp2.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
> index 1e592abc9067..6ffff929b22a 100644
> --- a/drivers/net/ethernet/marvell/mvpp2.c
> +++ b/drivers/net/ethernet/marvell/mvpp2.c
> @@ -359,6 +359,8 @@
>  #define MVPP22_SMI_MISC_CFG_REG			0x1204
>  #define     MVPP22_SMI_POLLING_EN		BIT(10)
>  
> +#define MVPP22_SMI_PHY_ADDR(port)		(0x120c + (port) * 0x4)
> +
>  #define MVPP22_GMAC_BASE(port)		(0x7000 + (port) * 0x1000 + 0xe00)
>  
>  #define MVPP2_CAUSE_TXQ_SENT_DESC_ALL_MASK	0xff
> @@ -5939,7 +5941,9 @@ static void mvpp21_get_mac_address(struct mvpp2_port *port, unsigned char *addr)
>  
>  static int mvpp2_phy_connect(struct mvpp2_port *port)
>  {
> +	struct mvpp2 *priv = port->priv;
>  	struct phy_device *phy_dev;
> +	u32 phy_addr;
>  
>  	phy_dev = of_phy_connect(port->dev, port->phy_node, mvpp2_link_event, 0,
>  				 port->phy_interface);
> @@ -5954,6 +5958,16 @@ static int mvpp2_phy_connect(struct mvpp2_port *port)
>  	port->duplex  = 0;
>  	port->speed   = 0;
>  
> +	if (priv->hw_version != MVPP22)
> +		return 0;
> +
> +	/* Set the SMI PHY address */
> +	if (of_property_read_u32(port->phy_node, "reg", &phy_addr)) {
> +		netdev_err(port->dev, "cannot find the PHY address\n");
> +		return -EINVAL;
> +	}
> +
> +	writel(phy_addr, priv->iface_base + MVPP22_SMI_PHY_ADDR(port->gop_id));
>  	return 0;
>  }

Hi Antoine

You could use phy_dev->mdiodev->addr, rather than parse the DT.

Why does the MAC need to know this address? The phylib and PHY driver
should be the only thing accessing the PHY, otherwise you are asking
for trouble.

What if the PHY is hanging off some other mdio bus? I've got a
freescale board with dual ethernets and a Marvell switch on the
hardware MDIO bus and a PHY on a bit-banging MDIO bus.

	 Andrew

^ permalink raw reply

* [PATCH iproute2] bridge: Assume master at FDB modification
From: Arkadi Sharshevsky @ 2017-07-26 15:36 UTC (permalink / raw)
  To: netdev; +Cc: davem, stephen, nikolay, vivien.didelot, mlxsw,
	Arkadi Sharshevsky

According to the man page the master flag should be the default, yet, the
current code assumes otherwise.

Signed-off-by: Arkadi Sharshevsky <arkadis@mellanox.com>
---
 bridge/fdb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bridge/fdb.c b/bridge/fdb.c
index e5cebf9..7c77157 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -496,9 +496,9 @@ static int fdb_modify(int cmd, int flags, int argc, char **argv)
 		return -1;
 	}
 
-	/* Assume self */
+	/* Assume master */
 	if (!(req.ndm.ndm_flags&(NTF_SELF|NTF_MASTER)))
-		req.ndm.ndm_flags |= NTF_SELF;
+		req.ndm.ndm_flags |= NTF_MASTER;
 
 	/* Assume permanent */
 	if (!(req.ndm.ndm_state&(NUD_PERMANENT|NUD_REACHABLE)))
-- 
2.4.11

^ permalink raw reply related

* [PATCH net] udp6: fix socket leak on early demux
From: Paolo Abeni @ 2017-07-26 15:29 UTC (permalink / raw)
  To: netdev
  Cc: David S. Miller, Eric Dumazet, Sam Edwards, Marc Haber,
	Subash Abhinov Kasiviswanathan

When an early demuxed packet reaches __udp6_lib_lookup_skb(), the
sk reference is retrieved and used, but the relevant reference
count is leaked and the socket destructor is never called.
Beyond leaking the sk memory, if there are pending UDP packets
in the receive queue, even the related accounted memory is leaked.

In the long run, this will cause persistent forward allocation errors
and no UDP skbs (both ipv4 and ipv6) will be able to reach the
user-space.

Fix this by explicitly accessing the early demux reference before
the lookup, and properly decreasing the socket reference count
after usage.

Also drop the skb_steal_sock() in __udp6_lib_lookup_skb(), and
the now obsoleted comment about "socket cache".

The newly added code is derived from the current ipv4 code for the
similar path.

Reported-by: Sam Edwards <CFSworks@gmail.com>
Reported-by: Marc Haber <mh+netdev@zugschlus.de>
Fixes: 5425077d73e0 ("net: ipv6: Add early demux handler for UDP unicast")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 include/net/udp.h |  1 +
 net/ipv4/udp.c    |  3 ++-
 net/ipv6/udp.c    | 28 +++++++++++++++++++---------
 3 files changed, 22 insertions(+), 10 deletions(-)

diff --git a/include/net/udp.h b/include/net/udp.h
index 56ce2d2a612d..cc8036987dcb 100644
--- a/include/net/udp.h
+++ b/include/net/udp.h
@@ -260,6 +260,7 @@ static inline struct sk_buff *skb_recv_udp(struct sock *sk, unsigned int flags,
 }
 
 void udp_v4_early_demux(struct sk_buff *skb);
+void udp_sk_rx_dst_set(struct sock *sk, struct dst_entry *dst);
 int udp_get_port(struct sock *sk, unsigned short snum,
 		 int (*saddr_cmp)(const struct sock *,
 				  const struct sock *));
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index fac7cb9e3b0f..e6276fa3750b 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -1928,7 +1928,7 @@ static int udp_queue_rcv_skb(struct sock *sk, struct sk_buff *skb)
 /* For TCP sockets, sk_rx_dst is protected by socket lock
  * For UDP, we use xchg() to guard against concurrent changes.
  */
-static void udp_sk_rx_dst_set(struct sock *sk, struct dst_entry *dst)
+void udp_sk_rx_dst_set(struct sock *sk, struct dst_entry *dst)
 {
 	struct dst_entry *old;
 
@@ -1937,6 +1937,7 @@ static void udp_sk_rx_dst_set(struct sock *sk, struct dst_entry *dst)
 		dst_release(old);
 	}
 }
+EXPORT_SYMBOL(udp_sk_rx_dst_set);
 
 /*
  *	Multicasts and broadcasts go to each listener.
diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c
index 4a3e65626e8b..e74fe497d823 100644
--- a/net/ipv6/udp.c
+++ b/net/ipv6/udp.c
@@ -291,11 +291,7 @@ static struct sock *__udp6_lib_lookup_skb(struct sk_buff *skb,
 					  struct udp_table *udptable)
 {
 	const struct ipv6hdr *iph = ipv6_hdr(skb);
-	struct sock *sk;
 
-	sk = skb_steal_sock(skb);
-	if (unlikely(sk))
-		return sk;
 	return __udp6_lib_lookup(dev_net(skb->dev), &iph->saddr, sport,
 				 &iph->daddr, dport, inet6_iif(skb),
 				 udptable, skb);
@@ -804,6 +800,25 @@ int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
 	if (udp6_csum_init(skb, uh, proto))
 		goto csum_error;
 
+	/* Check if the socket is already available, e.g. due to early demux */
+	sk = skb_steal_sock(skb);
+	if (sk) {
+		struct dst_entry *dst = skb_dst(skb);
+		int ret;
+
+		if (unlikely(sk->sk_rx_dst != dst))
+			udp_sk_rx_dst_set(sk, dst);
+
+		ret = udpv6_queue_rcv_skb(sk, skb);
+		sock_put(sk);
+		/* a return value > 0 means to resubmit the input, but
+		 * it wants the return to be -protocol, or 0
+		 */
+		if (ret > 0)
+			return -ret;
+		return 0;
+	}
+
 	/*
 	 *	Multicast receive code
 	 */
@@ -812,11 +827,6 @@ int __udp6_lib_rcv(struct sk_buff *skb, struct udp_table *udptable,
 				saddr, daddr, udptable, proto);
 
 	/* Unicast */
-
-	/*
-	 * check socket cache ... must talk to Alan about his plans
-	 * for sock caches... i'll skip this for now.
-	 */
 	sk = __udp6_lib_lookup_skb(skb, uh->source, uh->dest, udptable);
 	if (sk) {
 		int ret;
-- 
2.13.3

^ permalink raw reply related

* Re: [PATCH net] Revert "vhost: cache used event for better performance"
From: Michael S. Tsirkin @ 2017-07-26 15:21 UTC (permalink / raw)
  To: Jason Wang; +Cc: virtualization, kvm, netdev, linux-kernel
In-Reply-To: <117542fc-eef9-c043-7c9e-daafceb7db4e@redhat.com>

On Wed, Jul 26, 2017 at 09:18:02PM +0800, Jason Wang wrote:
> 
> 
> On 2017年07月26日 20:57, Michael S. Tsirkin wrote:
> > On Wed, Jul 26, 2017 at 04:03:17PM +0800, Jason Wang wrote:
> > > This reverts commit 809ecb9bca6a9424ccd392d67e368160f8b76c92. Since it
> > > was reported to break vhost_net. We want to cache used event and use
> > > it to check for notification. We try to valid cached used event by
> > > checking whether or not it was ahead of new, but this is not correct
> > > all the time, it could be stale and there's no way to know about this.
> > > 
> > > Signed-off-by: Jason Wang<jasowang@redhat.com>
> > Could you supply a bit more data here please?  How does it get stale?
> > What does guest need to do to make it stale?  This will be helpful if
> > anyone wants to bring it back, or if we want to extend the protocol.
> > 
> 
> The problem we don't know whether or not guest has published a new used
> event. The check vring_need_event(vq->last_used_event, new + vq->num, new)
> is not sufficient to check for this.
> 
> Thanks

Figured it out after an offline discussion.

^ permalink raw reply

* [PATCH 2/2] phy: bcm-ns-usb3: fix MDIO_BUS dependency
From: Arnd Bergmann @ 2017-07-26 15:14 UTC (permalink / raw)
  To: netdev, Kishon Vijay Abraham I, David S . Miller
  Cc: Andrew Lunn, Florian Fainelli, linux-kernel, Raviteja Garimella,
	Arnd Bergmann, Rafał Miłecki
In-Reply-To: <20170726151442.3913622-1-arnd@arndb.de>

The driver attempts to 'select MDIO_DEVICE', but the code
is actually a loadable module when PHYLIB=m:

drivers/phy/broadcom/phy-bcm-ns-usb3.o: In function `bcm_ns_usb3_mdiodev_phy_write':
phy-bcm-ns-usb3.c:(.text.bcm_ns_usb3_mdiodev_phy_write+0x28): undefined reference to `mdiobus_write'
drivers/phy/broadcom/phy-bcm-ns-usb3.o: In function `bcm_ns_usb3_module_exit':
phy-bcm-ns-usb3.c:(.exit.text+0x18): undefined reference to `mdio_driver_unregister'
drivers/phy/broadcom/phy-bcm-ns-usb3.o: In function `bcm_ns_usb3_module_init':
phy-bcm-ns-usb3.c:(.init.text+0x18): undefined reference to `mdio_driver_register'
phy-bcm-ns-usb3.c:(.init.text+0x38): undefined reference to `mdio_driver_unregister'

Using 'depends on MDIO_BUS' instead will avoid the link error.

Fixes: af850e14a7ae ("phy: bcm-ns-usb3: add MDIO driver using proper bus layer")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/phy/broadcom/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/phy/broadcom/Kconfig b/drivers/phy/broadcom/Kconfig
index 37371b89b14f..64fc59c3ae6d 100644
--- a/drivers/phy/broadcom/Kconfig
+++ b/drivers/phy/broadcom/Kconfig
@@ -30,8 +30,8 @@ config PHY_BCM_NS_USB3
 	tristate "Broadcom Northstar USB 3.0 PHY Driver"
 	depends on ARCH_BCM_IPROC || COMPILE_TEST
 	depends on HAS_IOMEM && OF
+	depends on MDIO_BUS
 	select GENERIC_PHY
-	select MDIO_DEVICE
 	help
 	  Enable this to support Broadcom USB 3.0 PHY connected to the USB
 	  controller on Northstar family.
-- 
2.9.0

^ permalink raw reply related

* [PATCH 1/2] net: phy: rework Kconfig settings for MDIO_BUS
From: Arnd Bergmann @ 2017-07-26 15:13 UTC (permalink / raw)
  To: netdev, Kishon Vijay Abraham I, David S . Miller, Andrew Lunn,
	Florian Fainelli
  Cc: linux-kernel, Raviteja Garimella, Arnd Bergmann, Raju Lakkaraju

I still see build errors in randconfig builds and have had this
patch for a while to locally work around it:

drivers/built-in.o: In function `xgene_mdio_probe':
mux-core.c:(.text+0x352154): undefined reference to `of_mdiobus_register'
mux-core.c:(.text+0x352168): undefined reference to `mdiobus_free'
mux-core.c:(.text+0x3521c0): undefined reference to `mdiobus_alloc_size'

The idea is that CONFIG_MDIO_BUS now reflects whether the mdio_bus
code is built-in or a module, and other drivers that use the core
code can simply depend on that, instead of having a complex
dependency line.

Fixes: 90eff9096c01 ("net: phy: Allow splitting MDIO bus/device support from PHYs")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
---
Submitted this version on June 22 2017 originally as RFC, resending
for inclusion this time.

Who should pick up the two patches? I guess they could go either
through netdev or the generic-phy tree, but they should stay together
as a series.
---
 drivers/net/phy/Kconfig | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index 2dda72004a7d..928fd892f167 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -7,7 +7,16 @@ menuconfig MDIO_DEVICE
 	help
 	   MDIO devices and driver infrastructure code.
 
-if MDIO_DEVICE
+config MDIO_BUS
+	tristate
+	default m if PHYLIB=m
+	default MDIO_DEVICE
+	help
+	  This internal symbol is used for link time dependencies and it
+	  reflects whether the mdio_bus/mdio_device code is built as a
+	  loadable module or built-in.
+
+if MDIO_BUS
 
 config MDIO_BCM_IPROC
 	tristate "Broadcom iProc MDIO bus controller"
@@ -28,7 +37,6 @@ config MDIO_BCM_UNIMAC
 
 config MDIO_BITBANG
 	tristate "Bitbanged MDIO buses"
-	depends on !(MDIO_DEVICE=y && PHYLIB=m)
 	help
 	  This module implements the MDIO bus protocol in software,
 	  for use by low level drivers that export the ability to
@@ -127,7 +135,6 @@ config MDIO_THUNDER
 	tristate "ThunderX SOCs MDIO buses"
 	depends on 64BIT
 	depends on PCI
-	depends on !(MDIO_DEVICE=y && PHYLIB=m)
 	select MDIO_CAVIUM
 	help
 	  This driver supports the MDIO interfaces found on Cavium
-- 
2.9.0

^ permalink raw reply related

* Re: [PATCH V2 3/4] net-next: dsa: fix flow dissection
From: Andrew Lunn @ 2017-07-26 15:10 UTC (permalink / raw)
  To: John Crispin
  Cc: Vivien Didelot, Florian Fainelli, David S . Miller, netdev,
	linux-kernel
In-Reply-To: <20170721085813.30789-3-john@phrozen.org>

On Fri, Jul 21, 2017 at 10:58:12AM +0200, John Crispin wrote:
> RPS and probably other kernel features are currently broken on some if not
> all DSA devices. The root cause of this is that skb_hash will call the
> flow_dissector. At this point the skb still contains the magic switch header
> and the skb->protocol field is not set up to the correct 802.3 value yet.
> By the time the tag specific code is called, removing the header and
> properly setting the protocol an invalid hash is already set. In the case
> of the mt7530 this will result in all flows always having the same hash.
> 
> This patch makes the flow dissector honour the nh and protocol offset
> defined by the dsa tag driver thus fixing dissection, hashing and RPS.
> 
> Signed-off-by: John Crispin <john@phrozen.org>
> ---
>  net/core/flow_dissector.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
> index fc5fc4594c90..1268ae75c3b3 100644
> --- a/net/core/flow_dissector.c
> +++ b/net/core/flow_dissector.c
> @@ -4,6 +4,7 @@
>  #include <linux/ip.h>
>  #include <linux/ipv6.h>
>  #include <linux/if_vlan.h>
> +#include <net/dsa.h>
>  #include <net/ip.h>
>  #include <net/ipv6.h>
>  #include <net/gre.h>
> @@ -440,6 +441,17 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
>  			 skb->vlan_proto : skb->protocol;
>  		nhoff = skb_network_offset(skb);
>  		hlen = skb_headlen(skb);
> +
> +		if (unlikely(netdev_uses_dsa(skb->dev))) {
> +			const struct dsa_device_ops *ops;
> +			u8 *p = (u8 *)data;
> +
> +			ops = skb->dev->dsa_ptr->tag_ops;
> +			if (ops->hash_proto_off)
> +				proto = (u16)p[ops->hash_proto_off];

Hi John

Unfortunately, this is not generic enough to work for DSA and EDSA
tagging. With these tagging protocols, the size of the tag depends on
the presence or not of a VLAN header.

To make this work for all tagging protocols, we are going to need to
add an a new op to tag_ops.

    Andrew

^ permalink raw reply

* (unknown), 
From: venkatvenkatsubra @ 2017-07-26 14:35 UTC (permalink / raw)
  To: netdev

Greetings Netdev

http://mondesign.jp/list-view.php?result=2b7f5x3fc4gxussdn





venkatvenkatsubra

^ permalink raw reply

* Re: [PATCH V2 1/4] net-next: dsa: move struct dsa_device_ops to the global header file
From: Andrew Lunn @ 2017-07-26 14:53 UTC (permalink / raw)
  To: John Crispin
  Cc: Vivien Didelot, Florian Fainelli, David S . Miller, netdev,
	linux-kernel
In-Reply-To: <20170721085813.30789-1-john@phrozen.org>

On Fri, Jul 21, 2017 at 10:58:10AM +0200, John Crispin wrote:
> We need to access this struct from within the flow_dissector to fix
> dissection for packets coming in on DSA devices.
> 
> Signed-off-by: John Crispin <john@phrozen.org>

Reviewed-by: Andrew Lunn <andrew@lunn.ch>

    Andrew

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox