Netdev List
 help / color / mirror / Atom feed
* Re: [PATCH v2 net-next 0/8] bpf: Add option to set mark and priority in cgroup sock programs
From: David Miller @ 2017-08-29 21:53 UTC (permalink / raw)
  To: dsahern; +Cc: netdev, daniel, ast, tj
In-Reply-To: <1503687941-626-1-git-send-email-dsahern@gmail.com>

From: David Ahern <dsahern@gmail.com>
Date: Fri, 25 Aug 2017 12:05:33 -0700

> Add option to set mark and priority in addition to bound device for newly
> created sockets. Also, allow the bpf programs to use the get_current_uid_gid
> helper meaning socket marks, priority and device can be set base on the
> uid/gid of the running process.
> 
> For flexbility in deploying these programs, option is added to allow cgroups
> to be walked from current to root running any program attached. This allows
> one cgroup level to control the device a socket is bound to (e.g, a VRF) while
> cgroups can be used to set socket marks and priority.
> 
> Sample programs are updated to demonstrate the new options.
> 
> v2
> - added flag to control recursive behavior as requested by Alexei
> - added comment to sock_filter_func_proto regarding use of
>   get_current_uid_gid helper
> - updated test programs for recursive option

I'm marking this patch series as "deferred" while the semantic issues
keep getting discussed.

Thanks.

^ permalink raw reply

* Re: [PATCH net-next 0/4] Endian fixes for SYSTEMPORT/SF2/MDIO
From: David Miller @ 2017-08-29 21:52 UTC (permalink / raw)
  To: f.fainelli; +Cc: netdev, opendmb, andrew, vivien.didelot
In-Reply-To: <d8ae99de-398d-f314-309f-50c36df9240a@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Tue, 29 Aug 2017 14:45:30 -0700

> On 08/29/2017 02:42 PM, David Miller wrote:
>> From: Florian Fainelli <f.fainelli@gmail.com>
>> Date: Tue, 29 Aug 2017 11:39:41 -0700
>> 
>>> While trying an ARM BE kernel for kinks, the 3 drivers below started not
>>> working and the reasons why became pretty obvious because the register space
>>> remains LE (hardwired), except for Broadcom MIPS where it follows the CPU's
>>> native endian (let's call that a feature).
>> 
>> Series applied, thanks Florian.
> 
> If you have not pushed yet, seems like not, can you check you applied v2
> of this patch series, in particular this patch:
> 
> http://patchwork.ozlabs.org/patch/807296/

If you didn't keep messing with the patchwork state of patches in my
queue this confusion would not have happened.

I did happen to apply v2, but because of all of the confusion you keep
creating, I used the header message from v1.

It's already pushed out so there is nothing we can do about it.

^ permalink raw reply

* Re: [PATCH net-next v3] bridge: fdb add and delete tracepoints
From: David Miller @ 2017-08-29 21:49 UTC (permalink / raw)
  To: roopa; +Cc: nikolay, netdev, f.fainelli, bridge
In-Reply-To: <1504037817-38107-1-git-send-email-roopa@cumulusnetworks.com>

From: Roopa Prabhu <roopa@cumulusnetworks.com>
Date: Tue, 29 Aug 2017 13:16:57 -0700

> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> 
> A few useful tracepoints to trace bridge forwarding
> database updates.
> 
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> ---
> v2: address comments from florian
> v3: remove stray character '=' in print (pointed out by florian)

Applied, thanks.

^ permalink raw reply

* Re: [PATCH net-next 0/4] Endian fixes for SYSTEMPORT/SF2/MDIO
From: Florian Fainelli @ 2017-08-29 21:45 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, opendmb, andrew, vivien.didelot
In-Reply-To: <20170829.144254.740239595444825239.davem@davemloft.net>

On 08/29/2017 02:42 PM, David Miller wrote:
> From: Florian Fainelli <f.fainelli@gmail.com>
> Date: Tue, 29 Aug 2017 11:39:41 -0700
> 
>> While trying an ARM BE kernel for kinks, the 3 drivers below started not
>> working and the reasons why became pretty obvious because the register space
>> remains LE (hardwired), except for Broadcom MIPS where it follows the CPU's
>> native endian (let's call that a feature).
> 
> Series applied, thanks Florian.

If you have not pushed yet, seems like not, can you check you applied v2
of this patch series, in particular this patch:

http://patchwork.ozlabs.org/patch/807296/

Thanks!
-- 
Florian

^ permalink raw reply

* Re: [PATCH net-next 0/4] Endian fixes for SYSTEMPORT/SF2/MDIO
From: David Miller @ 2017-08-29 21:42 UTC (permalink / raw)
  To: f.fainelli; +Cc: netdev, opendmb, andrew, vivien.didelot
In-Reply-To: <1504031985-52808-1-git-send-email-f.fainelli@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Tue, 29 Aug 2017 11:39:41 -0700

> While trying an ARM BE kernel for kinks, the 3 drivers below started not
> working and the reasons why became pretty obvious because the register space
> remains LE (hardwired), except for Broadcom MIPS where it follows the CPU's
> native endian (let's call that a feature).

Series applied, thanks Florian.

^ permalink raw reply

* Re: [PATCH 0/4] irda: move it to drivers/staging so we can delete it
From: Ondrej Zary @ 2017-08-29 21:32 UTC (permalink / raw)
  To: David Miller; +Cc: gregkh, samuel, netdev, linux-kernel, devel
In-Reply-To: <20170828.164208.1908438929113102094.davem@davemloft.net>

On Tuesday 29 August 2017 01:42:08 David Miller wrote:
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Date: Sun, 27 Aug 2017 17:03:30 +0200
>
> > The IRDA code has long been obsolete and broken.  So, to keep people
> > from trying to use it, and to prevent people from having to maintain it,
> > let's move it to drivers/staging/ so that we can delete it entirely from
> > the kernel in a few releases.
>
> No objection, I'll apply this to net-next, thanks Greg.

IRDA works fine in Debian 9 (kernel 4.9) and I use it for simple file 
transfer. Hope I'm not the only one...

# irattach /dev/ttyS0 -d tekram -s
# irdadump
21:28:52.830350 xid:cmd aed8eb79 > ffffffff S=6 s=0 (14)
21:28:52.922368 xid:cmd aed8eb79 > ffffffff S=6 s=1 (14)
21:28:53.014350 xid:cmd aed8eb79 > ffffffff S=6 s=2 (14)
21:28:53.106338 xid:cmd aed8eb79 > ffffffff S=6 s=3 (14)
21:28:53.190276 xid:rsp aed8eb79 < 000035d1 S=6 s=3 Nokia 6230i hint=b125 [ 
PnP Modem Fax Telephony IrCOMM IrOBEX ] (28)
21:28:53.198384 xid:cmd aed8eb79 > ffffffff S=6 s=4 (14)
21:28:53.290382 xid:cmd aed8eb79 > ffffffff S=6 s=5 (14)
21:28:53.382341 xid:cmd aed8eb79 > ffffffff S=6 s=* pentium hint=0400 [ 
Computer ] (23)
^C
8 packets received by filter

$ obexftp -i -l MMC
Connecting..\done
Receiving "MMC".../<?xml version="1.0"?>
<!DOCTYPE folder-listing SYSTEM "obex-folder-listing.dtd"
 [ <!ATTLIST folder mem-type CDATA #IMPLIED> ]>
<folder-listing version="1.0">
    <parent-folder />
    <file name="Image000.jpg" size="304300" modified="20160219T135924" 
user-perm="RWD"/>
    <file name="Image001.jpg" size="270037" modified="20170811T233122" 
user-perm="RWD"/>
    <file name="Image004.jpg" size="53519" modified="20170814T074550" 
user-perm="RWD"/>
....
$ obexftp -i -c MMC -g Image004.jpg
Connecting..\done
Sending "MMC"...|done
Receiving "Image004.jpg"...-done
Disconnecting..\done


-- 
Ondrej Zary

^ permalink raw reply

* Re: [PATCH] DSA support for Micrel KSZ8895
From: Florian Fainelli @ 2017-08-29 21:23 UTC (permalink / raw)
  To: Pavel Machek, Andrew Lunn, Woojung.Huh
  Cc: Maxim Uvarov, nathan.leigh.conrad, Vivien Didelot, netdev,
	linux-kernel, Tristram.Ha
In-Reply-To: <20170829211519.GA24221@amd>

On 08/29/2017 02:15 PM, Pavel Machek wrote:
> On Tue 2017-08-29 14:26:04, Andrew Lunn wrote:
>>> But the MDIO emaulation code is from their driver, after lots of
>>> deletions.
>>
>> Is this driver supposed to run on lots of different OSs? That would
>> explain why they ignored the Linux MDIO and PHY layers.
> 
> It did not look particulary portable.

Part of the problem is that they need to duplicate the standard MII
definitions, whereas we could re-use those from include/linux/mii.h
and/or mdio.h.

> 
>> If possible, please make use of the Linux infrastructure.
> 
> I did not find any infrastructure I could use instead
> ksz_mdio_emulation.

fixed PHY/swphy.c is as close as it could get, but it is a highly
simplified version of this.

> 
> Now, drivers/net/phy/spi_ks8995.c can access the PHY registers, and I
> can not see any translation there, so there may be something I'm
> missing.

I don't see anything in that driver that seems to access PHY registers
what makes you think it does?

There's got to be a way to perform indirect accesses through SPI,
Woojung, do you know?

> 
> Pointers would be welcome at this point.
> 
> Thanks,
> 									Pavel
> 


-- 
Florian

^ permalink raw reply

* Re: mlxsw and rtnl lock
From: Arkadi Sharshevsky @ 2017-08-29 21:18 UTC (permalink / raw)
  To: David Ahern, Ido Schimmel; +Cc: Jiri Pirko, netdev@vger.kernel.org, mlxsw
In-Reply-To: <59774544-88d7-1f2a-82c6-28bb6c7ac747@gmail.com>



On 08/29/2017 11:04 PM, David Ahern wrote:
> On 8/29/17 12:10 AM, Arkadi Sharshevsky wrote:
>>
>>
>> On 08/28/2017 09:00 PM, David Ahern wrote:
>>> On 8/26/17 11:04 AM, Ido Schimmel wrote:
>>>> Regarding the silent abort, that's intentional. You can look at the same
>>>> code in v4.9 - when the chain was still blocking - and you'll see that
>>>> we didn't propagate the error even then. This was discussed in the past
>>>> and the conclusion was that user doesn't expect to operation to fail. If
>>>> hardware resources are exceeded, we let the kernel take care of the
>>>> forwarding instead.
>>>>
>>>
>>> In addition to Roopa's comments... The silent abort is not a good user
>>> experience. Right now it's add a network address or route, cross fingers
>>> and hope it does not overflow some limit (nexthop, ecmp, neighbor,
>>> prefix, etc) that triggers the offload abort.
>>>
>>> The mlxsw driver queries for some limits (e.g., max rifs) but I don't
>>> see any query related to current usage, and there is no API to pass any
>>> of that data to user space so user space has no programmatic way to
>>> handle this. I realize you are aware of this limitation. The point is to
>>> emphasize the need to resolve this.
>>>
>>
>> We actually thought about providing he user some tools to understand
>> the ASIC's limitations by introducing the 'resource' object to devlink.
>>
>> By linking dpipe tables to resources the user can understand which
>> hardware processes share a common resource, furthermore this resources
>> usage could be observed. By this more visibility can be obtained.
>>
>> Its not a remedy for the silent abort, but, maybe a notification
>> can be sent from devlink in case of abort that some resources is
>> full.
>>
>> This proposition was sent as RFC several weeks ago.
>>
> 
> Do you have patches (kernel and devlink) for the proposal?
> 

No, only the design RFC which describe the UAPI, devlink
commands and the devlink/driver interactions.

I wanted to receive some feedback before the coding.

^ permalink raw reply

* Re: [PATCH] DSA support for Micrel KSZ8895
From: Pavel Machek @ 2017-08-29 21:15 UTC (permalink / raw)
  To: Andrew Lunn
  Cc: Maxim Uvarov, Woojung.Huh, nathan.leigh.conrad, Vivien Didelot,
	Florian Fainelli, netdev, linux-kernel, Tristram.Ha
In-Reply-To: <20170829122604.GA22093@lunn.ch>

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

On Tue 2017-08-29 14:26:04, Andrew Lunn wrote:
> > But the MDIO emaulation code is from their driver, after lots of
> > deletions.
> 
> Is this driver supposed to run on lots of different OSs? That would
> explain why they ignored the Linux MDIO and PHY layers.

It did not look particulary portable.

> If possible, please make use of the Linux infrastructure.

I did not find any infrastructure I could use instead
ksz_mdio_emulation.

Now, drivers/net/phy/spi_ks8995.c can access the PHY registers, and I
can not see any translation there, so there may be something I'm
missing.

Pointers would be welcome at this point.

Thanks,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: [PATCH RFC 2/2] virtio_net: enable probing for NEEDS_RESET support
From: Michael S. Tsirkin @ 2017-08-29 21:13 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Network Development, Jason Wang, virtualization, Willem de Bruijn,
	virtio-dev
In-Reply-To: <CAF=yD-+gPOYSgq8ckJ4wt3TPQYEnnB36g=Q5G4=yfoesGc=4hA@mail.gmail.com>

On Tue, Aug 29, 2017 at 05:02:29PM -0400, Willem de Bruijn wrote:
> + virtio-dev
> 
> On Tue, Aug 29, 2017 at 4:38 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Tue, Aug 29, 2017 at 04:27:41PM -0400, Willem de Bruijn wrote:
> >> On Tue, Aug 29, 2017 at 4:16 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> > On Tue, Aug 29, 2017 at 04:07:59PM -0400, Willem de Bruijn wrote:
> >> >> From: Willem de Bruijn <willemb@google.com>
> >> >>
> >> >> Implement a mechanism to signal that a virtio device implements the
> >> >> VIRTIO_CONFIG_S_NEEDS_RESET command.
> >> >>
> >> >> Testing for VIRTIO_CONFIG_S_NEEDS_RESET support by issuing the request
> >> >> and verifying the reset state would require an expensive state change.
> >> >>
> >> >> To avoid that, add a status bit to the feature register used during
> >> >> feature negotiation between host and guest.
> >> >>
> >> >> Set the bit for virtio-net, which supports the feature.
> >> >>
> >> >> Signed-off-by: Willem de Bruijn <willemb@google.com>
> >> >
> >> > All virtio 1 devices have the reset feature
> >>
> >> I don't quite follow. No device drivers implement this request currently?
> >
> > Depends. Spec 1.0 describes the bit and that driver can respond
> > by reseting the device. You seem to do something else
> > in this patchset, but as designed in 1.0 it does not seem to need
> > a feature bit.
> 
> I see. So support is designed to be best effort?
> 
> The feature bit is only needed if driver support is optional.
> 
> The ack response is necessary if the device acts differently
> depending on whether the reset happened. The device has
> to reset its local state, too, so I think that this is needed.

That reset should only happen when guest driver resets the device.
And spec already has a mechanism for that anyway.

> 
> >> > so maybe guest does
> >> > not need this flag. Does device need it? Does device really
> >> > care that guest can't recover?
> >>
> >> If all device drivers support it, then the flag is not needed.
> >>
> >> But as long as legacy device drivers can exist that do not support
> >> this feature, it has to have a way of differentiating between the two.
> >
> > Why? Device won't set this unless it's in a bad state. In that case,
> > setting the bit is harmless even if drivers ignore it.
> 
> The goal is for the device to be able to rely on the driver reset to get
> to a good state even if it gets it into a bad state.
> 
> That allows it to implement optimizations that could get it into that bad
> state.

I see. I don't think this is what the need reset was designed for.

We can extend it to cover this case but let's add a bit more
documentation then.

And in particular won't it be better if we could just reset one ring,
and not the whole device state? This might be nicer so flows on other
rings aren't disrupted.

> 
> In particular, in the edge case where the device performs backpressure,
> takes the descriptor out of the avail ring and does not immediately post
> it to the used ring.
> 
> A reset will make the guest free all delayed packets and treat any
> unsent and unacknowledged as network drops. This allows the
> device to indeed drop long delayed packets when they eventually
> surface (e.g., leave a qdisc queue).

In this particular case, won't it be easier for device to just
report all packets as used, without involving the guest?

> This is of course not safe with
> zerocopy packets.


I wonder if we can teach kernel to drop zero copy packets too.

-- 
MST

^ permalink raw reply

* Re: [PATCH RFC 2/2] virtio_net: enable probing for NEEDS_RESET support
From: Willem de Bruijn @ 2017-08-29 21:02 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Network Development, Jason Wang, virtualization, Willem de Bruijn,
	virtio-dev
In-Reply-To: <20170829233525-mutt-send-email-mst@kernel.org>

+ virtio-dev

On Tue, Aug 29, 2017 at 4:38 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Aug 29, 2017 at 04:27:41PM -0400, Willem de Bruijn wrote:
>> On Tue, Aug 29, 2017 at 4:16 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
>> > On Tue, Aug 29, 2017 at 04:07:59PM -0400, Willem de Bruijn wrote:
>> >> From: Willem de Bruijn <willemb@google.com>
>> >>
>> >> Implement a mechanism to signal that a virtio device implements the
>> >> VIRTIO_CONFIG_S_NEEDS_RESET command.
>> >>
>> >> Testing for VIRTIO_CONFIG_S_NEEDS_RESET support by issuing the request
>> >> and verifying the reset state would require an expensive state change.
>> >>
>> >> To avoid that, add a status bit to the feature register used during
>> >> feature negotiation between host and guest.
>> >>
>> >> Set the bit for virtio-net, which supports the feature.
>> >>
>> >> Signed-off-by: Willem de Bruijn <willemb@google.com>
>> >
>> > All virtio 1 devices have the reset feature
>>
>> I don't quite follow. No device drivers implement this request currently?
>
> Depends. Spec 1.0 describes the bit and that driver can respond
> by reseting the device. You seem to do something else
> in this patchset, but as designed in 1.0 it does not seem to need
> a feature bit.

I see. So support is designed to be best effort?

The feature bit is only needed if driver support is optional.

The ack response is necessary if the device acts differently
depending on whether the reset happened. The device has
to reset its local state, too, so I think that this is needed.


>> > so maybe guest does
>> > not need this flag. Does device need it? Does device really
>> > care that guest can't recover?
>>
>> If all device drivers support it, then the flag is not needed.
>>
>> But as long as legacy device drivers can exist that do not support
>> this feature, it has to have a way of differentiating between the two.
>
> Why? Device won't set this unless it's in a bad state. In that case,
> setting the bit is harmless even if drivers ignore it.

The goal is for the device to be able to rely on the driver reset to get
to a good state even if it gets it into a bad state.

That allows it to implement optimizations that could get it into that bad
state.

In particular, in the edge case where the device performs backpressure,
takes the descriptor out of the avail ring and does not immediately post
it to the used ring.

A reset will make the guest free all delayed packets and treat any
unsent and unacknowledged as network drops. This allows the
device to indeed drop long delayed packets when they eventually
surface (e.g., leave a qdisc queue). This is of course not safe with
zerocopy packets.

^ permalink raw reply

* [PATCH net-next v2 4/4] net: phy: mdio-bcm-unimac: Use correct I/O accessors
From: Florian Fainelli @ 2017-08-29 20:35 UTC (permalink / raw)
  To: netdev; +Cc: davem, opendmb, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <1504038918-49254-1-git-send-email-f.fainelli@gmail.com>

The driver currently uses __raw_{read,write}l which works for all
platforms supported: Broadcom MIPS LE/BE (native endian), ARM LE (native
endian) but not ARM BE (registers are still LE). Switch to using the
proper accessors for all platforms and explain why Broadcom MIPS BE is
special here, in doing so, we introduce a couple of helper functions to
abstract these differences.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/phy/mdio-bcm-unimac.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

diff --git a/drivers/net/phy/mdio-bcm-unimac.c b/drivers/net/phy/mdio-bcm-unimac.c
index 73c5267a11fd..08e0647b85e2 100644
--- a/drivers/net/phy/mdio-bcm-unimac.c
+++ b/drivers/net/phy/mdio-bcm-unimac.c
@@ -47,18 +47,38 @@ struct unimac_mdio_priv {
 	void			*wait_func_data;
 };
 
+static inline u32 unimac_mdio_readl(struct unimac_mdio_priv *priv, u32 offset)
+{
+	/* MIPS chips strapped for BE will automagically configure the
+	 * peripheral registers for CPU-native byte order.
+	 */
+	if (IS_ENABLED(CONFIG_MIPS) && IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
+		return __raw_readl(priv->base + offset);
+	else
+		return readl_relaxed(priv->base + offset);
+}
+
+static inline void unimac_mdio_writel(struct unimac_mdio_priv *priv, u32 val,
+				      u32 offset)
+{
+	if (IS_ENABLED(CONFIG_MIPS) && IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
+		__raw_writel(val, priv->base + offset);
+	else
+		writel_relaxed(val, priv->base + offset);
+}
+
 static inline void unimac_mdio_start(struct unimac_mdio_priv *priv)
 {
 	u32 reg;
 
-	reg = __raw_readl(priv->base + MDIO_CMD);
+	reg = unimac_mdio_readl(priv, MDIO_CMD);
 	reg |= MDIO_START_BUSY;
-	__raw_writel(reg, priv->base + MDIO_CMD);
+	unimac_mdio_writel(priv, reg, MDIO_CMD);
 }
 
 static inline unsigned int unimac_mdio_busy(struct unimac_mdio_priv *priv)
 {
-	return __raw_readl(priv->base + MDIO_CMD) & MDIO_START_BUSY;
+	return unimac_mdio_readl(priv, MDIO_CMD) & MDIO_START_BUSY;
 }
 
 static int unimac_mdio_poll(void *wait_func_data)
@@ -87,7 +107,7 @@ static int unimac_mdio_read(struct mii_bus *bus, int phy_id, int reg)
 
 	/* Prepare the read operation */
 	cmd = MDIO_RD | (phy_id << MDIO_PMD_SHIFT) | (reg << MDIO_REG_SHIFT);
-	__raw_writel(cmd, priv->base + MDIO_CMD);
+	unimac_mdio_writel(priv, cmd, MDIO_CMD);
 
 	/* Start MDIO transaction */
 	unimac_mdio_start(priv);
@@ -96,7 +116,7 @@ static int unimac_mdio_read(struct mii_bus *bus, int phy_id, int reg)
 	if (ret)
 		return ret;
 
-	cmd = __raw_readl(priv->base + MDIO_CMD);
+	cmd = unimac_mdio_readl(priv, MDIO_CMD);
 
 	/* Some broken devices are known not to release the line during
 	 * turn-around, e.g: Broadcom BCM53125 external switches, so check for
@@ -118,7 +138,7 @@ static int unimac_mdio_write(struct mii_bus *bus, int phy_id,
 	/* Prepare the write operation */
 	cmd = MDIO_WR | (phy_id << MDIO_PMD_SHIFT) |
 		(reg << MDIO_REG_SHIFT) | (0xffff & val);
-	__raw_writel(cmd, priv->base + MDIO_CMD);
+	unimac_mdio_writel(priv, cmd, MDIO_CMD);
 
 	unimac_mdio_start(priv);
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v2 3/4] net: systemport: Set correct RSB endian bits based on host
From: Florian Fainelli @ 2017-08-29 20:35 UTC (permalink / raw)
  To: netdev; +Cc: davem, opendmb, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <1504038918-49254-1-git-send-email-f.fainelli@gmail.com>

RSB_SWAP0 needs to match the host CPU endian, and it needs to be set
for LE and clear for BE. RSB_SWAP1 must always be cleared for SYSTEMPORT
Lite.

With these settings, we have the Receive Status Block always match the
host endian and we do not need to perform any conversion. Since there is
not necessarily a CONFIG_CPU_LITTLE_ENDIAN option defined, we test for
!CONFIG_CPU_BIG_ENDIAN which is guaranteed to be set.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/bcmsysport.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index a7e84292af50..931751e4f369 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -1762,10 +1762,14 @@ static void rbuf_init(struct bcm_sysport_priv *priv)
 	reg = rbuf_readl(priv, RBUF_CONTROL);
 	reg |= RBUF_4B_ALGN | RBUF_RSB_EN;
 	/* Set a correct RSB format on SYSTEMPORT Lite */
-	if (priv->is_lite) {
+	if (priv->is_lite)
 		reg &= ~RBUF_RSB_SWAP1;
+
+	/* Set a correct RSB format based on host endian */
+	if (!IS_ENABLED(CONFIG_CPU_BIG_ENDIAN))
 		reg |= RBUF_RSB_SWAP0;
-	}
+	else
+		reg &= ~RBUF_RSB_SWAP0;
 	rbuf_writel(priv, reg, RBUF_CONTROL);
 }
 
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v2 2/4] net: dsa: bcm_sf2: Use correct I/O accessors
From: Florian Fainelli @ 2017-08-29 20:35 UTC (permalink / raw)
  To: netdev; +Cc: davem, opendmb, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <1504038918-49254-1-git-send-email-f.fainelli@gmail.com>

The Starfigther 2 driver currently uses __raw_{read,write}l which means
native I/O endian. This works correctly for an ARM LE kernel (default)
but fails miserably on an ARM BE (BE8) kernel where registers are kept
little endian, so replace uses with {read,write}l_relaxed here which is
what we want because this is all performance sensitive code.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/dsa/bcm_sf2.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/dsa/bcm_sf2.h b/drivers/net/dsa/bcm_sf2.h
index 7d3030e04f11..d9c96b281fc0 100644
--- a/drivers/net/dsa/bcm_sf2.h
+++ b/drivers/net/dsa/bcm_sf2.h
@@ -130,12 +130,12 @@ static inline u32 bcm_sf2_mangle_addr(struct bcm_sf2_priv *priv, u32 off)
 #define SF2_IO_MACRO(name) \
 static inline u32 name##_readl(struct bcm_sf2_priv *priv, u32 off)	\
 {									\
-	return __raw_readl(priv->name + off);				\
+	return readl_relaxed(priv->name + off);				\
 }									\
 static inline void name##_writel(struct bcm_sf2_priv *priv,		\
 				  u32 val, u32 off)			\
 {									\
-	__raw_writel(val, priv->name + off);				\
+	writel_relaxed(val, priv->name + off);				\
 }									\
 
 /* Accesses to 64-bits register requires us to latch the hi/lo pairs
@@ -179,23 +179,23 @@ static inline u32 bcm_sf2_mangle_addr(struct bcm_sf2_priv *priv, u32 off)
 static inline u32 core_readl(struct bcm_sf2_priv *priv, u32 off)
 {
 	u32 tmp = bcm_sf2_mangle_addr(priv, off);
-	return __raw_readl(priv->core + tmp);
+	return readl_relaxed(priv->core + tmp);
 }
 
 static inline void core_writel(struct bcm_sf2_priv *priv, u32 val, u32 off)
 {
 	u32 tmp = bcm_sf2_mangle_addr(priv, off);
-	__raw_writel(val, priv->core + tmp);
+	writel_relaxed(val, priv->core + tmp);
 }
 
 static inline u32 reg_readl(struct bcm_sf2_priv *priv, u16 off)
 {
-	return __raw_readl(priv->reg + priv->reg_offsets[off]);
+	return readl_relaxed(priv->reg + priv->reg_offsets[off]);
 }
 
 static inline void reg_writel(struct bcm_sf2_priv *priv, u32 val, u16 off)
 {
-	__raw_writel(val, priv->reg + priv->reg_offsets[off]);
+	writel_relaxed(val, priv->reg + priv->reg_offsets[off]);
 }
 
 SF2_IO64_MACRO(core);
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v2 1/4] net: systemport: Use correct I/O accessors
From: Florian Fainelli @ 2017-08-29 20:35 UTC (permalink / raw)
  To: netdev; +Cc: davem, opendmb, andrew, vivien.didelot, Florian Fainelli
In-Reply-To: <1504038918-49254-1-git-send-email-f.fainelli@gmail.com>

The SYSTEMPORT driver currently uses __raw_{read,write}l which means
native I/O endian. This works correctly for an ARM LE kernel (default)
but fails miserably on an ARM BE (BE8) kernel where registers are kept
little endian, so replace uses with {read,write}l_relaxed here which is
what we want because this is all performance sensitive code.

Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 drivers/net/ethernet/broadcom/bcmsysport.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/bcmsysport.c b/drivers/net/ethernet/broadcom/bcmsysport.c
index b3a21418f511..a7e84292af50 100644
--- a/drivers/net/ethernet/broadcom/bcmsysport.c
+++ b/drivers/net/ethernet/broadcom/bcmsysport.c
@@ -32,13 +32,13 @@
 #define BCM_SYSPORT_IO_MACRO(name, offset) \
 static inline u32 name##_readl(struct bcm_sysport_priv *priv, u32 off)	\
 {									\
-	u32 reg = __raw_readl(priv->base + offset + off);		\
+	u32 reg = readl_relaxed(priv->base + offset + off);		\
 	return reg;							\
 }									\
 static inline void name##_writel(struct bcm_sysport_priv *priv,		\
 				  u32 val, u32 off)			\
 {									\
-	__raw_writel(val, priv->base + offset + off);			\
+	writel_relaxed(val, priv->base + offset + off);			\
 }									\
 
 BCM_SYSPORT_IO_MACRO(intrl2_0, SYS_PORT_INTRL2_0_OFFSET);
@@ -59,14 +59,14 @@ static inline u32 rdma_readl(struct bcm_sysport_priv *priv, u32 off)
 {
 	if (priv->is_lite && off >= RDMA_STATUS)
 		off += 4;
-	return __raw_readl(priv->base + SYS_PORT_RDMA_OFFSET + off);
+	return readl_relaxed(priv->base + SYS_PORT_RDMA_OFFSET + off);
 }
 
 static inline void rdma_writel(struct bcm_sysport_priv *priv, u32 val, u32 off)
 {
 	if (priv->is_lite && off >= RDMA_STATUS)
 		off += 4;
-	__raw_writel(val, priv->base + SYS_PORT_RDMA_OFFSET + off);
+	writel_relaxed(val, priv->base + SYS_PORT_RDMA_OFFSET + off);
 }
 
 static inline u32 tdma_control_bit(struct bcm_sysport_priv *priv, u32 bit)
@@ -110,10 +110,10 @@ static inline void dma_desc_set_addr(struct bcm_sysport_priv *priv,
 				     dma_addr_t addr)
 {
 #ifdef CONFIG_PHYS_ADDR_T_64BIT
-	__raw_writel(upper_32_bits(addr) & DESC_ADDR_HI_MASK,
+	writel_relaxed(upper_32_bits(addr) & DESC_ADDR_HI_MASK,
 		     d + DESC_ADDR_HI_STATUS_LEN);
 #endif
-	__raw_writel(lower_32_bits(addr), d + DESC_ADDR_LO);
+	writel_relaxed(lower_32_bits(addr), d + DESC_ADDR_LO);
 }
 
 static inline void tdma_port_write_desc_addr(struct bcm_sysport_priv *priv,
-- 
1.9.1

^ permalink raw reply related

* [PATCH net-next v2 0/4] Endian fixes for SYSTEMPORT/SF2/MDIO
From: Florian Fainelli @ 2017-08-29 20:35 UTC (permalink / raw)
  To: netdev; +Cc: davem, opendmb, andrew, vivien.didelot, Florian Fainelli

Hi David,

While trying an ARM BE kernel for kinks, the 3 drivers below started not
working and the reasons why became pretty obvious because the register space
remains LE (hardwired), except for Broadcom MIPS where it follows the CPU's
native endian (let's call that a feature).

Thanks!

Changes in v2:

- correctly set RSB_SWAP1 and RSB_SWAP0 for all combinations, now properly
  tested on SYSTEMPORT (BCM7445), SYSTEMPORT Lite (BCM7278) under both endian

Florian Fainelli (4):
  net: systemport: Use correct I/O accessors
  net: dsa: bcm_sf2: Use correct I/O accessors
  net: systemport: Set correct RSB endian bits based on host
  net: phy: mdio-bcm-unimac: Use correct I/O accessors

 drivers/net/dsa/bcm_sf2.h                  | 12 +++++------
 drivers/net/ethernet/broadcom/bcmsysport.c | 20 +++++++++++--------
 drivers/net/phy/mdio-bcm-unimac.c          | 32 ++++++++++++++++++++++++------
 3 files changed, 44 insertions(+), 20 deletions(-)

-- 
1.9.1

^ permalink raw reply

* Re: [PATCH net-next] virtio-net: invoke zerocopy callback on xmit path if no tx napi
From: Michael S. Tsirkin @ 2017-08-29 20:40 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Koichiro Den, Jason Wang, virtualization, Network Development
In-Reply-To: <CAF=yD-L5MG5RCZzT8EZ0coFiuQLp-2UQN0zzQefVnEd7rMQbyw@mail.gmail.com>

On Tue, Aug 29, 2017 at 03:53:08PM -0400, Willem de Bruijn wrote:
> On Tue, Aug 29, 2017 at 3:42 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Tue, Aug 29, 2017 at 03:35:38PM -0400, Willem de Bruijn wrote:
> >> By the way, I have had an unrelated patch outstanding for a while
> >> to have virtio-net support the VIRTIO_CONFIG_S_NEEDS_RESET
> >> command. Will send that as RFC.
> >
> > Oh nice.
> 
> Great :)
> 
> > One needs to be careful about locking there which is why
> > no devices support that yet.
> 
> I originally wrote it based on the virtnet_reset function introduced
> for xdp. Calling this from virtnet_config_changed_work is non trivial,
> as virtnet_freeze_down waits until no config worker is running.
> 
> Otherwise, I could not find any constraints on when freeze may be
> called, and it largely follows the same path. I hope I didn't miss anything.

The issue is that on freeze processes are not running so we
generally know no new packets will arrive (might be wrong
for bridging, then it's a bug). On device error you must
prevent new skbs from coming in, etc.

-- 
MST

^ permalink raw reply

* Re: [PATCH RFC 2/2] virtio_net: enable probing for NEEDS_RESET support
From: Michael S. Tsirkin @ 2017-08-29 20:38 UTC (permalink / raw)
  To: Willem de Bruijn
  Cc: Network Development, Jason Wang, virtualization, Willem de Bruijn
In-Reply-To: <CAF=yD-L95x1EPKFeiPiZ2hbTzwYmDaG9O54+0tUW_4opuRKDMw@mail.gmail.com>

On Tue, Aug 29, 2017 at 04:27:41PM -0400, Willem de Bruijn wrote:
> On Tue, Aug 29, 2017 at 4:16 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> > On Tue, Aug 29, 2017 at 04:07:59PM -0400, Willem de Bruijn wrote:
> >> From: Willem de Bruijn <willemb@google.com>
> >>
> >> Implement a mechanism to signal that a virtio device implements the
> >> VIRTIO_CONFIG_S_NEEDS_RESET command.
> >>
> >> Testing for VIRTIO_CONFIG_S_NEEDS_RESET support by issuing the request
> >> and verifying the reset state would require an expensive state change.
> >>
> >> To avoid that, add a status bit to the feature register used during
> >> feature negotiation between host and guest.
> >>
> >> Set the bit for virtio-net, which supports the feature.
> >>
> >> Signed-off-by: Willem de Bruijn <willemb@google.com>
> >
> > All virtio 1 devices have the reset feature
> 
> I don't quite follow. No device drivers implement this request currently?

Depends. Spec 1.0 describes the bit and that driver can respond
by reseting the device. You seem to do something else
in this patchset, but as designed in 1.0 it does not seem to need
a feature bit.

> > so maybe guest does
> > not need this flag. Does device need it? Does device really
> > care that guest can't recover?
> 
> If all device drivers support it, then the flag is not needed.
> 
> But as long as legacy device drivers can exist that do not support
> this feature, it has to have a way of differentiating between the two.

Why? Device won't set this unless it's in a bad state. In that case,
setting the bit is harmless even if drivers ignore it.

-- 
MST

^ permalink raw reply

* Re: [PATCH net-next v3] bridge: fdb add and delete tracepoints
From: Nikolay Aleksandrov @ 2017-08-29 20:35 UTC (permalink / raw)
  To: Roopa Prabhu, davem; +Cc: netdev, f.fainelli, bridge
In-Reply-To: <1504037817-38107-1-git-send-email-roopa@cumulusnetworks.com>

On 29/08/17 23:16, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> 
> A few useful tracepoints to trace bridge forwarding
> database updates.
> 
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> ---
> v2: address comments from florian
> v3: remove stray character '=' in print (pointed out by florian)
> 
>  include/trace/events/bridge.h |   98 +++++++++++++++++++++++++++++++++++++++++
>  net/bridge/br_fdb.c           |    7 +++
>  net/core/net-traces.c         |    6 +++
>  3 files changed, 111 insertions(+)
>  create mode 100644 include/trace/events/bridge.h
> 

Acked-by: Nikolay Aleksandrov <nikolay@cumulusnetworks.com>

^ permalink raw reply

* Re: [PATCH] Documentation: fix little inconsistencies
From: Pavel Machek @ 2017-08-29 20:34 UTC (permalink / raw)
  To: Jonathan Corbet, davem, netdev
  Cc: Darrick J. Wong, linux-kernel, linux-doc, tytso, linux-ext4,
	Andrew Morton
In-Reply-To: <20170829130945.7603d035@lwn.net>

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

On Tue 2017-08-29 13:09:45, Jonathan Corbet wrote:
> On Tue, 29 Aug 2017 09:50:57 -0700
> "Darrick J. Wong" <darrick.wong@oracle.com> wrote:
> 
> > > Anything wrong here? It is fixing extra '+' in the ascii art...  
> > 
> > There's nothing incorrect here, I merely thought it odd to send a fix
> > for networking documentation to the ext4 list, but not netdev?
> 
> In fact, davem likes to handle networking documentation patches himself,
> so resending this one to netdev would be a good idea.

Its a typo in documentation, do we really need to waste any more time
with it? Dave, can you just ack it?

> > index 5e40e1f..6309e90 100644
> > --- a/Documentation/networking/switchdev.txt
> > +++ b/Documentation/networking/switchdev.txt
> > @@ -29,7 +29,7 @@ with SR-IOV or soft switches, such as OVS, are
> > possible.
> >                        sw1p1  +  sw1p3  +  sw1p5  +          eth1
> >                          +    |    +    |    +    |            +
> >                          |    |    |    |    |    |            |
> > -                     +--+----+----+----+-+--+----+---+  +-----+-----+
> > +                     +--+----+----+----+----+----+---+  +-----+-----+

Besides, if documentation fixes should go to Davem/netdev,
getmaintainer.pl should say so:

pavel@duo:/data/l/linux-n900$ scripts/get_maintainer.pl -f
Documentation/networking
Jonathan Corbet <corbet@lwn.net> (maintainer:DOCUMENTATION)
"David S. Miller" <davem@davemloft.net> (commit_signer:52/87=60%)
David Howells <dhowells@redhat.com>
(commit_signer:7/87=8%,authored:8/87=9%)
Florian Fainelli <f.fainelli@gmail.com> (commit_signer:6/87=7%)
Mauro Carvalho Chehab <mchehab@kernel.org>
(commit_signer:6/87=7%,authored:5/87=6%)
Yuchung Cheng <ycheng@google.com> (commit_signer:6/87=7%)
Hangbin Liu <liuhangbin@gmail.com> (authored:5/87=6%)
linux-doc@vger.kernel.org (open list:DOCUMENTATION)
linux-kernel@vger.kernel.org (open list)
pavel@duo:/data/l/linux-n900$

									
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]

^ permalink raw reply

* Re: [PATCH net-next v3] bridge: fdb add and delete tracepoints
From: Florian Fainelli @ 2017-08-29 20:30 UTC (permalink / raw)
  To: Roopa Prabhu, davem; +Cc: nikolay, netdev, bridge
In-Reply-To: <1504037817-38107-1-git-send-email-roopa@cumulusnetworks.com>

On 08/29/2017 01:16 PM, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> 
> A few useful tracepoints to trace bridge forwarding
> database updates.
> 
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>

Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>

thanks for quick turnaround!
-- 
Florian

^ permalink raw reply

* Re: [PATCH RFC 2/2] virtio_net: enable probing for NEEDS_RESET support
From: Willem de Bruijn @ 2017-08-29 20:27 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Network Development, Jason Wang, virtualization, Willem de Bruijn
In-Reply-To: <20170829231606-mutt-send-email-mst@kernel.org>

On Tue, Aug 29, 2017 at 4:16 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> On Tue, Aug 29, 2017 at 04:07:59PM -0400, Willem de Bruijn wrote:
>> From: Willem de Bruijn <willemb@google.com>
>>
>> Implement a mechanism to signal that a virtio device implements the
>> VIRTIO_CONFIG_S_NEEDS_RESET command.
>>
>> Testing for VIRTIO_CONFIG_S_NEEDS_RESET support by issuing the request
>> and verifying the reset state would require an expensive state change.
>>
>> To avoid that, add a status bit to the feature register used during
>> feature negotiation between host and guest.
>>
>> Set the bit for virtio-net, which supports the feature.
>>
>> Signed-off-by: Willem de Bruijn <willemb@google.com>
>
> All virtio 1 devices have the reset feature

I don't quite follow. No device drivers implement this request currently?

> so maybe guest does
> not need this flag. Does device need it? Does device really
> care that guest can't recover?

If all device drivers support it, then the flag is not needed.

But as long as legacy device drivers can exist that do not support
this feature, it has to have a way of differentiating between the two.

^ permalink raw reply

* Re: [PATCH net-next v2 00/10] net: dsa: add generic debugfs interface
From: Andrew Lunn @ 2017-08-29 20:27 UTC (permalink / raw)
  To: Florian Fainelli
  Cc: Arkadi Sharshevsky, Jiri Pirko, Vivien Didelot, netdev,
	linux-kernel, kernel, David S. Miller, Egil Hjelmeland,
	John Crispin, Woojung Huh, Sean Wang, Nikita Yushchenko,
	Chris Healy, mlxsw
In-Reply-To: <9dd804f2-8d40-2a76-e692-a60b10182834@gmail.com>

On Tue, Aug 29, 2017 at 12:19:08PM -0700, Florian Fainelli wrote:
> On 08/29/2017 12:05 PM, Arkadi Sharshevsky wrote:
> > 
> > 
> > On 08/29/2017 03:50 PM, Andrew Lunn wrote:
> >> On Tue, Aug 29, 2017 at 08:25:23AM +0200, Jiri Pirko wrote:
> >>> Mon, Aug 28, 2017 at 10:08:34PM CEST, andrew@lunn.ch wrote:
> >>>>> I see this overlaps a lot with DPIPE. Why won't you use that to expose
> >>>>> your hw state?
> >>>>
> >>>> We took a look at dpipe and i talked to you about using it for this
> >>>> sort of thing at netconf/netdev. But dpipe has issues displaying the
> >>>> sort of information we have. I never figured out how to do two
> >>>> dimensional tables. The output of the dpipe command is pretty
> >>>> unreadable. A lot of the information being dumped here is not about
> >>>> the data pipe, etc.
> >>>
> >>> So improve it. No problem. Also, we extend it to support what you neede.
> >>
> >> Will i did try to do this back in March. And i failed.
> >>
> >> Lets start with stats. Vivien gives an example on the cover letter:
> >>
> >>     # pr -mt switch0/port{5,6}/stats
> >>     in_good_octets      : 0             in_good_octets      : 13824
> >>     in_bad_octets       : 0             in_bad_octets       : 0
> >>     in_unicast          : 0             in_unicast          : 0
> >>     in_broadcasts       : 0             in_broadcasts       : 216
> >>     in_multicasts       : 0             in_multicasts       : 0
> >>     in_pause            : 0             in_pause            : 0
> >>     in_undersize        : 0             in_undersize        : 0
> >>
> >> This is what i tried to implement using dpipe. It is a simple two
> >> dimensional table. First column is a string, second a u64. In debugfs
> >> we have such a table per port. That fits with the hierarchy that each
> >> port is a directory in debugfs. But it could also be implemented as
> >> one table with N+1 columns, for N switch ports.
> >>
> > 
> > Hi Andrew,
> > 
> > This looks to me like basic L2 statistics that are obtained via
> > ethtool, I remember you had this problem with the DSA and CPU port.
> > Is this still the case?
> 
> Yes, there are no net_device representors for CPU and DSA ports because
> if we did that, it would be confusing as we would be creating two
> network devices for both ends of the pipe. For DSA (inter-switch)
> interfaces you would have one "dsa" netdev for each adjacent switch so
> two DSA interface represent the inter switch link.
> 
> For the "CPU" port, you have the master network device (e.g: eth0) and
> the "cpu" network device, this is confusing. "cpu" is not usable, since
> it does not make sense for the "cpu" to send traffic via this interface,
> the model is to terminate user-facing ports and use a tag to deliver
> packets to the right interfaces. For "dsa" it's pretty much the same story.

The point of the story is that ethtool does not cover this use
case. We need a different way to expose these statistics for
debugging, and ideally the statistics for all the ports, not just DSA
and CPU.

> > I remembered we wanted to use dpipe for the DSA routing table
> > and IP priority table.

No, we wanted to use dpipe as a generic mechanism to get debug tables
out of the switch. The DSA routing table and the IP priority tables
could be candidates sometime in the future. But since most switches
don't actually have these, we are not so interested in them at the
moment. We are concentrating on tables that all DSA switches are
likely to have. Stuff we can implement once, and it works for all DSA
switches.

> > I think both those processes really look like match/action table
> > , thus they can be modeled successfully by dpipe.

And this is probably the core of the problem with dpipe. Very little
in an average switch is a match/action. We need a generic table. The
table is well specified, in that i can tell you the types of the
columns. We know the number of columns in the table at runtime, but
maybe not the number of rows until we reach the end of the table.  And
ideally, we don't want to have to change the user space tool every
time we add a new table. The type info and the number of columns
should be enough for the user space tool to print it.

       Andrew

^ permalink raw reply

* Re: [PATCH RFC 1/2] virtio_net: implement VIRTIO_CONFIG_S_NEEDS_RESET
From: Michael S. Tsirkin @ 2017-08-29 20:20 UTC (permalink / raw)
  To: Willem de Bruijn; +Cc: netdev, jasowang, virtualization, Willem de Bruijn
In-Reply-To: <20170829200759.13975-2-willemdebruijn.kernel@gmail.com>

On Tue, Aug 29, 2017 at 04:07:58PM -0400, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> Implement the reset communication request defined in the VIRTIO 1.0
> specification and introduces in Linux in commit c00bbcf862896 ("virtio:
> add VIRTIO_CONFIG_S_NEEDS_RESET device status bit").
> 
> Since that patch, the virtio-net driver has added a virtnet_reset
> function that implements the requested behavior through calls to the
> power management freeze and restore functions.
> 
> That function has recently been reverted when its sole caller was
> updated. Bring it back and listen for the request from the host on
> the config channel.
> 
> Implement the feature analogous to other config requests. In
> particular, acknowledge the request in the same manner as the
> NET_S_ANNOUNCE link announce request, by responding with a new
> VIRTIO_NET_CTRL_${TYPE} command. On reception, the host must check
> the config status register for success or failure.


Pls make it clearer why do you need these interface extensions.

> The existing freeze handler verifies that no config changes are
> running concurrently. Elide this check for reset. The request is
> always handled from the config workqueue. No other config requests
> can be active or scheduled concurrently on vi->config.

You need to prevent packet TX from being in progress.

> 
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> ---
>  drivers/net/virtio_net.c        | 69 +++++++++++++++++++++++++++++++++++------
>  include/uapi/linux/virtio_net.h |  4 +++

virtio dev or another virtio TC list must be Cc'd on any proposed API changes.


>  2 files changed, 64 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 52ae78ca3d38..5e349226f7c1 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1458,12 +1458,11 @@ static void virtnet_netpoll(struct net_device *dev)
>  }
>  #endif
>  
> -static void virtnet_ack_link_announce(struct virtnet_info *vi)
> +static void virtnet_ack(struct virtnet_info *vi, u8 class, u8 cmd)
>  {
>  	rtnl_lock();
> -	if (!virtnet_send_command(vi, VIRTIO_NET_CTRL_ANNOUNCE,
> -				  VIRTIO_NET_CTRL_ANNOUNCE_ACK, NULL))
> -		dev_warn(&vi->dev->dev, "Failed to ack link announce.\n");
> +	if (!virtnet_send_command(vi, class, cmd, NULL))
> +		dev_warn(&vi->dev->dev, "Failed to ack %u.%u\n", class, cmd);
>  	rtnl_unlock();
>  }
>  
> @@ -1857,13 +1856,16 @@ static const struct ethtool_ops virtnet_ethtool_ops = {
>  	.set_link_ksettings = virtnet_set_link_ksettings,
>  };
>  
> -static void virtnet_freeze_down(struct virtio_device *vdev)
> +static void virtnet_freeze_down(struct virtio_device *vdev, bool in_reset)
>  {
>  	struct virtnet_info *vi = vdev->priv;
>  	int i;
>  
> -	/* Make sure no work handler is accessing the device */
> -	flush_work(&vi->config_work);
> +	/* Make sure no work handler is accessing the device,
> +	 * unless this call is made from the reset work handler itself.
> +	 */
> +	if (!in_reset)
> +		flush_work(&vi->config_work);
>  
>  	netif_device_detach(vi->dev);
>  	netif_tx_disable(vi->dev);
> @@ -1878,6 +1880,7 @@ static void virtnet_freeze_down(struct virtio_device *vdev)
>  }
>  
>  static int init_vqs(struct virtnet_info *vi);
> +static void remove_vq_common(struct virtnet_info *vi);
>  
>  static int virtnet_restore_up(struct virtio_device *vdev)
>  {
> @@ -1906,6 +1909,45 @@ static int virtnet_restore_up(struct virtio_device *vdev)
>  	return err;
>  }
>  
> +static int virtnet_reset(struct virtnet_info *vi)
> +{
> +	struct virtio_device *dev = vi->vdev;
> +	bool failed;
> +	int ret;
> +
> +	virtio_config_disable(dev);
> +	failed = dev->config->get_status(dev) & VIRTIO_CONFIG_S_FAILED;
> +	virtnet_freeze_down(dev, true);
> +	remove_vq_common(vi);
> +
> +	virtio_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE);
> +	virtio_add_status(dev, VIRTIO_CONFIG_S_DRIVER);
> +
> +	/* Restore the failed status (see virtio_device_restore). */
> +	if (failed)
> +		virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
> +
> +	ret = virtio_finalize_features(dev);
> +	if (ret)
> +		goto err;
> +
> +	ret = virtnet_restore_up(dev);
> +	if (ret)
> +		goto err;
> +
> +	ret = virtnet_set_queues(vi, vi->curr_queue_pairs);
> +	if (ret)
> +		goto err;
> +
> +	virtio_add_status(dev, VIRTIO_CONFIG_S_DRIVER_OK);
> +	virtio_config_enable(dev);
> +	return 0;
> +
> +err:
> +	virtio_add_status(dev, VIRTIO_CONFIG_S_FAILED);
> +	return ret;
> +}
> +
>  static int virtnet_set_guest_offloads(struct virtnet_info *vi, u64 offloads)
>  {
>  	struct scatterlist sg;
> @@ -2085,7 +2127,16 @@ static void virtnet_config_changed_work(struct work_struct *work)
>  
>  	if (v & VIRTIO_NET_S_ANNOUNCE) {
>  		netdev_notify_peers(vi->dev);
> -		virtnet_ack_link_announce(vi);
> +		virtnet_ack(vi, VIRTIO_NET_CTRL_ANNOUNCE,
> +			    VIRTIO_NET_CTRL_ANNOUNCE_ACK);
> +	}
> +
> +	if (vi->vdev->config->get_status(vi->vdev) &
> +	    VIRTIO_CONFIG_S_NEEDS_RESET) {
> +		virtnet_reset(vi);
> +		virtnet_ack(vi, VIRTIO_NET_CTRL_RESET,
> +			    VIRTIO_NET_CTRL_RESET_ACK);
> +
>  	}
>  
>  	/* Ignore unknown (future) status bits */
> @@ -2708,7 +2759,7 @@ static __maybe_unused int virtnet_freeze(struct virtio_device *vdev)
>  	struct virtnet_info *vi = vdev->priv;
>  
>  	virtnet_cpu_notif_remove(vi);
> -	virtnet_freeze_down(vdev);
> +	virtnet_freeze_down(vdev, false);
>  	remove_vq_common(vi);
>  
>  	return 0;
> diff --git a/include/uapi/linux/virtio_net.h b/include/uapi/linux/virtio_net.h
> index fc353b518288..188fdc528f13 100644
> --- a/include/uapi/linux/virtio_net.h
> +++ b/include/uapi/linux/virtio_net.h
> @@ -245,4 +245,8 @@ struct virtio_net_ctrl_mq {
>  #define VIRTIO_NET_CTRL_GUEST_OFFLOADS   5
>  #define VIRTIO_NET_CTRL_GUEST_OFFLOADS_SET        0
>  
> +/* Signal that the driver received and executed the reset command. */
> +#define VIRTIO_NET_CTRL_RESET			6
> +#define VIRTIO_NET_CTRL_RESET_ACK		0
> +
>  #endif /* _UAPI_LINUX_VIRTIO_NET_H */
> -- 
> 2.14.1.342.g6490525c54-goog

^ permalink raw reply

* Re: [PATCH net-next 0/4] Endian fixes for SYSTEMPORT/SF2/MDIO
From: Florian Fainelli @ 2017-08-29 20:17 UTC (permalink / raw)
  To: netdev; +Cc: davem, opendmb, andrew, vivien.didelot
In-Reply-To: <1504031985-52808-1-git-send-email-f.fainelli@gmail.com>

On 08/29/2017 11:39 AM, Florian Fainelli wrote:
> Hi David,
> 
> While trying an ARM BE kernel for kinks, the 3 drivers below started not
> working and the reasons why became pretty obvious because the register space
> remains LE (hardwired), except for Broadcom MIPS where it follows the CPU's
> native endian (let's call that a feature).

David, I found a minor problem in the RSB endian configuration for
SYSTEMPOR Lite that uses two bits for that, I will repost shortly.

> 
> Thanks!
> 
> Florian Fainelli (4):
>   net: systemport: Use correct I/O accessors
>   net: dsa: bcm_sf2: Use correct I/O accessors
>   net: systemport: Set correct RSB endian bits based on host
>   net: phy: mdio-bcm-unimac: Use correct I/O accessors
> 
>  drivers/net/dsa/bcm_sf2.h                  | 12 +++++------
>  drivers/net/ethernet/broadcom/bcmsysport.c | 21 ++++++++++++--------
>  drivers/net/phy/mdio-bcm-unimac.c          | 32 ++++++++++++++++++++++++------
>  3 files changed, 45 insertions(+), 20 deletions(-)
> 


-- 
Florian

^ 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