Re: [PATCH] netfilter: nat: constify rhashtable_params

[Pablo Neira Ayuso]

On Wed, Aug 30, 2017 at 05:18:04PM +0530, Arvind Yadav wrote:
> rhashtable_params are not supposed to change at runtime. All
> Functions rhashtable_* working with const rhashtable_params
> provided by . So mark the non-const structs
> as const.

Applied to nf, thanks.

Re: [PATCH v2] netfilter: xt_hashlimit: fix build error caused by 64bit division

[Pablo Neira Ayuso]

On Fri, Sep 08, 2017 at 01:38:58AM -0400, Vishwanath Pai wrote:
> 64bit division causes build/link errors on 32bit architectures. It
> prints out error messages like:
> 
> ERROR: "__aeabi_uldivmod" [net/netfilter/xt_hashlimit.ko] undefined!
> 
> The value of avg passed through by userspace in BYTE mode cannot exceed
> U32_MAX. Which means 64bit division in user2rate_bytes is unnecessary.
> To fix this I have changed the type of param 'user' to u32.
> 
> Since anything greater than U32_MAX is an invalid input we error out in
> hashlimit_mt_check_common() when this is the case.
> 
> Changes in v2:
>   Making return type as u32 would cause an overflow for small
>   values of 'user' (for example 2, 3 etc). To avoid this I bumped up
>   'r' to u64 again as well as the return type. This is OK since the
>   variable that stores the result is u64. We still avoid 64bit
>   division here since 'user' is u32.

Applied, thanks.

Re: [PATCH 2/2] netfilter/libxt_hashlimit: new feature/algorithm for xt_hashlimit

[Pablo Neira Ayuso]

On Fri, Aug 18, 2017 at 04:59:06PM -0400, Vishwanath Pai wrote:
> This patch adds a new feature to hashlimit that allows matching on the
> current packet/byte rate without rate limiting. This can be enabled
> with a new flag --hashlimit-rate-match. The match returns true if the
> current rate of packets is above/below the user specified value.

Applied to iptables:

http://git.netfilter.org/iptables/commit/?id=1c32e5606fdf53856cba0cd9bc7b3f8b584b2cc2

Thanks.

Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

[Andrew Lunn]

> > > +static int mdio_mux_syscon_switch_fn(int current_child, int 
> > > desired_child,
> > > +  void *data)
> > > +{
> > > + struct stmmac_priv *priv = data;
> > > + struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
> > > + u32 reg, val;
> > > + int ret = 0;
> > > + bool need_reset = false;
> > > +
> > > + if (current_child ^ desired_child) {
> > > + regmap_read(gmac->regmap, SYSCON_EMAC_REG, );
> > > + switch (desired_child) {
> > > + case DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID:
> > > + dev_info(priv->device, "Switch mux to internal PHY");
> > > + val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SELECT;
> > > + if (gmac->use_internal_phy)
> > > + need_reset = true;
> > > + break;
> > 
> > This i don't get. Why do you need use_internal_phy? Isn't that
> > implicit from DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID? Is it even possible to
> > use an external PHY on the internal MDIO bus?
> > 
> 
> On my H3 box with external PHY, the MDIO mux library first select (for scan 
> ?) the internal MDIO.
> Without use_internal_phy usage, this board will launch a reset to use the 
> internal MDIO... and this reset timeout/fail.

Do you know why the reset times out/fails?

   Andrew

Re: [PATCH iproute2 1/2] lib/libnetlink: re malloc buff if size is not enough

[Phil Sutter]

Hi Hangbin,

On Fri, Sep 08, 2017 at 06:14:56PM +0800, Hangbin Liu wrote:
[...]
> diff --git a/lib/libnetlink.c b/lib/libnetlink.c
> index be7ac86..37cfb5a 100644
> --- a/lib/libnetlink.c
> +++ b/lib/libnetlink.c
> @@ -402,6 +402,59 @@ static void rtnl_dump_error(const struct rtnl_handle 
> *rth,
>   }
>  }
>  
> +static int rtnl_recvmsg(int fd, struct msghdr *msg, char **buf)
> +{
> + struct iovec *iov;
> + int len = -1, buf_len = 32768;
> + char *buffer = *buf;

Isn't it possible to make 'buffer' static instead of the two 'buf'
variables in rtnl_dump_filter_l() and __rtnl_talk()? Then we would have
only a single buffer which is shared between both functions instead of
two which are independently allocated.

> +
> + int flag = MSG_PEEK | MSG_TRUNC;
> +
> + if (buffer == NULL)
> +re_malloc:
> + buffer = malloc(buf_len);

I think using realloc() here is more appropriate since there is no need
to free the buffer in beforehand and calling realloc(NULL, len) is
equivalent to calling malloc(len). I think 'realloc' is also a better
name for the goto label.

> + if (buffer == NULL) {
> + fprintf(stderr, "malloc error: no enough buffer\n");

Minor typo here: s/no/not/

> + return -1;

Return -ENOMEM?

> + }
> +
> + iov = msg->msg_iov;
> + iov->iov_base = buffer;
> + iov->iov_len = buf_len;
> +
> +re_recv:

Just call this 'recv'? (Not really important though.)

> + len = recvmsg(fd, msg, flag);
> +
> + if (len < 0) {
> + if (errno == EINTR || errno == EAGAIN)
> + return 0;

Instead of returning 0 (which makes callers retry), goto re_recv?

> + fprintf(stderr, "netlink receive error %s (%d)\n",
> + strerror(errno), errno);
> + return len;
> + }
> +
> + if (len == 0) {
> + fprintf(stderr, "EOF on netlink\n");
> + return -1;

Return -ENODATA here? (Initially I though about -EOF, but EOF is -1 so
that would be incorrect).

> + }
> +
> + if (len > buf_len) {
> + free(buffer);

If you use realloc() above, this can be dropped.

> + buf_len = len;

For this to work you have to make buf_len static too, otherwise you will
unnecessarily reallocate the buffer. Oh, and that also requires the
single buffer (as pointed out above) because you will otherwise use a
common buf_len for both static buffers passed to this function.

> + flag = 0;
> + goto re_malloc;
> + }
> +
> + if (flag != 0) {
> + flag = 0;
> + goto re_recv;
> + }
> +
> + *buf = buffer;
> + return len;
> +}
> +
>  int rtnl_dump_filter_l(struct rtnl_handle *rth,
>  const struct rtnl_dump_filter_arg *arg)
>  {
> @@ -413,31 +466,20 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
>   .msg_iov = ,
>   .msg_iovlen = 1,
>   };
> - char buf[32768];
> + static char *buf = NULL;

If you keep the static buffer in rtnl_recvmsg(), there is no need to
assign NULL here.

>   int dump_intr = 0;
>  
> - iov.iov_base = buf;
>   while (1) {
>   int status;
>   const struct rtnl_dump_filter_arg *a;
>   int found_done = 0;
>   int msglen = 0;
>  
> - iov.iov_len = sizeof(buf);
> - status = recvmsg(rth->fd, , 0);
> -
> - if (status < 0) {
> - if (errno == EINTR || errno == EAGAIN)
> - continue;
> - fprintf(stderr, "netlink receive error %s (%d)\n",
> - strerror(errno), errno);
> - return -1;
> - }
> -
> - if (status == 0) {
> - fprintf(stderr, "EOF on netlink\n");
> - return -1;
> - }
> + status = rtnl_recvmsg(rth->fd, , );
> + if (status < 0)
> + return status;
> + else if (status == 0)
> + continue;

When retrying inside rtnl_recvmsg(), it won't return 0 anymore. I
believe the whole 'while (1)' loop could go away then.

Everything noted for rtnl_dump_filter_l() applies to __rtnl_talk() as
well.

Thanks, Phil

Re: [PATCH net] bpf: don't select potentially stale ri->map from buggy xdp progs

[Jesper Dangaard Brouer]

On Fri, 08 Sep 2017 12:34:28 +0200 Daniel Borkmann  wrote:

> On 09/08/2017 07:06 AM, Jesper Dangaard Brouer wrote:
> > On Fri,  8 Sep 2017 00:14:51 +0200
> > Daniel Borkmann  wrote:
> >  
> >> +  /* This is really only caused by a deliberately crappy
> >> +   * BPF program, normally we would never hit that case,
> >> +   * so no need to inform someone via tracepoints either,
> >> +   * just bail out.
> >> +   */
> >> +  if (unlikely(map_owner != xdp_prog))
> >> +  return -EINVAL;  
> >
> > IMHO we do need to call the tracepoint here.  It is not just crappy
> > BPF-progs that cause this situation, it is also drivers not implementing
> > XDP_REDIRECT yet (which is all but ixgbe).  Due to the level XDP
> > operates at, tracepoints are the only way users can runtime troubleshoot
> > their XDP programs.  
> 
> Drivers not implementing XDP_REDIRECT don't even get there in
> the first place. What they will do is to hit the 'default' case
> when they check for the action code from the BPF program. Then
> call into bpf_warn_invalid_xdp_action(act), and fall-through
> to hit the tracepoint at trace_xdp_exception() which is also
> triggered by XDP_ABORTED usually. So when that happens we do
> complain loudly and call a tracepoint already. We should probably
> tweak the bpf_warn_invalid_xdp_action() message a little to make
> it clear that the action could also just be unsupported by the
> driver instead of being illegal.

Yes. drivers not implementing XDP_REDIRECT will cause a tracepoint
trace_xdp_exception() to be called for its _own_ packets.

But it will still setup and leave map and map_owner pointer dangling.
Another NIC can load an xdp_prog that return XDP_REDIRECT, which will hit
above if-statement, and its packets will disappear, without getting
recorded by a tracepoint (thus hard to debug!).

The fundamental point is that tracepoints is the way we choose to
handle debugging XDP programs.  Thus, we must trigger a tracepoint when
a packet gets dropped.  Even in this unlikely case.

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

Re: [PATCH iproute2 1/2] lib/libnetlink: re malloc buff if size is not enough

[Michal Kubecek]

On Fri, Sep 08, 2017 at 01:02:47PM +0200, Phil Sutter wrote:
> Hi Hangbin,
> 
> On Fri, Sep 08, 2017 at 06:14:56PM +0800, Hangbin Liu wrote:
> [...]
> > diff --git a/lib/libnetlink.c b/lib/libnetlink.c
> > index be7ac86..37cfb5a 100644
> > --- a/lib/libnetlink.c
> > +++ b/lib/libnetlink.c
> > @@ -402,6 +402,59 @@ static void rtnl_dump_error(const struct rtnl_handle 
> > *rth,
> > }
> >  }
> >  
> > +static int rtnl_recvmsg(int fd, struct msghdr *msg, char **buf)
> > +{
> > +   struct iovec *iov;
> > +   int len = -1, buf_len = 32768;
> > +   char *buffer = *buf;
> 
> Isn't it possible to make 'buffer' static instead of the two 'buf'
> variables in rtnl_dump_filter_l() and __rtnl_talk()? Then we would have
> only a single buffer which is shared between both functions instead of
> two which are independently allocated.

Do we have to worry about reentrancy? Only arpd is multithreaded in
iproute2 but there might be also other users of libnetlink.

> > +
> > +   int flag = MSG_PEEK | MSG_TRUNC;
> > +
> > +   if (buffer == NULL)
> > +re_malloc:
> > +   buffer = malloc(buf_len);
> 
> I think using realloc() here is more appropriate since there is no need
> to free the buffer in beforehand and calling realloc(NULL, len) is
> equivalent to calling malloc(len). I think 'realloc' is also a better
> name for the goto label.
> 
> > +   if (buffer == NULL) {
> > +   fprintf(stderr, "malloc error: no enough buffer\n");
> 
> Minor typo here: s/no/not/
> 
> > +   return -1;
> 
> Return -ENOMEM?

Hm... the only caller of rtnl_dump_filter_l only checks if the return
value is negative. Even worse, at least some of the functions calling
__rtnl_talk() check errno (or use perror()) instead, even if it's not
always preserved. That's something for a wider cleanup, I would say.

> 
> > +   }
> > +
> > +   iov = msg->msg_iov;
> > +   iov->iov_base = buffer;
> > +   iov->iov_len = buf_len;
> > +
> > +re_recv:
> 
> Just call this 'recv'? (Not really important though.)
> 
> > +   len = recvmsg(fd, msg, flag);
> > +
> > +   if (len < 0) {
> > +   if (errno == EINTR || errno == EAGAIN)
> > +   return 0;
> 
> Instead of returning 0 (which makes callers retry), goto re_recv?
> 
> > +   fprintf(stderr, "netlink receive error %s (%d)\n",
> > +   strerror(errno), errno);
> > +   return len;
> > +   }
> > +
> > +   if (len == 0) {
> > +   fprintf(stderr, "EOF on netlink\n");
> > +   return -1;
> 
> Return -ENODATA here? (Initially I though about -EOF, but EOF is -1 so
> that would be incorrect).
> 
> > +   }
> > +
> > +   if (len > buf_len) {
> > +   free(buffer);
> 
> If you use realloc() above, this can be dropped.
> 
> > +   buf_len = len;
> 
> For this to work you have to make buf_len static too, otherwise you will
> unnecessarily reallocate the buffer. Oh, and that also requires the
> single buffer (as pointed out above) because you will otherwise use a
> common buf_len for both static buffers passed to this function.
> 
> > +   flag = 0;
> > +   goto re_malloc;
> > +   }
> > +
> > +   if (flag != 0) {
> > +   flag = 0;
> > +   goto re_recv;
> > +   }
> > +
> > +   *buf = buffer;
> > +   return len;
> > +}
> > +
> >  int rtnl_dump_filter_l(struct rtnl_handle *rth,
> >const struct rtnl_dump_filter_arg *arg)
> >  {
> > @@ -413,31 +466,20 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
> > .msg_iov = ,
> > .msg_iovlen = 1,
> > };
> > -   char buf[32768];
> > +   static char *buf = NULL;
> 
> If you keep the static buffer in rtnl_recvmsg(), there is no need to
> assign NULL here.
> 
> > int dump_intr = 0;
> >  
> > -   iov.iov_base = buf;
> > while (1) {
> > int status;
> > const struct rtnl_dump_filter_arg *a;
> > int found_done = 0;
> > int msglen = 0;
> >  
> > -   iov.iov_len = sizeof(buf);
> > -   status = recvmsg(rth->fd, , 0);
> > -
> > -   if (status < 0) {
> > -   if (errno == EINTR || errno == EAGAIN)
> > -   continue;
> > -   fprintf(stderr, "netlink receive error %s (%d)\n",
> > -   strerror(errno), errno);
> > -   return -1;
> > -   }
> > -
> > -   if (status == 0) {
> > -   fprintf(stderr, "EOF on netlink\n");
> > -   return -1;
> > -   }
> > +   status = rtnl_recvmsg(rth->fd, , );
> > +   if (status < 0)
> > +   return status;
> > +   else if (status == 0)
> > +   continue;
> 
> When retrying inside rtnl_recvmsg(), it won't return 0 anymore. I
> believe the whole 'while (1)' loop could go away then.

Doesn't this loop also handle the response divided into multiple
packets?

Michal Kubecek

[PATCH net-next 2/3] net: ethernet: socionext: add AVE ethernet driver

[Kunihiko Hayashi]

The UniPhier platform from Socionext provides the AVE ethernet
controller that includes MAC and MDIO bus supporting RGMII/RMII
modes. The controller is named AVE.

Signed-off-by: Kunihiko Hayashi 
Signed-off-by: Jassi Brar 
---
 drivers/net/ethernet/Kconfig |1 +
 drivers/net/ethernet/Makefile|1 +
 drivers/net/ethernet/socionext/Kconfig   |   22 +
 drivers/net/ethernet/socionext/Makefile  |4 +
 drivers/net/ethernet/socionext/sni_ave.c | 1618 ++
 5 files changed, 1646 insertions(+)
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
index c604213..d50519e 100644
--- a/drivers/net/ethernet/Kconfig
+++ b/drivers/net/ethernet/Kconfig
@@ -170,6 +170,7 @@ source "drivers/net/ethernet/sis/Kconfig"
 source "drivers/net/ethernet/sfc/Kconfig"
 source "drivers/net/ethernet/sgi/Kconfig"
 source "drivers/net/ethernet/smsc/Kconfig"
+source "drivers/net/ethernet/socionext/Kconfig"
 source "drivers/net/ethernet/stmicro/Kconfig"
 source "drivers/net/ethernet/sun/Kconfig"
 source "drivers/net/ethernet/tehuti/Kconfig"
diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
index a0a03d4..9f55b36 100644
--- a/drivers/net/ethernet/Makefile
+++ b/drivers/net/ethernet/Makefile
@@ -81,6 +81,7 @@ obj-$(CONFIG_SFC) += sfc/
 obj-$(CONFIG_SFC_FALCON) += sfc/falcon/
 obj-$(CONFIG_NET_VENDOR_SGI) += sgi/
 obj-$(CONFIG_NET_VENDOR_SMSC) += smsc/
+obj-$(CONFIG_NET_VENDOR_SOCIONEXT) += socionext/
 obj-$(CONFIG_NET_VENDOR_STMICRO) += stmicro/
 obj-$(CONFIG_NET_VENDOR_SUN) += sun/
 obj-$(CONFIG_NET_VENDOR_TEHUTI) += tehuti/
diff --git a/drivers/net/ethernet/socionext/Kconfig 
b/drivers/net/ethernet/socionext/Kconfig
new file mode 100644
index 000..788f26f
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Kconfig
@@ -0,0 +1,22 @@
+config NET_VENDOR_SOCIONEXT
+   bool "Socionext ethernet drivers"
+   default y
+   ---help---
+ Option to select ethernet drivers for Socionext platforms.
+
+ Note that the answer to this question doesn't directly affect the
+ kernel: saying N will just cause the configurator to skip all
+ the questions about Agere devices. If you say Y, you will be asked
+ for your specific card in the following questions.
+
+if NET_VENDOR_SOCIONEXT
+
+config SNI_AVE
+   tristate "Socionext AVE ethernet support"
+   depends on (ARCH_UNIPHIER || COMPILE_TEST) && OF
+   select PHYLIB
+   ---help---
+ Driver for gigabit ethernet MACs, called AVE, in the
+ Socionext UniPhier family.
+
+endif #NET_VENDOR_SOCIONEXT
diff --git a/drivers/net/ethernet/socionext/Makefile 
b/drivers/net/ethernet/socionext/Makefile
new file mode 100644
index 000..0356341
--- /dev/null
+++ b/drivers/net/ethernet/socionext/Makefile
@@ -0,0 +1,4 @@
+#
+# Makefile for all ethernet ip drivers on Socionext platforms
+#
+obj-$(CONFIG_SNI_AVE) += sni_ave.o
diff --git a/drivers/net/ethernet/socionext/sni_ave.c 
b/drivers/net/ethernet/socionext/sni_ave.c
new file mode 100644
index 000..c870777
--- /dev/null
+++ b/drivers/net/ethernet/socionext/sni_ave.c
@@ -0,0 +1,1618 @@
+/**
+ * sni_ave.c - Socionext UniPhier AVE ethernet driver
+ *
+ * Copyright 2014 Panasonic Corporation
+ * Copyright 2015-2017 Socionext Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* General Register Group */
+#define AVE_IDR0x0 /* ID */
+#define AVE_VR 0x4 /* Version */
+#define AVE_GRR0x8 /* Global Reset */
+#define AVE_CFGR   0xc /* Configuration */
+
+/* Interrupt Register Group */
+#define AVE_GIMR   0x100   /* Global Interrupt Mask */
+#define AVE_GISR   0x104   /* Global Interrupt Status */
+
+/* MAC Register Group */
+#define AVE_TXCR   0x200   /* TX Setup */
+#define AVE_RXCR   0x204   /* RX Setup */
+#define AVE_RXMAC1R0x208   /* MAC address (lower) */
+#define AVE_RXMAC2R0x20c   /* MAC address (upper) */
+#define AVE_MDIOCTR0x214   /* MDIO Control */
+#define AVE_MDIOAR 0x218   /* MDIO Address */
+#define AVE_MDIOWDR0x21c   /* MDIO Data */
+#define AVE_MDIOSR 0x220   /* MDIO 

[PATCH net-next 1/3] dt-bindings: net: add DT bindings for Socionext UniPhier AVE

[Kunihiko Hayashi]

DT bindings for the AVE ethernet controller found on Socionext's
UniPhier platforms.

Signed-off-by: Kunihiko Hayashi 
Signed-off-by: Jassi Brar 
---
 .../bindings/net/socionext,uniphier-ave4.txt   | 44 ++
 1 file changed, 44 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt

diff --git a/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt 
b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
new file mode 100644
index 000..57ae96d
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
@@ -0,0 +1,44 @@
+* Socionext AVE ethernet controller
+
+This describes the devicetree bindings for AVE ethernet controller
+implemented on Socionext UniPhier SoCs.
+
+Required properties:
+ - compatible:  Should be "socionext,uniphier-ave4"
+ - reg: Address where registers are mapped and size of region.
+ - interrupts: IRQ common for mac and phy interrupts.
+ - phy-mode: See ethernet.txt in the same directory.
+ - #address-cells: Must be <1>.
+ - #size-cells: Must be <0>.
+ - Node, with 'reg' property, for each PHY on the MDIO bus.
+
+Optional properties:
+ - socionext,desc-bits: 32/64 descriptor size. Default 32.
+ - local-mac-address: See ethernet.txt in the same directory.
+ - pinctrl-names: List of assigned state names, see pinctrl
+   binding documentation.
+ - pinctrl-0: List of phandles to configure the GPIO pin used
+   as interrupt line, see also generic and your platform
+   specific pinctrl binding documentation.
+ - socionext,internal-phy-interrupt: Boolean to denote if the
+   PHY interrupt is internally handled by the MAC.
+
+
+Example:
+
+   eth: ethernet@6500 {
+   compatible = "socionext,uniphier-ave4";
+   reg = <0x6500 0x8500>;
+   interrupts = <0 66 4>;
+   pinctrl-names = "default";
+   pinctrl-0 = <_ether_rgmii>;
+   phy-mode = "rgmii";
+   socionext,desc-bits = <64>;
+   local-mac-address = [00 00 00 00 00 00];
+
+   #address-cells = <1>;
+   #size-cells = <0>;
+   ethphy: ethphy@1 {
+   reg = <1>;
+   };
+   };
-- 
2.7.4

[PATCH net-next 0/3] add UniPhier AVE ethernet support

[Kunihiko Hayashi]

This series adds support for Socionext AVE ethernet controller implemented
on UniPhier SoCs. This driver supports RGMII/RMII modes.

Furthermore, this series includes support for realtek RTL8201F PHY to be
implemented on some supported boards.

Jassi Brar (1):
  net: phy: realtek: add RTL8201F phy-id and functions

Kunihiko Hayashi (2):
  dt-bindings: net: add DT bindings for Socionext UniPhier AVE
  net: ethernet: socionext: add AVE ethernet driver

 .../bindings/net/socionext,uniphier-ave4.txt   |   44 +
 drivers/net/ethernet/Kconfig   |1 +
 drivers/net/ethernet/Makefile  |1 +
 drivers/net/ethernet/socionext/Kconfig |   22 +
 drivers/net/ethernet/socionext/Makefile|4 +
 drivers/net/ethernet/socionext/sni_ave.c   | 1618 
 drivers/net/phy/realtek.c  |   45 +
 7 files changed, 1735 insertions(+)
 create mode 100644 
Documentation/devicetree/bindings/net/socionext,uniphier-ave4.txt
 create mode 100644 drivers/net/ethernet/socionext/Kconfig
 create mode 100644 drivers/net/ethernet/socionext/Makefile
 create mode 100644 drivers/net/ethernet/socionext/sni_ave.c

-- 
2.7.4

Re: [PATCH RFC 0/6] Modify KSZ9477 DSA driver in preparation to add other KSZ switch drivers.

[Vivien Didelot]

Hi Tristram,

tristram...@microchip.com writes:

> From: Tristram Ha 
>
> This series of patches is to modify the original KSZ9477 DSA driver so that 
> other KSZ switch drivers can be added and use the common code.

Please see Documentation/process/submitting-patches.rst. Use a correct
formatting for the messages, and send you patchset as a thread.

> This patch set is against net-next.

The net subsystem is somehow special. Please see
Documentation/networking/netdev-FAQ.txt. The tree must be indicated in
the subject prefix of the patch series.


Thanks,

Vivien

[PATCH net-next 3/3] net: phy: realtek: add RTL8201F phy-id and functions

[Kunihiko Hayashi]

From: Jassi Brar 

Add RTL8201F phy-id and the related functions to the driver.

The original patch is as follows:
https://patchwork.kernel.org/patch/2538341/

Signed-off-by: Jongsung Kim 
Signed-off-by: Jassi Brar 
Signed-off-by: Kunihiko Hayashi 
---
 drivers/net/phy/realtek.c | 45 +
 1 file changed, 45 insertions(+)

diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
index 9cbe645..d9974ce 100644
--- a/drivers/net/phy/realtek.c
+++ b/drivers/net/phy/realtek.c
@@ -29,10 +29,23 @@
 #define RTL8211F_PAGE_SELECT   0x1f
 #define RTL8211F_TX_DELAY  0x100
 
+#define RTL8201F_ISR   0x1e
+#define RTL8201F_PAGE_SELECT   0x1f
+#define RTL8201F_IER   0x13
+
 MODULE_DESCRIPTION("Realtek PHY driver");
 MODULE_AUTHOR("Johnson Leung");
 MODULE_LICENSE("GPL");
 
+static int rtl8201_ack_interrupt(struct phy_device *phydev)
+{
+   int err;
+
+   err = phy_read(phydev, RTL8201F_ISR);
+
+   return (err < 0) ? err : 0;
+}
+
 static int rtl821x_ack_interrupt(struct phy_device *phydev)
 {
int err;
@@ -54,6 +67,25 @@ static int rtl8211f_ack_interrupt(struct phy_device *phydev)
return (err < 0) ? err : 0;
 }
 
+static int rtl8201_config_intr(struct phy_device *phydev)
+{
+   int err;
+
+   /* switch to page 7 */
+   phy_write(phydev, RTL8201F_PAGE_SELECT, 0x7);
+
+   if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+   err = phy_write(phydev, RTL8201F_IER,
+   BIT(13) | BIT(12) | BIT(11));
+   else
+   err = phy_write(phydev, RTL8201F_IER, 0);
+
+   /* restore to default page 0 */
+   phy_write(phydev, RTL8201F_PAGE_SELECT, 0x0);
+
+   return err;
+}
+
 static int rtl8211b_config_intr(struct phy_device *phydev)
 {
int err;
@@ -129,6 +161,18 @@ static struct phy_driver realtek_drvs[] = {
.config_aneg= _config_aneg,
.read_status= _read_status,
}, {
+   .phy_id = 0x001cc816,
+   .name   = "RTL8201F 10/100Mbps Ethernet",
+   .phy_id_mask= 0x001f,
+   .features   = PHY_BASIC_FEATURES,
+   .flags  = PHY_HAS_INTERRUPT,
+   .config_aneg= _config_aneg,
+   .read_status= _read_status,
+   .ack_interrupt  = _ack_interrupt,
+   .config_intr= _config_intr,
+   .suspend= genphy_suspend,
+   .resume = genphy_resume,
+   }, {
.phy_id = 0x001cc912,
.name   = "RTL8211B Gigabit Ethernet",
.phy_id_mask= 0x001f,
@@ -181,6 +225,7 @@ static struct phy_driver realtek_drvs[] = {
 module_phy_driver(realtek_drvs);
 
 static struct mdio_device_id __maybe_unused realtek_tbl[] = {
+   { 0x001cc816, 0x001f },
{ 0x001cc912, 0x001f },
{ 0x001cc914, 0x001f },
{ 0x001cc915, 0x001f },
-- 
2.7.4

Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added

[Maxim Uvarov]

2017-09-08 0:54 GMT+03:00 Andrew Lunn :
>> -- compatible: For external switch chips, compatible string must be exactly 
>> one
>> -  of: "microchip,ksz9477"
>> +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip,
>> +   "microchip,ksz8795" for KSZ8795 chip,
>> +   "microchip,ksz8794" for KSZ8794 chip,
>> +   "microchip,ksz8765" for KSZ8765 chip,
>> +   "microchip,ksz8895" for KSZ8895 chip,
>> +   "microchip,ksz8864" for KSZ8864 chip,
>> +   "microchip,ksz8873" for KSZ8873 chip,
>> +   "microchip,ksz8863" for KSZ8863 chip,
>> +   "microchip,ksz8463" for KSZ8463 chip
>

all that chips have the same spi access to get chip id on probe(). I
prefer common microship,ksz-spi rather than somebody will always
maintain that list.

Maxim.

> This part of this patch should be in a patch of the series that
> actually adds support for these chips. Don't document chips until you
> actually support them.
>
>>  See Documentation/devicetree/bindings/dsa/dsa.txt for a list of additional  
>> required and optional properties.
>> @@ -13,60 +20,60 @@ Examples:
>>
>>  Ethernet switch connected via SPI to the host, CPU port wired to eth0:
>>
>> - eth0: ethernet@10001000 {
>> - fixed-link {
>> - speed = <1000>;
>> - full-duplex;
>> - };
>> - };
>> + eth0: ethernet@10001000 {
>> + fixed-link {
>> + speed = <1000>;
>> + full-duplex;
>> + };
>> + };
>>
>> - spi1: spi@f8008000 {
>> - pinctrl-0 = <_spi_ksz>;
>> - cs-gpios = < 25 0>;
>> - id = <1>;
>> - status = "okay";
>> + spi1: spi@f8008000 {
>> + cs-gpios = < 25 0>;
>> + id = <1>;
>> + status = "okay";
>>
>> - ksz9477: ksz9477@0 {
>> - compatible = 
>> "microchip,ksz9477";
>> - reg = <0>;
>> + ksz9477: ksz9477@0 {
>> + compatible = "microchip,ksz9477";
>> + reg = <0>;
>>
>> - 
>> spi-max-frequency = <4400>;
>> - spi-cpha;
>> - spi-cpol;
>> + spi-max-frequency = <4400>;
>> + spi-cpha;
>> + spi-cpol;
>> +
>> + status = "okay";
>> + ports {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> + port@0 {
>> + reg = <0>;
>> + label = "lan1";
>> + };
>> + port@1 {
>> + reg = <1>;
>> + label = "lan2";
>> + };
>> + port@2 {
>> + reg = <2>;
>> + label = "lan3";
>> + };
>> + port@3 {
>> + reg = <3>;
>> + label = "lan4";
>> + };
>> + port@4 {
>> + reg = <4>;
>> + label = "lan5";
>> + };
>> + port@5 {
>> + reg = <5>;
>> + label = "cpu";
>> + ethernet = <>;
>> + fixed-link {
>> + speed = <1000>;
>> + full-duplex;
>> + };
>> + };
>> + };
>> + };
>> + };
>>
>> - status = 
>> "okay";
>> - ports {
>> -
>>  #address-cells = <1>;
>> -
>>  

Re: [PATCH net] bpf: don't select potentially stale ri->map from buggy xdp progs

[Daniel Borkmann]

On 09/08/2017 01:52 PM, Jesper Dangaard Brouer wrote:

On Fri, 08 Sep 2017 12:34:28 +0200 Daniel Borkmann  wrote:

On 09/08/2017 07:06 AM, Jesper Dangaard Brouer wrote:

On Fri,  8 Sep 2017 00:14:51 +0200
Daniel Borkmann  wrote:


+   /* This is really only caused by a deliberately crappy
+* BPF program, normally we would never hit that case,
+* so no need to inform someone via tracepoints either,
+* just bail out.
+*/
+   if (unlikely(map_owner != xdp_prog))
+   return -EINVAL;


IMHO we do need to call the tracepoint here.  It is not just crappy
BPF-progs that cause this situation, it is also drivers not implementing
XDP_REDIRECT yet (which is all but ixgbe).  Due to the level XDP
operates at, tracepoints are the only way users can runtime troubleshoot
their XDP programs.


Drivers not implementing XDP_REDIRECT don't even get there in
the first place. What they will do is to hit the 'default' case
when they check for the action code from the BPF program. Then
call into bpf_warn_invalid_xdp_action(act), and fall-through
to hit the tracepoint at trace_xdp_exception() which is also
triggered by XDP_ABORTED usually. So when that happens we do
complain loudly and call a tracepoint already. We should probably
tweak the bpf_warn_invalid_xdp_action() message a little to make
it clear that the action could also just be unsupported by the
driver instead of being illegal.


Yes. drivers not implementing XDP_REDIRECT will cause a tracepoint
trace_xdp_exception() to be called for its _own_ packets.


Yep, plus a big one time warning for the case a user doesn't
look at tracepoints initially.


But it will still setup and leave map and map_owner pointer dangling.
Another NIC can load an xdp_prog that return XDP_REDIRECT, which will hit
above if-statement, and its packets will disappear, without getting
recorded by a tracepoint (thus hard to debug!).


If a user wants to reproduce this exact error, he would need
to go and reload the program on the driver not supporting the
XDP_REDIRECT in the first place, and then reload his buggy program
on the other driver supporting XDP_REDIRECT but w/o having called
bpf_xdp_redirect_map(), so exactly once on the switch from one
driver to another with this misuse, any subsequent packets will
trigger _trace_xdp_redirect_err(), same way as if the buggy
program was loaded to the 2nd driver from the beginning since
the map and ifindex etc will be zero, hence my comment on this.

Re: [PATCH iproute2 2/2] lib/libnetlink: update rtnl_talk to support malloc buff at run time

[Hangbin Liu]

On Fri, Sep 08, 2017 at 01:06:52PM +0200, Phil Sutter wrote:
> Hi Hangbin,
> 
> On Fri, Sep 08, 2017 at 06:14:57PM +0800, Hangbin Liu wrote:
> [...]
> > diff --git a/genl/ctrl.c b/genl/ctrl.c
> > index 448988e..699657b 100644
> > --- a/genl/ctrl.c
> > +++ b/genl/ctrl.c
> > @@ -55,6 +55,7 @@ int genl_ctrl_resolve_family(const char *family)
> > };
> > struct nlmsghdr *nlh = 
> > struct genlmsghdr *ghdr = 
> > +   struct nlmsghdr *answer = NULL;
> 
> I don't think it's necessary to assign NULL here or in any of the other
> cases.

OK
> 
> > if (rtnl_open_byproto(, 0, NETLINK_GENERIC) < 0) {
> > fprintf(stderr, "Cannot open generic netlink socket\n");
> > @@ -63,19 +64,19 @@ int genl_ctrl_resolve_family(const char *family)
> >  
> > addattr_l(nlh, 128, CTRL_ATTR_FAMILY_NAME, family, strlen(family) + 1);
> >  
> > -   if (rtnl_talk(, nlh, nlh, sizeof(req)) < 0) {
> > +   if (rtnl_talk(, nlh, ) < 0) {
> 
> I didn't check, but it's likely that at least in some cases 'nlh'
> contains some buffer space to hold the answer (if it will be larger than
> the input). If so, this could be dropped since the buffer is no longer
> reused.

Yes, I will remove the buffer.

Thanks
Hangbin

Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

[Corentin Labbe]

On Fri, Sep 08, 2017 at 03:05:20PM +0200, Andrew Lunn wrote:
> > +#define DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID   0
> > +#define DWMAC_sUN8I_MDIO_MUX_EXTERNAL_ID   1
> >  
> >  /* H3/A64 specific bits */
> >  #define SYSCON_RMII_EN BIT(13) /* 1: enable RMII (overrides 
> > EPIT) */
> > @@ -634,6 +639,76 @@ static int sun8i_dwmac_reset(struct stmmac_priv *priv)
> > return 0;
> >  }
> >  
> > +/* MDIO multiplexing switch function
> > + * This function is called by the mdio-mux layer when it thinks the mdio 
> > bus
> > + * multiplexer needs to switch.
> > + * 'current_child' is the current value of the mux register
> > + * 'desired_child' is the value of the 'reg' property of the target child 
> > MDIO
> > + * node.
> > + * The first time this function is called, current_child == -1.
> > + * If current_child == desired_child, then the mux is already set to the
> > + * correct bus.
> > + *
> > + * Note that we do not use reg/mask like mdio-mux-mmioreg because we need 
> > to
> > + * know easily which bus is used (reset must be done only for desired bus).
> > + */
> > +static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
> > +void *data)
> > +{
> > +   struct stmmac_priv *priv = data;
> > +   struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
> > +   u32 reg, val;
> > +   int ret = 0;
> > +   bool need_reset = false;
> > +
> > +   if (current_child ^ desired_child) {
> > +   regmap_read(gmac->regmap, SYSCON_EMAC_REG, );
> > +   switch (desired_child) {
> > +   case DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID:
> > +   dev_info(priv->device, "Switch mux to internal PHY");
> > +   val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SELECT;
> > +   if (gmac->use_internal_phy)
> > +   need_reset = true;
> > +   break;
> 
> This i don't get. Why do you need use_internal_phy? Isn't that
> implicit from DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID? Is it even possible to
> use an external PHY on the internal MDIO bus?
> 

On my H3 box with external PHY, the MDIO mux library first select (for scan ?) 
the internal MDIO.
Without use_internal_phy usage, this board will launch a reset to use the 
internal MDIO... and this reset timeout/fail.
After the MDIO mux select the external MDIO.

> > +   case DWMAC_sUN8I_MDIO_MUX_EXTERNAL_ID:
> > +   dev_info(priv->device, "Switch mux to external PHY");
> > +   val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SHUTDOWN;
> > +   if (!gmac->use_internal_phy)
> > +   need_reset = true;
> > +   break;
> 
> And is it possible to use the internal PHY on the external bus?
> 

I need to check that.

Regards

Re: [PATCH net-next 2/3] net: ethernet: socionext: add AVE ethernet driver

[Andrew Lunn]

> +static int ave_mdio_busywait(struct net_device *ndev)
> +{
> + int ret = 1, loop = 100;
> + u32 mdiosr;
> +
> + /* wait until completion */
> + while (1) {
> + mdiosr = ave_r32(ndev, AVE_MDIOSR);
> + if (!(mdiosr & AVE_MDIOSR_STS))
> + break;
> +
> + usleep_range(10, 20);
> + if (!loop--) {
> + netdev_err(ndev,
> +"failed to read from MDIO (status:0x%08x)\n",
> +mdiosr);
> + ret = 0;

ETIMEDOUT would be better.

> + break;
> + }
> + }
> +
> + return ret;

and then return 0 on success. That is the normal convention for return
values. An error code, and 0.

> +static int ave_mdiobus_write(struct mii_bus *bus,
> +  int phyid, int regnum, u16 val)
> +{
> + struct net_device *ndev = bus->priv;
> + u32 mdioctl;
> +
> + /* write address */
> + ave_w32(ndev, AVE_MDIOAR, (phyid << 8) | regnum);
> +
> + /* write data */
> + ave_w32(ndev, AVE_MDIOWDR, val);
> +
> + /* write request */
> + mdioctl = ave_r32(ndev, AVE_MDIOCTR);
> + ave_w32(ndev, AVE_MDIOCTR, mdioctl | AVE_MDIOCTR_WREQ);
> +
> + if (!ave_mdio_busywait(ndev)) {
> + netdev_err(ndev, "phy-%d reg-%x write failed\n",
> +phyid, regnum);
> + return -1;

If ave_mdio_busywait() returns ETIMEDOUT, you can just return
it. Returning -1 is not good.

> + }
> +
> + return 0;
> +}
> +
> +static irqreturn_t ave_interrupt(int irq, void *netdev)
> +{
> + struct net_device *ndev = (struct net_device *)netdev;
> + struct ave_private *priv = netdev_priv(ndev);
> + u32 gimr_val, gisr_val;
> +
> + gimr_val = ave_irq_disable_all(ndev);
> +
> + /* get interrupt status */
> + gisr_val = ave_r32(ndev, AVE_GISR);
> +
> + /* PHY */
> + if (gisr_val & AVE_GI_PHY) {
> + ave_w32(ndev, AVE_GISR, AVE_GI_PHY);
> + if (priv->internal_phy_interrupt)
> + phy_mac_interrupt(ndev->phydev, ndev->phydev->link);

Humm. I don't think this is correct. You are supposed to give it the
new link state, not the old.

What does a PHY interrupt mean here? 

> +static void ave_adjust_link(struct net_device *ndev)
> +{
> + struct ave_private *priv = netdev_priv(ndev);
> + struct phy_device *phydev = ndev->phydev;
> + u32 val, txcr, rxcr, rxcr_org;
> +
> + /* set RGMII speed */
> + val = ave_r32(ndev, AVE_TXCR);
> + val &= ~(AVE_TXCR_TXSPD_100 | AVE_TXCR_TXSPD_1G);
> +
> + if (priv->phy_mode == PHY_INTERFACE_MODE_RGMII &&
> + phydev->speed == SPEED_1000)

phy_interface_mode_is_rgmii(), so that you handle all the RGMII modes.

> + val |= AVE_TXCR_TXSPD_1G;
> + else if (phydev->speed == SPEED_100)
> + val |= AVE_TXCR_TXSPD_100;
> +
> + ave_w32(ndev, AVE_TXCR, val);
> +
> + /* set RMII speed (100M/10M only) */
> + if (priv->phy_mode != PHY_INTERFACE_MODE_RGMII) {

Not so safe. It would be better to check for the modes you actually
support.

> + if (phydev->link)
> + netif_carrier_on(ndev);
> + else
> + netif_carrier_off(ndev);

I don't think you need this. The phylib should do it for you.

> +
> + phy_print_status(phydev);
> +}
> +
> +static int ave_init(struct net_device *ndev)
> +{
> + struct ave_private *priv = netdev_priv(ndev);
> + struct device *dev = ndev->dev.parent;
> + struct device_node *phy_node, *np = dev->of_node;
> + struct phy_device *phydev;
> + const void *mac_addr;
> + u32 supported;
> +
> + /* get mac address */
> + mac_addr = of_get_mac_address(np);
> + if (mac_addr)
> + ether_addr_copy(ndev->dev_addr, mac_addr);
> +
> + /* if the mac address is invalid, use random mac address */
> + if (!is_valid_ether_addr(ndev->dev_addr)) {
> + eth_hw_addr_random(ndev);
> + dev_warn(dev, "Using random MAC address: %pM\n",
> +  ndev->dev_addr);
> + }
> +
> + /* attach PHY with MAC */
> + phy_node =  of_get_next_available_child(np, NULL);

???

Should this not be looking for a phy-handle property?
Documentation/devicetree/binds/net/ethernet.txt:

- phy-handle: phandle, specifies a reference to a node representing a PHY
  device; this property is described in the Devicetree Specification and so
  preferred;


> + phydev = of_phy_connect(ndev, phy_node,
> + ave_adjust_link, 0, priv->phy_mode);
> + if (!phydev) {
> + dev_err(dev, "could not attach to PHY\n");
> + return -ENODEV;
> + }
> + of_node_put(phy_node);
> +
> + priv->phydev = phydev;
> + phydev->autoneg = AUTONEG_ENABLE;
> + phydev->speed = 0;
> + phydev->duplex = 0;

And this should not be needed.

> +
> + dev_info(dev, 

Re: [PATCH iproute2 2/2] lib/libnetlink: update rtnl_talk to support malloc buff at run time

[Phil Sutter]

Hi Hangbin,

On Fri, Sep 08, 2017 at 06:14:57PM +0800, Hangbin Liu wrote:
[...]
> diff --git a/genl/ctrl.c b/genl/ctrl.c
> index 448988e..699657b 100644
> --- a/genl/ctrl.c
> +++ b/genl/ctrl.c
> @@ -55,6 +55,7 @@ int genl_ctrl_resolve_family(const char *family)
>   };
>   struct nlmsghdr *nlh = 
>   struct genlmsghdr *ghdr = 
> + struct nlmsghdr *answer = NULL;

I don't think it's necessary to assign NULL here or in any of the other
cases.

>   if (rtnl_open_byproto(, 0, NETLINK_GENERIC) < 0) {
>   fprintf(stderr, "Cannot open generic netlink socket\n");
> @@ -63,19 +64,19 @@ int genl_ctrl_resolve_family(const char *family)
>  
>   addattr_l(nlh, 128, CTRL_ATTR_FAMILY_NAME, family, strlen(family) + 1);
>  
> - if (rtnl_talk(, nlh, nlh, sizeof(req)) < 0) {
> + if (rtnl_talk(, nlh, ) < 0) {

I didn't check, but it's likely that at least in some cases 'nlh'
contains some buffer space to hold the answer (if it will be larger than
the input). If so, this could be dropped since the buffer is no longer
reused.

Apart from that, looks good to me!

Thanks, Phil

Re: [PATCH net 0/2] netfilter: ipvs: some fixes in sctp_conn_schedule

[Pablo Neira Ayuso]

On Thu, Aug 31, 2017 at 01:59:08PM +0200, Simon Horman wrote:
> On Mon, Aug 28, 2017 at 06:17:32PM +0200, Pablo Neira Ayuso wrote:
> > On Sun, Aug 20, 2017 at 01:38:06PM +0800, Xin Long wrote:
> > > Patch 1/2 fixes the regression introduced by commit 5e26b1b3abce.
> > > Patch 2/2 makes ipvs not create conn for sctp ABORT packet.
> > 
> > Will wait for Julian and Simon to tell me what I should do with this.
> 
> Hi Pablo,
> 
> could you take these directly with Julian's Ack and the following?
> 
> Signed-off-by: Simon Horman 

Series applied, thanks!

Re: [PATCH] net:netfilter alloc xt_byteslimit_htable with wrong size

[Pablo Neira Ayuso]

On Fri, Sep 08, 2017 at 11:00:16AM +0800, zhizhou.t...@gmail.com wrote:
> From: Zhizhou Tian 
> 
> struct xt_byteslimit_htable used hlist_head,
> but alloc memory with sizeof(struct list_head)

Applied, thanks.

For the record, I have mangled the patch titled to:

netfilter: xt_hashlimit: alloc hashtable with right size

Re: [PATCH net] bpf: don't select potentially stale ri->map from buggy xdp progs

[Jesper Dangaard Brouer]

On Fri, 08 Sep 2017 14:34:13 +0200
Daniel Borkmann  wrote:

> On 09/08/2017 01:52 PM, Jesper Dangaard Brouer wrote:
> > On Fri, 08 Sep 2017 12:34:28 +0200 Daniel Borkmann  
> > wrote:  
> >> On 09/08/2017 07:06 AM, Jesper Dangaard Brouer wrote:  
> >>> On Fri,  8 Sep 2017 00:14:51 +0200
> >>> Daniel Borkmann  wrote:
> >>>  
>  +/* This is really only caused by a deliberately crappy
>  + * BPF program, normally we would never hit that case,
>  + * so no need to inform someone via tracepoints either,
>  + * just bail out.
>  + */
>  +if (unlikely(map_owner != xdp_prog))
>  +return -EINVAL;  
> >>>
> >>> IMHO we do need to call the tracepoint here.  It is not just crappy
> >>> BPF-progs that cause this situation, it is also drivers not implementing
> >>> XDP_REDIRECT yet (which is all but ixgbe).  Due to the level XDP
> >>> operates at, tracepoints are the only way users can runtime troubleshoot
> >>> their XDP programs.  
> >>
> >> Drivers not implementing XDP_REDIRECT don't even get there in
> >> the first place. What they will do is to hit the 'default' case
> >> when they check for the action code from the BPF program. Then
> >> call into bpf_warn_invalid_xdp_action(act), and fall-through
> >> to hit the tracepoint at trace_xdp_exception() which is also
> >> triggered by XDP_ABORTED usually. So when that happens we do
> >> complain loudly and call a tracepoint already. We should probably
> >> tweak the bpf_warn_invalid_xdp_action() message a little to make
> >> it clear that the action could also just be unsupported by the
> >> driver instead of being illegal.  
> >
> > Yes. drivers not implementing XDP_REDIRECT will cause a tracepoint
> > trace_xdp_exception() to be called for its _own_ packets.  
> 
> Yep, plus a big one time warning for the case a user doesn't
> look at tracepoints initially.
> 
> > But it will still setup and leave map and map_owner pointer dangling.
> > Another NIC can load an xdp_prog that return XDP_REDIRECT, which will hit
> > above if-statement, and its packets will disappear, without getting
> > recorded by a tracepoint (thus hard to debug!).  
> 
> If a user wants to reproduce this exact error, he would need
> to go and reload the program on the driver not supporting the
> XDP_REDIRECT in the first place, and then reload his buggy program
> on the other driver supporting XDP_REDIRECT but w/o having called
> bpf_xdp_redirect_map(), so exactly once on the switch from one
> driver to another with this misuse, any subsequent packets will
> trigger _trace_xdp_redirect_err(), same way as if the buggy
> program was loaded to the 2nd driver from the beginning since
> the map and ifindex etc will be zero, hence my comment on this.

We can agree that the second program that experience the side-effect is
also buggy, as just returning XDP_REDIRECT without calling
bpf_xdp_redirect_map() or bpf_xdp_redirect(), is a bug in the bpf
program.  Thus, the comment about a "deliberately crappy BPF program"
is not wrong.

You don't have to load and unload xdp programs.  My test is simply
having two XDP programs running.  1. xdp_redirect_map on mlx5 which
doesn't implement XDP_REDIRECT, and 2. a "deliberately crappy BPF
program" on ixgbe that just returns XDP_REDIRECT.

In below output I've used -EFAULT == -14 to capture this situation
happening:

 ksoftirqd/328 [003]  3437.829882: xdp:xdp_redirect_err: prog_id=3 
action=REDIRECT ifindex=2 to_ifindex=0 err=-22
 swapper 0 [005]  3437.829882: xdp:xdp_redirect_err: prog_id=3 
action=REDIRECT ifindex=2 to_ifindex=0 err=-22
 ksoftirqd/0 7 [000]  3437.829882:xdp:xdp_exception: prog_id=5 
action=REDIRECT ifindex=7
 ksoftirqd/434 [004]  3437.829882: xdp:xdp_redirect_err: prog_id=3 
action=REDIRECT ifindex=2 to_ifindex=0 err=-22
 ksoftirqd/222 [002]  3437.829882: xdp:xdp_redirect_err: prog_id=3 
action=REDIRECT ifindex=2 to_ifindex=0 err=-22
 ksoftirqd/328 [003]  3437.829882: xdp:xdp_redirect_err: prog_id=3 
action=REDIRECT ifindex=2 to_ifindex=0 err=-22
 swapper 0 [005]  3437.829882: xdp:xdp_redirect_err: prog_id=3 
action=REDIRECT ifindex=2 to_ifindex=0 err=-22
 swapper 0 [005]  3437.829882: xdp:xdp_redirect_err: prog_id=3 
action=REDIRECT ifindex=2 to_ifindex=0 err=-22
 ksoftirqd/328 [003]  3437.829882: xdp:xdp_redirect_err: prog_id=3 
action=REDIRECT ifindex=2 to_ifindex=0 err=-22
 ksoftirqd/222 [002]  3437.829882: xdp:xdp_redirect_err: prog_id=3 
action=REDIRECT ifindex=2 to_ifindex=0 err=-22
 ksoftirqd/434 [004]  3437.829882: xdp:xdp_redirect_err: prog_id=3 
action=REDIRECT ifindex=2 to_ifindex=0 err=-22
 ksoftirqd/328 [003]  3437.829882: xdp:xdp_redirect_err: prog_id=3 
action=REDIRECT ifindex=2 to_ifindex=0 err=-22
 ksoftirqd/222 

Re: [RFC net-next 2/5] net/sched: Introduce Credit Based Shaper (CBS) qdisc

[Henrik Austad]

On Thu, Aug 31, 2017 at 06:26:22PM -0700, Vinicius Costa Gomes wrote:
> This queueing discipline implements the shaper algorithm defined by
> the 802.1Q-2014 Section 8.6.8.2 and detailed in Annex L.
> 
> It's primary usage is to apply some bandwidth reservation to user
> defined traffic classes, which are mapped to different queues via the
> mqprio qdisc.
> 
> Initially, it only supports offloading the traffic shaping work to
> supporting controllers.
> 
> Later, when a software implementation is added, the current dependency
> on being installed "under" mqprio can be lifted.
> 
> Signed-off-by: Vinicius Costa Gomes 
> Signed-off-by: Jesus Sanchez-Palencia 

So, I started testing this in my VM to make sure I didn't do anything silly 
and it exploded spectacularly as it used the underlying e1000 driver which 
does not have a ndo_setup_tc present.

I commented inline below, but as a tl;dr

The cbs_init() would call into cbs_change() that would correctly detect 
that ndo_setup_tc is missing and abort. However, at this stage it would try 
to desroy the qdisc and in cbs_destroy() there's no such guard leading to a

BUG: unable to handle kernel NULL pointer dereference at 0010

Fixing that, another NULL would be found when the code walks into 
qdisc_destroy(q->qdisc).

Apart from this, it loaded fine after some wrestling with tc :)

Awesome! :D

> ---
>  include/linux/netdevice.h |   1 +
>  net/sched/Kconfig |  11 ++
>  net/sched/Makefile|   1 +
>  net/sched/sch_cbs.c   | 286 
> ++
>  4 files changed, 299 insertions(+)
>  create mode 100644 net/sched/sch_cbs.c
> 
> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
> index 35de8312e0b5..dd9a2ecd0c03 100644
> --- a/include/linux/netdevice.h
> +++ b/include/linux/netdevice.h
> @@ -775,6 +775,7 @@ enum tc_setup_type {
>   TC_SETUP_CLSFLOWER,
>   TC_SETUP_CLSMATCHALL,
>   TC_SETUP_CLSBPF,
> + TC_SETUP_CBS,
>  };
>  
>  /* These structures hold the attributes of xdp state that are being passed
> diff --git a/net/sched/Kconfig b/net/sched/Kconfig
> index e70ed26485a2..c03d86a7775e 100644
> --- a/net/sched/Kconfig
> +++ b/net/sched/Kconfig
> @@ -172,6 +172,17 @@ config NET_SCH_TBF
> To compile this code as a module, choose M here: the
> module will be called sch_tbf.
>  
> +config NET_SCH_CBS

Shouldn't this depend on NET_SCH_MQPRIO as it is supposed to hook into 
that?

@@ -173,6 +173,7 @@ config NET_SCH_TBF
  module will be called sch_tbf.
 
 config NET_SCH_CBS
+   depends on NET_SCH_MQPRIO
tristate "Credit Based Shaper (CBS)"
---help---
  Say Y here if you want to use the Credit Based Shaper (CBS) packet

> + tristate "Credit Based Shaper (CBS)"
> + ---help---
> +   Say Y here if you want to use the Credit Based Shaper (CBS) packet
> +   scheduling algorithm.
> +
> +   See the top of  for more details.
> +
> +   To compile this code as a module, choose M here: the
> +   module will be called sch_cbs.
> +
>  config NET_SCH_GRED
>   tristate "Generic Random Early Detection (GRED)"
>   ---help---
> diff --git a/net/sched/Makefile b/net/sched/Makefile
> index 7b915d226de7..80c8f92d162d 100644
> --- a/net/sched/Makefile
> +++ b/net/sched/Makefile
> @@ -52,6 +52,7 @@ obj-$(CONFIG_NET_SCH_FQ_CODEL)  += sch_fq_codel.o
>  obj-$(CONFIG_NET_SCH_FQ) += sch_fq.o
>  obj-$(CONFIG_NET_SCH_HHF)+= sch_hhf.o
>  obj-$(CONFIG_NET_SCH_PIE)+= sch_pie.o
> +obj-$(CONFIG_NET_SCH_CBS)+= sch_cbs.o
>  
>  obj-$(CONFIG_NET_CLS_U32)+= cls_u32.o
>  obj-$(CONFIG_NET_CLS_ROUTE4) += cls_route.o
> diff --git a/net/sched/sch_cbs.c b/net/sched/sch_cbs.c
> new file mode 100644
> index ..1c86a9e14150
> --- /dev/null
> +++ b/net/sched/sch_cbs.c
> @@ -0,0 +1,286 @@
> +/*
> + * net/sched/sch_cbs.c   Credit Based Shaper
> + *
> + *   This program is free software; you can redistribute it and/or
> + *   modify it under the terms of the GNU General Public License
> + *   as published by the Free Software Foundation; either version
> + *   2 of the License, or (at your option) any later version.
> + *
> + * Authors:  Vininicius Costa Gomes 
> + *
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +struct cbs_sched_data {
> + struct Qdisc *qdisc; /* Inner qdisc, default - pfifo queue */
> + s32 queue;
> + s32 locredit;
> + s32 hicredit;
> + s32 sendslope;
> + s32 idleslope;
> +};
> +
> +static int cbs_enqueue(struct sk_buff *skb, struct Qdisc *sch,
> +struct sk_buff **to_free)
> +{
> + struct cbs_sched_data *q = qdisc_priv(sch);
> + int ret;
> +
> + ret = qdisc_enqueue(skb, q->qdisc, to_free);
> + if (ret != 

Re: [PATCH net-next 3/3] net: phy: realtek: add RTL8201F phy-id and functions

[Andrew Lunn]

On Fri, Sep 08, 2017 at 10:02:11PM +0900, Kunihiko Hayashi wrote:
> From: Jassi Brar 
> 
> Add RTL8201F phy-id and the related functions to the driver.
> 
> The original patch is as follows:
> https://patchwork.kernel.org/patch/2538341/
> 
> Signed-off-by: Jongsung Kim 
> Signed-off-by: Jassi Brar 
> Signed-off-by: Kunihiko Hayashi 

Reviewed-by: Andrew Lunn 

Andrew

Re: [PATCH iproute2 1/2] lib/libnetlink: re malloc buff if size is not enough

[Hangbin Liu]

Hi Phil,

Thanks for the comments, see replies bellow.

On Fri, Sep 08, 2017 at 01:02:47PM +0200, Phil Sutter wrote:
> Hi Hangbin,
> 
> On Fri, Sep 08, 2017 at 06:14:56PM +0800, Hangbin Liu wrote:
> [...]
> > diff --git a/lib/libnetlink.c b/lib/libnetlink.c
> > index be7ac86..37cfb5a 100644
> > --- a/lib/libnetlink.c
> > +++ b/lib/libnetlink.c
> > @@ -402,6 +402,59 @@ static void rtnl_dump_error(const struct rtnl_handle 
> > *rth,
> > }
> >  }
> >  
> > +static int rtnl_recvmsg(int fd, struct msghdr *msg, char **buf)
> > +{
> > +   struct iovec *iov;
> > +   int len = -1, buf_len = 32768;
> > +   char *buffer = *buf;
> 
> Isn't it possible to make 'buffer' static instead of the two 'buf'
> variables in rtnl_dump_filter_l() and __rtnl_talk()? Then we would have
> only a single buffer which is shared between both functions instead of
> two which are independently allocated.

I was also thinking of this before. But in function ipaddrlabel_flush()

if (rtnl_dump_filter(, flush_addrlabel, NULL) < 0)

It will cal rtnl_dump_filter_l() first via
rtnl_dump_filter() -> rtnl_dump_filter_nc() -> rtnl_dump_filter_l().

Then call rtnl_talk() later via call back
a->filter(, h, a->arg1) -> flush_addrlabel() -> rtnl_talk()

So if we only use one static buffer in rtnl_recvmsg(). Then it will be written
at lease twice.

The path looks like bellow in function rtnl_dump_filter_l()

while (1) {
status = rtnl_recvmsg(rth->fd, , ); <== write buf

for (a = arg; a->filter; a++) {
struct nlmsghdr *h = (struct nlmsghdr *)buf;<== 
assign buf to h

while (NLMSG_OK(h, msglen)) {

if (!rth->dump_fp) {
err = a->filter(, h, a->arg1);   
<== buf changed via rtnl_talk()
}

h = NLMSG_NEXT(h, msglen);  <== so h will 
also be changed
}
}
}

That's why I have to use two static buffers.
> 
> > +
> > +   int flag = MSG_PEEK | MSG_TRUNC;
> > +
> > +   if (buffer == NULL)
> > +re_malloc:
> > +   buffer = malloc(buf_len);
> 
> I think using realloc() here is more appropriate since there is no need
> to free the buffer in beforehand and calling realloc(NULL, len) is
> equivalent to calling malloc(len). I think 'realloc' is also a better
> name for the goto label.

Good idea.
> 
> > +   if (buffer == NULL) {
> > +   fprintf(stderr, "malloc error: no enough buffer\n");
> 
> Minor typo here: s/no/not/
> 
> > +   return -1;
> 
> Return -ENOMEM?
> 
> > +   }
> > +
> > +   iov = msg->msg_iov;
> > +   iov->iov_base = buffer;
> > +   iov->iov_len = buf_len;
> > +
> > +re_recv:
> 
> Just call this 'recv'? (Not really important though.)
> 
> > +   len = recvmsg(fd, msg, flag);
> > +
> > +   if (len < 0) {
> > +   if (errno == EINTR || errno == EAGAIN)
> > +   return 0;
> 
> Instead of returning 0 (which makes callers retry), goto re_recv?

Yes, will fix this.
> 
> > +   fprintf(stderr, "netlink receive error %s (%d)\n",
> > +   strerror(errno), errno);
> > +   return len;
> > +   }
> > +
> > +   if (len == 0) {
> > +   fprintf(stderr, "EOF on netlink\n");
> > +   return -1;
> 
> Return -ENODATA here? (Initially I though about -EOF, but EOF is -1 so
> that would be incorrect).
> 
> > +   }
> > +
> > +   if (len > buf_len) {
> > +   free(buffer);
> 
> If you use realloc() above, this can be dropped.

Yes.
> 
> > +   buf_len = len;
> 
> For this to work you have to make buf_len static too, otherwise you will
> unnecessarily reallocate the buffer. Oh, and that also requires the
> single buffer (as pointed out above) because you will otherwise use a
> common buf_len for both static buffers passed to this function.

Since we have to use two static bufffers. So how about check like

if (len > strlen(buffer))

> 
> > +   flag = 0;
> > +   goto re_malloc;
> > +   }
> > +
> > +   if (flag != 0) {
> > +   flag = 0;
> > +   goto re_recv;
> > +   }
> > +
> > +   *buf = buffer;
> > +   return len;
> > +}
> > +
> >  int rtnl_dump_filter_l(struct rtnl_handle *rth,
> >const struct rtnl_dump_filter_arg *arg)
> >  {
> > @@ -413,31 +466,20 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
> > .msg_iov = ,
> > .msg_iovlen = 1,
> > };
> > -   char buf[32768];
> > +   static char *buf = NULL;
> 
> If you keep the static buffer in rtnl_recvmsg(), there is no need to
> assign NULL here.
> 
> > int dump_intr = 0;
> >  
> > -   iov.iov_base = buf;
> > while (1) {
> > int status;
> > const struct rtnl_dump_filter_arg *a;
> > int found_done = 0;
> > int msglen = 0;
> >  
> > -   iov.iov_len = sizeof(buf);
> > -   status = 

Re: [PATCH net-next v2 01/10] net: dsa: add debugfs interface

[Vivien Didelot]

Hi Greg,

Greg KH  writes:

> I agree you shouldn't be using debugfs for this, but in the future, if
> you do write debugfs code, please take the following review into
> account:

Humm sorry I may not have given enough details. This was really meant
for debug and dev only, because DSA makes it hard to query directly the
hardware (some switch ports are not exposed to userspace as well.)

This is not meant to be used for anything real at all, or even be
compiled-in in a production kernel. That's why I found it appropriate.

So I am still wondering why it doesn't fit here, can you tell me why?

> You should _never_ care about the return value of a debugfs call, and
> you should not need to ever propagate the error upward.  The api was
> written to not need this.
>
> Just call the function, and return, that's it.  If you need to save the
> return value (i.e. it's a dentry), you also don't care, just save it and
> pass it to some other debugfs call, and all will still be fine.  Your
> code should never do anything different if a debugfs call succeeds or
> fails.

Thank for your interesting review! I'll cleanup my out-of-tree patches.


  Vivien

Re: [PATCH net-next 1/3] dt-bindings: net: add DT bindings for Socionext UniPhier AVE

[Andrew Lunn]

> + eth: ethernet@6500 {
> + compatible = "socionext,uniphier-ave4";
> + reg = <0x6500 0x8500>;
> + interrupts = <0 66 4>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_ether_rgmii>;
> + phy-mode = "rgmii";
> + socionext,desc-bits = <64>;
> + local-mac-address = [00 00 00 00 00 00];
> +
> + #address-cells = <1>;
> + #size-cells = <0>;

> + ethphy: ethphy@1 {
> + reg = <1>;
> + };


So you normally have an mdio node, and the phy as a children of that
node.

   mdio {
ethphy: ethernet-phy@6 {
reg = <6>;
};
};

Andrew

Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

[Andrew Lunn]

> +#define DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID 0
> +#define DWMAC_sUN8I_MDIO_MUX_EXTERNAL_ID 1
>  
>  /* H3/A64 specific bits */
>  #define SYSCON_RMII_EN   BIT(13) /* 1: enable RMII (overrides 
> EPIT) */
> @@ -634,6 +639,76 @@ static int sun8i_dwmac_reset(struct stmmac_priv *priv)
>   return 0;
>  }
>  
> +/* MDIO multiplexing switch function
> + * This function is called by the mdio-mux layer when it thinks the mdio bus
> + * multiplexer needs to switch.
> + * 'current_child' is the current value of the mux register
> + * 'desired_child' is the value of the 'reg' property of the target child 
> MDIO
> + * node.
> + * The first time this function is called, current_child == -1.
> + * If current_child == desired_child, then the mux is already set to the
> + * correct bus.
> + *
> + * Note that we do not use reg/mask like mdio-mux-mmioreg because we need to
> + * know easily which bus is used (reset must be done only for desired bus).
> + */
> +static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
> +  void *data)
> +{
> + struct stmmac_priv *priv = data;
> + struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
> + u32 reg, val;
> + int ret = 0;
> + bool need_reset = false;
> +
> + if (current_child ^ desired_child) {
> + regmap_read(gmac->regmap, SYSCON_EMAC_REG, );
> + switch (desired_child) {
> + case DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID:
> + dev_info(priv->device, "Switch mux to internal PHY");
> + val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SELECT;
> + if (gmac->use_internal_phy)
> + need_reset = true;
> + break;

This i don't get. Why do you need use_internal_phy? Isn't that
implicit from DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID? Is it even possible to
use an external PHY on the internal MDIO bus?

> + case DWMAC_sUN8I_MDIO_MUX_EXTERNAL_ID:
> + dev_info(priv->device, "Switch mux to external PHY");
> + val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SHUTDOWN;
> + if (!gmac->use_internal_phy)
> + need_reset = true;
> + break;

And is it possible to use the internal PHY on the external bus?

Andrew

Re: [PATCH iproute2 0/2] malloc correct buff at run time

[Michal Kubecek]

On Fri, Sep 08, 2017 at 06:14:55PM +0800, Hangbin Liu wrote:
> With commit 72b365e8e0fd ("libnetlink: Double the dump buffer size") and
> 460c03f3f3cc ("iplink: double the buffer size also in iplink_get()"), we
> extend the buffer size to avoid truncated message with large numbers of
> VFs. But just as Michal said, this is not future-proof since the NIC
> number is increasing. We have customer even has 220+ VFs now.

This sounds like the moment we hit the bigger problem with
IFLA_VFINFO_LIST is closer than I thought. The info block for one VF is
already 236 bytes long (or was in 4.4) so that 278 VFs would be over the
natural limit for IFLA_VFINFO_LIST attribute given by 16-bit nla_len
field.

> This is not make sense to hard code the buffer and increase it all the time.
> So let's just malloc the correct buff size at run time.
> 
> I'm not sure what init size would be suitable, so I keep use the original
> size. I have tried with a small size like 1024, and it also works.

I don't think it's that important as the command usually runs only for
short time so allocating a 32KB buffer even if we could do with less is
not a big issue. (I didn't really like the idea of a 32KB buffer on
stack but with malloc() it's OK, I would say.)

Michal Kubecek

[PATCH 1/1] forcedeth: remove tx_stop variable

[Zhu Yanjun]

The variable tx_stop is used to indicate the tx queue state: started
or stopped. In fact, the inline function netif_queue_stopped can do
the same work. So replace the variable tx_stop with the
function netif_queue_stopped.

Signed-off-by: Zhu Yanjun 
---
 drivers/net/ethernet/nvidia/forcedeth.c | 13 -
 1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/nvidia/forcedeth.c 
b/drivers/net/ethernet/nvidia/forcedeth.c
index 994a83a..e6e0de4 100644
--- a/drivers/net/ethernet/nvidia/forcedeth.c
+++ b/drivers/net/ethernet/nvidia/forcedeth.c
@@ -834,7 +834,6 @@ struct fe_priv {
u32 tx_pkts_in_progress;
struct nv_skb_map *tx_change_owner;
struct nv_skb_map *tx_end_flip;
-   int tx_stop;
 
/* TX software stats */
struct u64_stats_sync swstats_tx_syncp;
@@ -1939,7 +1938,6 @@ static void nv_init_tx(struct net_device *dev)
np->tx_pkts_in_progress = 0;
np->tx_change_owner = NULL;
np->tx_end_flip = NULL;
-   np->tx_stop = 0;
 
for (i = 0; i < np->tx_ring_size; i++) {
if (!nv_optimized(np)) {
@@ -2211,7 +2209,6 @@ static netdev_tx_t nv_start_xmit(struct sk_buff *skb, 
struct net_device *dev)
empty_slots = nv_get_empty_tx_slots(np);
if (unlikely(empty_slots <= entries)) {
netif_stop_queue(dev);
-   np->tx_stop = 1;
spin_unlock_irqrestore(>lock, flags);
return NETDEV_TX_BUSY;
}
@@ -2359,7 +2356,6 @@ static netdev_tx_t nv_start_xmit_optimized(struct sk_buff 
*skb,
empty_slots = nv_get_empty_tx_slots(np);
if (unlikely(empty_slots <= entries)) {
netif_stop_queue(dev);
-   np->tx_stop = 1;
spin_unlock_irqrestore(>lock, flags);
return NETDEV_TX_BUSY;
}
@@ -2583,8 +2579,8 @@ static int nv_tx_done(struct net_device *dev, int limit)
 
netdev_completed_queue(np->dev, tx_work, bytes_compl);
 
-   if (unlikely((np->tx_stop == 1) && (np->get_tx.orig != orig_get_tx))) {
-   np->tx_stop = 0;
+   if (unlikely(netif_queue_stopped(dev) &&
+(np->get_tx.orig != orig_get_tx))) {
netif_wake_queue(dev);
}
return tx_work;
@@ -2637,8 +2633,8 @@ static int nv_tx_done_optimized(struct net_device *dev, 
int limit)
 
netdev_completed_queue(np->dev, tx_work, bytes_cleaned);
 
-   if (unlikely((np->tx_stop == 1) && (np->get_tx.ex != orig_get_tx))) {
-   np->tx_stop = 0;
+   if (unlikely(netif_queue_stopped(dev) &&
+(np->get_tx.ex != orig_get_tx))) {
netif_wake_queue(dev);
}
return tx_work;
@@ -2724,7 +2720,6 @@ static void nv_tx_timeout(struct net_device *dev)
/* 2) complete any outstanding tx and do not give HW any limited tx 
pkts */
saved_tx_limit = np->tx_limit;
np->tx_limit = 0; /* prevent giving HW any limited pkts */
-   np->tx_stop = 0;  /* prevent waking tx queue */
if (!nv_optimized(np))
nv_tx_done(dev, np->tx_ring_size);
else
-- 
2.7.4

Re: Re: [PATCH] mm/vmstats: add counters for the page frag cache

[Kyeongdon Kim]

On 2017-09-04 오후 5:30, Konstantin Khlebnikov wrote:

On 04.09.2017 04:35, Kyeongdon Kim wrote:
> Thanks for your reply,
> But I couldn't find "NR_FRAGMENT_PAGES" in linux-next.git .. is that 
vmstat counter? or others?

>

I mean rather than adding bunch vmstat counters for operations it 
might be

worth to add page counter which will show current amount of these pages.
But this seems too low-level for tracking, common counters for all 
network

buffers would be more useful but much harder to implement.


Ok, thanks for the comment.
As I can see page owner is able to save stacktrace where allocation 
happened,
this makes debugging mostly trivial without any counters. If it adds 
too much
overhead - just track random 1% of pages, should be enough for finding 
leak.



As I said, we already used page owner tools to resolve the leak issue.
But that's extremely difficult to figure it out,
too much callstack and too much allocation orders(0 or more).
We couldn't easily get a clue event if we track 1% of pages..

In fact, I was writing another email to send a new patch with debug config.
We added a hash function to pick out address with callstack by using 
debugfs,

It could be showing the only page_frag_cache leak with owner.

for exmaple code :
+++ /mm/page_alloc.c
@@ -4371,7 +4371,9 @@ void *page_frag_alloc(struct page_frag_cache *nc,

    nc->pagecnt_bias--;
    nc->offset = offset;
+#ifdef CONFIG_PGFRAG_DEBUG
+   page_frag_debug_alloc(nc->va + offset);
+#endif
    return nc->va + offset;
 }
 EXPORT_SYMBOL(page_frag_alloc);
@@ -4382,7 +4384,9 @@ EXPORT_SYMBOL(page_frag_alloc);
 void page_frag_free(void *addr)
 {
    struct page *page = virt_to_head_page(addr);
+#ifdef CONFIG_PGFRAG_DEBUG
+   page_frag_debug_free(addr);
+#endif
    if (unlikely(put_page_testzero(page)))

Those counters that I added may be too much for the linux server or 
something.

However, I think the other systems may need to simple debugging method.
(like Android OS)

So if you can accept the patch with debug feature,
I will send it including counters.
but still thinking those counters don't need. I won't.

Anyway, I'm grateful for your feedback, means a lot to me.

Thanks,
Kyeongdon Kim
> As you know, page_frag_alloc() directly calls 
__alloc_pages_nodemask() function,
> so that makes too difficult to see memory usage in real time even 
though we have "/meminfo or /slabinfo.." information.
> If there was a way already to figure out the memory leakage from 
page_frag_cache in mainline, I agree your opinion

> but I think we don't have it now.
>
> If those counters too much in my patch,
> I can say two values (pgfrag_alloc and pgfrag_free) are enough to 
guess what will happen

> and would remove pgfrag_alloc_calls and pgfrag_free_calls.
>
> Thanks,
> Kyeongdon Kim
>

>> IMHO that's too much counters.
>> Per-node NR_FRAGMENT_PAGES should be enough for guessing what's 
going on.

>> Perf probes provides enough features for furhter debugging.
>>
>> On 01.09.2017 02:37, Kyeongdon Kim wrote:
>> > There was a memory leak problem when we did stressful test
>> > on Android device.
>> > The root cause of this was from page_frag_cache alloc
>> > and it was very hard to find out.
>> >
>> > We add to count the page frag allocation and free with function 
call.

>> > The gap between pgfrag_alloc and pgfrag_free is good to to calculate
>> > for the amount of page.
>> > The gap between pgfrag_alloc_calls and pgfrag_free_calls is for
>> > sub-indicator.
>> > They can see trends of memory usage during the test.
>> > Without it, it's difficult to check page frag usage so I believe we
>> > should add it.
>> >
>> > Signed-off-by: Kyeongdon Kim 
>> > ---

Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added

[Pavel Machek]

On Thu 2017-09-07 21:11:45, tristram...@microchip.com wrote:
> From: Tristram Ha 
> 
> Add other KSZ switches support so that patch check does not complain.

Nice; unlike Andrew I believe that adding compatible strings is ok
before the support is added. Device tree description is
Linux-independend.

But... you need to Cc: device tree maintainers on this one. They
usually take some time to respond, so it is nice to do this early.

> index 0ab8b39..34af0e0 100644
> --- a/Documentation/devicetree/bindings/net/dsa/ksz.txt
> +++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt
> @@ -3,8 +3,15 @@ Microchip KSZ Series Ethernet switches
>  
>  Required properties:
>  
> -- compatible: For external switch chips, compatible string must be exactly 
> one
> -  of: "microchip,ksz9477"
> +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip,
> +   "microchip,ksz8795" for KSZ8795 chip,
> +   "microchip,ksz8794" for KSZ8794 chip,
> +   "microchip,ksz8765" for KSZ8765 chip,
> +   "microchip,ksz8895" for KSZ8895 chip,
> +   "microchip,ksz8864" for KSZ8864 chip,
> +   "microchip,ksz8873" for KSZ8873 chip,
> +   "microchip,ksz8863" for KSZ8863 chip,
> +   "microchip,ksz8463" for KSZ8463 chip
>  
>  See Documentation/devicetree/bindings/dsa/dsa.txt for a list of additional  
> required and optional properties.

And you may want to wrap long line here, unless that's email client
playing another trick.
Pavel

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


signature.asc
Description: Digital signature

Re: [V2 PATCH net-next 1/2] xdp: implement xdp_redirect_map for generic XDP

[Daniel Borkmann]

On 09/08/2017 10:36 AM, Jesper Dangaard Brouer wrote:

On Thu, 07 Sep 2017 16:09:56 +0200
Daniel Borkmann  wrote:

On 09/07/2017 02:33 PM, Jesper Dangaard Brouer wrote:

Using bpf_redirect_map is allowed for generic XDP programs, but the
appropriate map lookup was never performed in xdp_do_generic_redirect().

Instead the map-index is directly used as the ifindex.  For the
xdp_redirect_map sample in SKB-mode '-S', this resulted in trying
sending on ifindex 0 which isn't valid, resulting in getting SKB
packets dropped.  Thus, the reported performance numbers are wrong in
commit 24251c264798 ("samples/bpf: add option for native and skb mode
for redirect apps") for the 'xdp_redirect_map -S' case.

It might seem innocent this was lacking, but it can actually crash the
kernel.  The potential crash is caused by not consuming redirect_info->map.
The bpf_redirect_map helper will set this_cpu_ptr(_info)->map
pointer, which will survive even after unloading the xdp bpf_prog and
deallocating the devmap data-structure.  This leaves a dead map
pointer around.  The kernel will crash when loading the xdp_redirect
sample (in native XDP mode) as it doesn't reset map (via bpf_redirect)
and returns XDP_REDIRECT, which will cause it to dereference the map
pointer.

Fixes: 6103aa96ec07 ("net: implement XDP_REDIRECT for xdp generic")
Fixes: 24251c264798 ("samples/bpf: add option for native and skb mode for redirect 
apps")
Signed-off-by: Jesper Dangaard Brouer 
---
   include/trace/events/xdp.h |4 ++--
   net/core/filter.c  |   14 +++---
   2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h
index 862575ac8da9..4e16c43fba10 100644
--- a/include/trace/events/xdp.h
+++ b/include/trace/events/xdp.h
@@ -138,11 +138,11 @@ DEFINE_EVENT_PRINT(xdp_redirect_template, 
xdp_redirect_map_err,

   #define _trace_xdp_redirect_map(dev, xdp, fwd, map, idx) \
 trace_xdp_redirect_map(dev, xdp, fwd ? fwd->ifindex : 0,\
-   0, map, idx);
+   0, map, idx)

   #define _trace_xdp_redirect_map_err(dev, xdp, fwd, map, idx, err)\
 trace_xdp_redirect_map_err(dev, xdp, fwd ? fwd->ifindex : 0,\
-   err, map, idx);
+   err, map, idx)

   #endif /* _TRACE_XDP_H */

diff --git a/net/core/filter.c b/net/core/filter.c
index 5912c738a7b2..3767470cab6c 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2566,13 +2566,19 @@ int xdp_do_generic_redirect(struct net_device *dev, 
struct sk_buff *skb,
struct bpf_prog *xdp_prog)
   {
struct redirect_info *ri = this_cpu_ptr(_info);
+   struct bpf_map *map = ri->map;
u32 index = ri->ifindex;
struct net_device *fwd;
unsigned int len;
int err = 0;

-   fwd = dev_get_by_index_rcu(dev_net(dev), index);
ri->ifindex = 0;
+   ri->map = NULL;
+
+   if (map)
+   fwd = __dev_map_lookup_elem(map, index);
+   else
+   fwd = dev_get_by_index_rcu(dev_net(dev), index);
if (unlikely(!fwd)) {
err = -EINVAL;
goto err;
@@ -2590,10 +2596,12 @@ int xdp_do_generic_redirect(struct net_device *dev, 
struct sk_buff *skb,
}

skb->dev = fwd;


Looks much better above, thanks!


-   _trace_xdp_redirect(dev, xdp_prog, index);
+   map ? _trace_xdp_redirect_map(dev, xdp_prog, fwd, map, index)
+   : _trace_xdp_redirect(dev, xdp_prog, index);


Could we rather make this in a way such that when the two
tracepoints are disabled and thus patched out, that we can
also omit the extra conditional which has no purpose then?


First of all I don't think it make much of a difference, I measured the
impact of the full patch to "cost" 1.62 nanosec (which is arguably
below the accuracy level of the system under test)

Secondly, I plan to optimize the map case for generic XDP later, where
I would naturally split this into two functions (as V1, and as
native-XDP), thus this extra conditional would go away.  As I've shown
offlist (to you, John and Andy) I demonstrated a 24% speedup via a
xmit_more hack for generic XDP.


Okay, that would be nice indeed to have xmit_more support for
generic XDP as well. If this is going to be split off anyway
later on as in xdp_do_redirect() case, then:

Acked-by: Daniel Borkmann 

Re: [PATCH v5 01/10] arm64: dts: allwinner: Restore EMAC changes

[Maxime Ripard]

On Fri, Sep 08, 2017 at 09:11:47AM +0200, Corentin Labbe wrote:
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts 
> b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
> index 1c2387bd5df6..968908761194 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
> @@ -50,6 +50,7 @@
>   compatible = "friendlyarm,nanopi-neo2", "allwinner,sun50i-h5";
>  
>   aliases {
> + ethernet0 = 
>   serial0 = 
>   };
>  
> @@ -108,6 +109,22 @@
>   status = "okay";
>  };
>  
> + {
> + pinctrl-names = "default";
> + pinctrl-0 = <_rgmii_pins>;
> + phy-supply = <_gmac_3v3>;
> + phy-handle = <_rgmii_phy>;
> + phy-mode = "rgmii";
> + status = "okay";
> +};
> +
> + {
> + ext_rgmii_phy: ethernet-phy@7 {
> + compatible = "ethernet-phy-ieee802.3-c22";
> + reg = <7>;
> + };
> +};
> +

This won't compile, you don't have that node in the H5 DTSI.

Maxime

-- 
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com


signature.asc
Description: PGP signature

Re: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ switch drivers.

[Pavel Machek]

> > > Signed-off-by: Tristram Ha 
> > > ---
> > > diff --git a/drivers/net/dsa/microchip/Makefile
> > > b/drivers/net/dsa/microchip/Makefile
> > > index ed335e2..0961c30 100644
> > > --- a/drivers/net/dsa/microchip/Makefile
> > > +++ b/drivers/net/dsa/microchip/Makefile
> > > @@ -1,2 +1,2 @@
> > > -obj-$(CONFIG_MICROCHIP_KSZ)  += ksz_common.o
> > > +obj-$(CONFIG_MICROCHIP_KSZ)  += ksz9477.o ksz_common.o
> > >  obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER)   += ksz_spi.o
> > 
> > Hi Tristram
> > 
> > I would of thought this would break the build. You don't add ksz9477.c 
> > until the
> > next patch.
> > 
> > Each patch needs to compile, otherwise you break git bisect.
> > 
> >  Andrew
> 
> Eventually the file will need to be broken in two, so you would like
>to see all 3 changes (Makefile, ksz_common.c, and ksz9477.c) in 1
> patch file?

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


signature.asc
Description: Digital signature

Re: [PATCH RFC 3/5] Add KSZ8795 switch driver

[Pavel Machek]

On Thu 2017-09-07 21:17:16, tristram...@microchip.com wrote:
> From: Tristram Ha 
> 
> Add KSZ8795 switch support with function code.

English? "Add KSZ8795 switch support." ?

> Signed-off-by: Tristram Ha 
> ---
> diff --git a/drivers/net/dsa/microchip/ksz8795.c 
> b/drivers/net/dsa/microchip/ksz8795.c
> new file mode 100644
> index 000..e4d4e6a
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/ksz8795.c
> @@ -0,0 +1,2066 @@
> +/*
> + * Microchip KSZ8795 switch driver
> + *
> + * Copyright (C) 2017 Microchip Technology Inc.
> + *   Tristram Ha 
> + *
> + * Permission to use, copy, modify, and/or distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */

This is not exactly GPL, right? But tagging below says it is
GPL. Please fix one.

> +/**
> + * Some counters do not need to be read too often because they are less 
> likely
> + * to increase much.
> + */

Kerneldoc markup but this does not look like kerneldoc. Use plain /* instead?

> +static void ksz_cfg(struct ksz_device *dev, u32 addr, u8 bits, bool set)
> +{
> + u8 data;
> +
> + ksz_read8(dev, addr, );
> + if (set)
> + data |= bits;
> + else
> + data &= ~bits;
> + ksz_write8(dev, addr, data);
> +}
> +
> +static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 
> bits,
> +  bool set)
> +{
> + u32 addr;
> + u8 data;
> +
> + addr = PORT_CTRL_ADDR(port, offset);
> + ksz_read8(dev, addr, );
> +
> + if (set)
> + data |= bits;
> + else
> + data &= ~bits;
> +
> + ksz_write8(dev, addr, data);
> +}

Other drivers will need this code, right? Does it make sense to put it
into header?

> +static int chk_last_port(struct ksz_device *dev, int p)
> +{
> + if (dev->last_port && p == dev->last_port)
> + p = dev->cpu_port;
> + return p;
> +}

We often prefix even static functions with common prefix. Up to you, I
guess.

> +static int ksz_reset_switch(struct ksz_device *dev)
> +{
> + /* reset switch */
> + ksz_write8(dev, REG_POWER_MANAGEMENT_1,
> +SW_SOFTWARE_POWER_DOWN << SW_POWER_MANAGEMENT_MODE_S);
> + ksz_write8(dev, REG_POWER_MANAGEMENT_1, 0);
> +
> + return 0;
> +}

This is going to be same in other drivers, right?

> +
> + for (timeout = 1; timeout > 0; timeout--) {
> + ksz_read8(dev, REG_IND_MIB_CHECK, );
> +
> + if (check & MIB_COUNTER_VALID) {
> + ksz_read32(dev, REG_IND_DATA_LO, );
> + if (addr < 2) {
> + u64 total;
> +
> + total = check & MIB_TOTAL_BYTES_H;
> + total <<= 32;
> + *cnt += total;
> + *cnt += data;
> + if (check & MIB_COUNTER_OVERFLOW) {
> + total = MIB_TOTAL_BYTES_H + 1;
> + total <<= 32;
> + *cnt += total;
> + }
> + } else {
> + if (check & MIB_COUNTER_OVERFLOW)
> + *cnt += MIB_PACKET_DROPPED + 1;
> + *cnt += data & MIB_PACKET_DROPPED;
> + }
> + break;
> + }
> + }

Why do you need a loop here? This is quite strange code. (And you have
similar strangeness elsewhere. Please fix.)

> +static int valid_dyn_entry(struct ksz_device *dev, u8 *data)
> +{
> + int timeout = 100;
> +
> + do {
> + ksz_read8(dev, REG_IND_DATA_CHECK, data);
> + timeout--;
> + } while ((*data & DYNAMIC_MAC_TABLE_NOT_READY) && timeout);
> +
> + /* Entry is not ready for accessing. */
> + if (*data & DYNAMIC_MAC_TABLE_NOT_READY) {
> + return 1;
> + /* Entry is ready for accessing. */
> + } else {
> + ksz_read8(dev, REG_IND_DATA_8, data);
> +
> + /* There is no valid entry in the table. */
> + if (*data & DYNAMIC_MAC_TABLE_MAC_EMPTY)
> + return 2;
> + }
> + return 0;
> +}

Normal calling convention is 0 

[PATCH iproute2 2/2] lib/libnetlink: update rtnl_talk to support malloc buff at run time

[Hangbin Liu]

This is an update for 460c03f3f3cc ("iplink: double the buffer size also in
iplink_get()"). After update, we will not need to double the buffer size
every time when VFs number increased.

With call like rtnl_talk(, , NULL, 0), we can simply remove the
length parameter.

With call like rtnl_talk(, nlh, nlh, sizeof(req), I add a new variable
answer to avoid overwrite data in nlh, because it may has more info after
nlh. also this will avoid nlh buffer not enough issue.

Signed-off-by: Hangbin Liu 
---
 bridge/fdb.c |  2 +-
 bridge/link.c|  2 +-
 bridge/mdb.c |  2 +-
 bridge/vlan.c|  2 +-
 genl/ctrl.c  | 14 --
 include/libnetlink.h |  6 +++---
 ip/ipaddress.c   |  5 +++--
 ip/ipaddrlabel.c |  4 ++--
 ip/ipfou.c   |  4 ++--
 ip/ipila.c   |  4 ++--
 ip/ipl2tp.c  |  8 
 ip/iplink.c  | 28 +++-
 ip/iplink_vrf.c  | 24 
 ip/ipmacsec.c|  2 +-
 ip/ipneigh.c |  2 +-
 ip/ipnetns.c | 13 +++--
 ip/ipntable.c|  2 +-
 ip/iproute.c | 20 +++-
 ip/iprule.c  |  7 ---
 ip/ipseg6.c  |  7 ---
 ip/iptoken.c |  2 +-
 ip/link_gre.c|  7 ---
 ip/link_gre6.c   |  7 ---
 ip/link_ip6tnl.c |  7 ---
 ip/link_iptnl.c  |  7 ---
 ip/link_vti.c|  7 ---
 ip/link_vti6.c   |  7 ---
 ip/tcp_metrics.c |  7 ---
 ip/xfrm_policy.c | 22 +++---
 ip/xfrm_state.c  | 25 -
 lib/libgenl.c|  5 +++--
 lib/libnetlink.c | 20 +---
 misc/ss.c|  2 +-
 tc/m_action.c|  9 -
 tc/tc_class.c|  2 +-
 tc/tc_filter.c   |  7 ---
 tc/tc_qdisc.c|  2 +-
 37 files changed, 151 insertions(+), 152 deletions(-)

diff --git a/bridge/fdb.c b/bridge/fdb.c
index e5cebf9..807914f 100644
--- a/bridge/fdb.c
+++ b/bridge/fdb.c
@@ -535,7 +535,7 @@ static int fdb_modify(int cmd, int flags, int argc, char 
**argv)
return -1;
}
 
-   if (rtnl_talk(, , NULL, 0) < 0)
+   if (rtnl_talk(, , NULL) < 0)
return -1;
 
return 0;
diff --git a/bridge/link.c b/bridge/link.c
index 93472ad..cc29a2a 100644
--- a/bridge/link.c
+++ b/bridge/link.c
@@ -426,7 +426,7 @@ static int brlink_modify(int argc, char **argv)
addattr_nest_end(, nest);
}
 
-   if (rtnl_talk(, , NULL, 0) < 0)
+   if (rtnl_talk(, , NULL) < 0)
return -1;
 
return 0;
diff --git a/bridge/mdb.c b/bridge/mdb.c
index e60ff3e..fbd8184 100644
--- a/bridge/mdb.c
+++ b/bridge/mdb.c
@@ -298,7 +298,7 @@ static int mdb_modify(int cmd, int flags, int argc, char 
**argv)
entry.vid = vid;
addattr_l(, sizeof(req), MDBA_SET_ENTRY, , sizeof(entry));
 
-   if (rtnl_talk(, , NULL, 0) < 0)
+   if (rtnl_talk(, , NULL) < 0)
return -1;
 
return 0;
diff --git a/bridge/vlan.c b/bridge/vlan.c
index ebcdace..5d68359 100644
--- a/bridge/vlan.c
+++ b/bridge/vlan.c
@@ -133,7 +133,7 @@ static int vlan_modify(int cmd, int argc, char **argv)
 
addattr_nest_end(, afspec);
 
-   if (rtnl_talk(, , NULL, 0) < 0)
+   if (rtnl_talk(, , NULL) < 0)
return -1;
 
return 0;
diff --git a/genl/ctrl.c b/genl/ctrl.c
index 448988e..699657b 100644
--- a/genl/ctrl.c
+++ b/genl/ctrl.c
@@ -55,6 +55,7 @@ int genl_ctrl_resolve_family(const char *family)
};
struct nlmsghdr *nlh = 
struct genlmsghdr *ghdr = 
+   struct nlmsghdr *answer = NULL;
 
if (rtnl_open_byproto(, 0, NETLINK_GENERIC) < 0) {
fprintf(stderr, "Cannot open generic netlink socket\n");
@@ -63,19 +64,19 @@ int genl_ctrl_resolve_family(const char *family)
 
addattr_l(nlh, 128, CTRL_ATTR_FAMILY_NAME, family, strlen(family) + 1);
 
-   if (rtnl_talk(, nlh, nlh, sizeof(req)) < 0) {
+   if (rtnl_talk(, nlh, ) < 0) {
fprintf(stderr, "Error talking to the kernel\n");
goto errout;
}
 
{
struct rtattr *tb[CTRL_ATTR_MAX + 1];
-   int len = nlh->nlmsg_len;
+   int len = answer->nlmsg_len;
struct rtattr *attrs;
 
-   if (nlh->nlmsg_type !=  GENL_ID_CTRL) {
+   if (answer->nlmsg_type !=  GENL_ID_CTRL) {
fprintf(stderr, "Not a controller message, nlmsg_len=%d 
"
-   "nlmsg_type=0x%x\n", nlh->nlmsg_len, 
nlh->nlmsg_type);
+   "nlmsg_type=0x%x\n", answer->nlmsg_len, 
answer->nlmsg_type);
goto errout;
}
 
@@ -299,6 +300,7 @@ static int ctrl_list(int cmd, int argc, char **argv)
.g.cmd = CTRL_CMD_GETFAMILY,
};
struct nlmsghdr *nlh = 
+   struct nlmsghdr 

[PATCH iproute2 0/2] malloc correct buff at run time

[Hangbin Liu]

With commit 72b365e8e0fd ("libnetlink: Double the dump buffer size") and
460c03f3f3cc ("iplink: double the buffer size also in iplink_get()"), we
extend the buffer size to avoid truncated message with large numbers of
VFs. But just as Michal said, this is not future-proof since the NIC
number is increasing. We have customer even has 220+ VFs now.

This is not make sense to hard code the buffer and increase it all the time.
So let's just malloc the correct buff size at run time.

I'm not sure what init size would be suitable, so I keep use the original
size. I have tried with a small size like 1024, and it also works.

I tested with most ip cmds and all looks good.

Hangbin Liu (2):
  lib/libnetlink: re malloc buff if size is not enough
  lib/libnetlink: update rtnl_talk to support malloc buff at run time

 bridge/fdb.c |   2 +-
 bridge/link.c|   2 +-
 bridge/mdb.c |   2 +-
 bridge/vlan.c|   2 +-
 genl/ctrl.c  |  14 +++---
 include/libnetlink.h |   6 +--
 ip/ipaddress.c   |   5 ++-
 ip/ipaddrlabel.c |   4 +-
 ip/ipfou.c   |   4 +-
 ip/ipila.c   |   4 +-
 ip/ipl2tp.c  |   8 ++--
 ip/iplink.c  |  28 +---
 ip/iplink_vrf.c  |  24 ---
 ip/ipmacsec.c|   2 +-
 ip/ipneigh.c |   2 +-
 ip/ipnetns.c |  13 +++---
 ip/ipntable.c|   2 +-
 ip/iproute.c |  20 +
 ip/iprule.c  |   7 +--
 ip/ipseg6.c  |   7 +--
 ip/iptoken.c |   2 +-
 ip/link_gre.c|   7 +--
 ip/link_gre6.c   |   7 +--
 ip/link_ip6tnl.c |   7 +--
 ip/link_iptnl.c  |   7 +--
 ip/link_vti.c|   7 +--
 ip/link_vti6.c   |   7 +--
 ip/tcp_metrics.c |   7 +--
 ip/xfrm_policy.c |  22 +-
 ip/xfrm_state.c  |  25 ++-
 lib/libgenl.c|   5 ++-
 lib/libnetlink.c | 118 ---
 misc/ss.c|   2 +-
 tc/m_action.c|   9 ++--
 tc/tc_class.c|   2 +-
 tc/tc_filter.c   |   7 +--
 tc/tc_qdisc.c|   2 +-
 37 files changed, 217 insertions(+), 184 deletions(-)

-- 
2.5.5

[PATCH iproute2 1/2] lib/libnetlink: re malloc buff if size is not enough

[Hangbin Liu]

With commit 72b365e8e0fd ("libnetlink: Double the dump buffer size")
we doubled the buffer size to support more VFs. But the VFs number is
increasing all the time. Some customers even use more than 200 VFs now.

We could not double it everytime when the buffer is not enough. Let's just
not hard code the buffer size and malloc the correct number when running.

Introduce a new function rtnl_recvmsg() to get correct buffer.

Signed-off-by: Hangbin Liu 
---
 lib/libnetlink.c | 98 ++--
 1 file changed, 66 insertions(+), 32 deletions(-)

diff --git a/lib/libnetlink.c b/lib/libnetlink.c
index be7ac86..37cfb5a 100644
--- a/lib/libnetlink.c
+++ b/lib/libnetlink.c
@@ -402,6 +402,59 @@ static void rtnl_dump_error(const struct rtnl_handle *rth,
}
 }
 
+static int rtnl_recvmsg(int fd, struct msghdr *msg, char **buf)
+{
+   struct iovec *iov;
+   int len = -1, buf_len = 32768;
+   char *buffer = *buf;
+
+   int flag = MSG_PEEK | MSG_TRUNC;
+
+   if (buffer == NULL)
+re_malloc:
+   buffer = malloc(buf_len);
+
+   if (buffer == NULL) {
+   fprintf(stderr, "malloc error: no enough buffer\n");
+   return -1;
+   }
+
+   iov = msg->msg_iov;
+   iov->iov_base = buffer;
+   iov->iov_len = buf_len;
+
+re_recv:
+   len = recvmsg(fd, msg, flag);
+
+   if (len < 0) {
+   if (errno == EINTR || errno == EAGAIN)
+   return 0;
+   fprintf(stderr, "netlink receive error %s (%d)\n",
+   strerror(errno), errno);
+   return len;
+   }
+
+   if (len == 0) {
+   fprintf(stderr, "EOF on netlink\n");
+   return -1;
+   }
+
+   if (len > buf_len) {
+   free(buffer);
+   buf_len = len;
+   flag = 0;
+   goto re_malloc;
+   }
+
+   if (flag != 0) {
+   flag = 0;
+   goto re_recv;
+   }
+
+   *buf = buffer;
+   return len;
+}
+
 int rtnl_dump_filter_l(struct rtnl_handle *rth,
   const struct rtnl_dump_filter_arg *arg)
 {
@@ -413,31 +466,20 @@ int rtnl_dump_filter_l(struct rtnl_handle *rth,
.msg_iov = ,
.msg_iovlen = 1,
};
-   char buf[32768];
+   static char *buf = NULL;
int dump_intr = 0;
 
-   iov.iov_base = buf;
while (1) {
int status;
const struct rtnl_dump_filter_arg *a;
int found_done = 0;
int msglen = 0;
 
-   iov.iov_len = sizeof(buf);
-   status = recvmsg(rth->fd, , 0);
-
-   if (status < 0) {
-   if (errno == EINTR || errno == EAGAIN)
-   continue;
-   fprintf(stderr, "netlink receive error %s (%d)\n",
-   strerror(errno), errno);
-   return -1;
-   }
-
-   if (status == 0) {
-   fprintf(stderr, "EOF on netlink\n");
-   return -1;
-   }
+   status = rtnl_recvmsg(rth->fd, , );
+   if (status < 0)
+   return status;
+   else if (status == 0)
+   continue;
 
if (rth->dump_fp)
fwrite(buf, 1, NLMSG_ALIGN(status), rth->dump_fp);
@@ -543,7 +585,7 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct 
nlmsghdr *n,
.msg_iov = ,
.msg_iovlen = 1,
};
-   char   buf[32768] = {};
+   static char *buf = NULL;
 
n->nlmsg_seq = seq = ++rtnl->seq;
 
@@ -556,22 +598,14 @@ static int __rtnl_talk(struct rtnl_handle *rtnl, struct 
nlmsghdr *n,
return -1;
}
 
-   iov.iov_base = buf;
while (1) {
-   iov.iov_len = sizeof(buf);
-   status = recvmsg(rtnl->fd, , 0);
+   status = rtnl_recvmsg(rtnl->fd, , );
+
+   if (status < 0)
+   return status;
+   else if (status == 0)
+   continue;
 
-   if (status < 0) {
-   if (errno == EINTR || errno == EAGAIN)
-   continue;
-   fprintf(stderr, "netlink receive error %s (%d)\n",
-   strerror(errno), errno);
-   return -1;
-   }
-   if (status == 0) {
-   fprintf(stderr, "EOF on netlink\n");
-   return -1;
-   }
if (msg.msg_namelen != sizeof(nladdr)) {
fprintf(stderr,
"sender address length == %d\n",
-- 
2.5.5

Re: [PATCH RFC 1/6] The file ksz_common.c will be used by other KSZ switch drivers.

[Pavel Machek]

On Thu 2017-09-07 21:08:58, tristram...@microchip.com wrote:
> From: Tristram Ha 
> 
> Break ksz_common.c into 2 files so that the common code can be used by other 
> KSZ switch drivers.
> 
> Signed-off-by: Tristram Ha 
> ---
> diff --git a/drivers/net/dsa/microchip/Makefile 
> b/drivers/net/dsa/microchip/Makefile
> index ed335e2..0961c30 100644
> --- a/drivers/net/dsa/microchip/Makefile
> +++ b/drivers/net/dsa/microchip/Makefile
> @@ -1,2 +1,2 @@
> -obj-$(CONFIG_MICROCHIP_KSZ)  += ksz_common.o
> +obj-$(CONFIG_MICROCHIP_KSZ)  += ksz9477.o ksz_common.o
>  obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER)   += ksz_spi.o

I believe you should also rename option to CONFIG_MICROCHIP_KSZ_9477
here... and introduce appropriate Kconfig change.

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


signature.asc
Description: Digital signature

Re: [PATCH RFC 2/5] Add KSZ8795 switch driver support in Makefile

[Pavel Machek]

On Fri 2017-09-08 00:43:09, Andrew Lunn wrote:
> On Thu, Sep 07, 2017 at 10:29:34PM +, tristram...@microchip.com wrote:
> > > -Original Message-
> > > From: Andrew Lunn [mailto:and...@lunn.ch]
> > > Sent: Thursday, September 07, 2017 2:56 PM
> > > To: Tristram Ha - C24268
> > > Cc: muva...@gmail.com; pa...@ucw.cz; nathan.leigh.con...@gmail.com;
> > > vivien.dide...@savoirfairelinux.com; f.faine...@gmail.com;
> > > netdev@vger.kernel.org; linux-ker...@vger.kernel.org; Woojung Huh - C21699
> > > Subject: Re: [PATCH RFC 2/5] Add KSZ8795 switch driver support in Makefile
> > > 
> > > On Thu, Sep 07, 2017 at 09:17:10PM +, tristram...@microchip.com wrote:
> > > > From: Tristram Ha 
> > > >
> > > > Add KSZ8795 switch support with SPI access.
> > > >
> > > > Signed-off-by: Tristram Ha 
> > > > ---
> > > > diff --git a/drivers/net/dsa/microchip/Makefile
> > > b/drivers/net/dsa/microchip/Makefile
> > > > index 0961c30..0d8ba48 100644
> > > > --- a/drivers/net/dsa/microchip/Makefile
> > > > +++ b/drivers/net/dsa/microchip/Makefile
> > > > @@ -1,2 +1,4 @@
> > > >  obj-$(CONFIG_MICROCHIP_KSZ)+= ksz9477.o ksz_common.o
> > > >  obj-$(CONFIG_MICROCHIP_KSZ_SPI_DRIVER) += ksz_spi.o
> > > > +obj-$(CONFIG_MICROCHIP_KSZ8795)+= ksz8795.o 
> > > > ksz_common.o
> > > > +obj-$(CONFIG_MICROCHIP_KSZ8795_SPI_DRIVER) += ksz8795_spi.o
> > > 
> > > I've not tried it, but i think this breaks the build
> > > 
> > >  Andrew
> > 
> > So you would like to have all 5 patches in 1 patch file?
> 
> Or maybe this one last? Would that stop the build from breaking?

This and Kconfig change should come together.. and probably last.
Pavel

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


signature.asc
Description: Digital signature

[PATCH v2] rtl8xxxu: Don't printk raw binary if serial number is not burned in.

[Adam Borowski]

I assume that a blank efuse comes with all ones, thus I did not bother
recognizing other possible junk values.  This matches 100% of dongles
I've seen (a single Gembird 8192eu).

Signed-off-by: Adam Borowski 
---
v2: strncmp("goofy string") -> memchr_inv()

Stefano Brivio wrote:
> You might want to use memchr_inv():
>
>if (memchr_inv(efuse->serial, 0xff, 11))
>dev_info(>udev->dev, "Serial: %.11s\n", efuse->serial);
>...
>
> Mostly cosmetic though.

This looks much better, thanks!


 drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c | 5 -
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c 
b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
index 80fee699f58a..38b2ba1ac6f8 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_8192e.c
@@ -614,7 +614,10 @@ static int rtl8192eu_parse_efuse(struct rtl8xxxu_priv 
*priv)
 
dev_info(>udev->dev, "Vendor: %.7s\n", efuse->vendor_name);
dev_info(>udev->dev, "Product: %.11s\n", efuse->device_name);
-   dev_info(>udev->dev, "Serial: %.11s\n", efuse->serial);
+   if (memchr_inv(efuse->serial, 0xff, 11))
+   dev_info(>udev->dev, "Serial: %.11s\n", efuse->serial);
+   else
+   dev_info(>udev->dev, "Serial not available.\n");
 
if (rtl8xxxu_debug & RTL8XXXU_DEBUG_EFUSE) {
unsigned char *raw = priv->efuse_wifi.raw;
-- 
2.14.1

Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Corentin Labbe]

On Fri, Sep 08, 2017 at 09:25:38AM +0200, Maxime Ripard wrote:
> On Fri, Sep 08, 2017 at 09:11:51AM +0200, Corentin Labbe wrote:
> > This patch add documentation about the MDIO switch used on sun8i-h3-emac
> > for integrated PHY.
> > 
> > Signed-off-by: Corentin Labbe 
> > ---
> >  .../devicetree/bindings/net/dwmac-sun8i.txt| 127 
> > +++--
> >  1 file changed, 120 insertions(+), 7 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
> > b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> > index 725f3b187886..3fa0e54825ea 100644
> > --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> > +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> > @@ -39,7 +39,7 @@ Optional properties for the following compatibles:
> >  - allwinner,leds-active-low: EPHY LEDs are active low
> >  
> >  Required child node of emac:
> > -- mdio bus node: should be named mdio
> > +- mdio bus node: should be labelled mdio
> 
> labels do not end up in the final DT (while the names do) so why are
> you making this change?
> 

I misunderstood label/name.
Anyway, this contrainst should leave due to "snps,dwmac-mdio MDIOs are 
automatically registered"

> >  
> >  Required properties of the mdio node:
> >  - #address-cells: shall be 1
> > @@ -48,14 +48,28 @@ Required properties of the mdio node:
> >  The device node referenced by "phy" or "phy-handle" should be a child node
> >  of the mdio node. See phy.txt for the generic PHY bindings.
> >  
> > -Required properties of the phy node with the following compatibles:
> > +The following compatibles require an mdio-mux node called "mdio-mux":
> > +  - "allwinner,sun8i-h3-emac"
> > +  - "allwinner,sun8i-v3s-emac":
> > +Required properties for the mdio-mux node:
> > +  - compatible = "mdio-mux"
> > +  - one child mdio for the integrated mdio
> > +  - one child mdio for the external mdio if present (V3s have none)
> > +Required properties for the mdio-mux children node:
> > +  - reg: 0 for internal MDIO bus, 1 for external MDIO bus
> > +
> > +The following compatibles require a PHY node representing the integrated
> > +PHY, under the integrated MDIO bus node if an mdio-mux node is used:
> >- "allwinner,sun8i-h3-emac",
> >- "allwinner,sun8i-v3s-emac":
> > +
> > +Required properties of the integrated phy node:
> >  - clocks: a phandle to the reference clock for the EPHY
> >  - resets: a phandle to the reset control for the EPHY
> > +- phy-is-integrated
> > +- Should be a child of the integrated mdio
> 
> I'm not sure what you mean by that, you ask that it should (so not
> required?) be a child of the integrated mdio...
> 

I will change words to "must"

> >  
> > -Example:
> > -
> > +Example with integrated PHY:
> >  emac: ethernet@1c0b000 {
> > compatible = "allwinner,sun8i-h3-emac";
> > syscon = <>;
> > @@ -72,13 +86,112 @@ emac: ethernet@1c0b000 {
> > phy-handle = <_mii_phy>;
> > phy-mode = "mii";
> > allwinner,leds-active-low;
> > +
> > +   mdio0: mdio {
> 
> (You don't label it mdio here, unlike what was asked before)
> 
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +   compatible = "snps,dwmac-mdio";
> > +   };
> 
> I think Rob wanted that node gone?
> 

MDIO mux does not work without a parent MDIO, either gived by "parent-bus" or 
directly via mdio_mux_init() (like it is the case in dwmac-sun8i)

> > +   mdio-mux {
> > +   compatible = "mdio-mux";
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   int_mdio: mdio@1 {
> > +   reg = <0>;
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +   int_mii_phy: ethernet-phy@1 {
> > +   reg = <1>;
> > +   clocks = < CLK_BUS_EPHY>;
> > +   resets = < RST_BUS_EPHY>;
> > +   phy-is-integrated
> > +   };
> > +   };
> 
> ... And in your example it's a child of the mdio mux?
> 

So I confirm, integrated PHY must be a child of integrated MDIO (that must be a 
child of mdio-mux).
The example is good.

> > +   ext_mdio: mdio@0 {
> > +   reg = <1>;
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +   };
> > +   };
> > +};
> > +
> > +Example with external PHY:
> > +emac: ethernet@1c0b000 {
> > +   compatible = "allwinner,sun8i-h3-emac";
> > +   syscon = <>;
> > +   reg = <0x01c0b000 0x104>;
> > +   interrupts = ;
> > +   interrupt-names = "macirq";
> > +   resets = < RST_BUS_EMAC>;
> > +   reset-names = "stmmaceth";
> > +   clocks = < CLK_BUS_EMAC>;
> > +   clock-names = "stmmaceth";
> > +   #address-cells = <1>;
> > +   #size-cells = <0>;
> > +
> > +   phy-handle = <_rgmii_phy>;
> > +   phy-mode = "rgmii";
> > +   allwinner,leds-active-low;
> > +
> > +   mdio0: mdio {
> 

[PATCH net] ipv6: fix memory leak with multiple tables during netns destruction

[Sabrina Dubroca]

fib6_net_exit only frees the main and local tables. If another table was
created with fib6_alloc_table, we leak it when the netns is destroyed.

Fix this in the same way ip_fib_net_exit cleans up tables, by walking
through the whole hashtable of fib6_table's. We can get rid of the
special cases for local and main, since they're also part of the
hashtable.

Reproducer:
ip netns add x
ip -net x -6 rule add from 6003:1::/64 table 100
ip netns del x

Reported-by: Jianlin Shi 
Fixes: 58f09b78b730 ("[NETNS][IPV6] ip6_fib - make it per network namespace")
Signed-off-by: Sabrina Dubroca 
---
 net/ipv6/ip6_fib.c | 25 +++--
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/net/ipv6/ip6_fib.c b/net/ipv6/ip6_fib.c
index a3b5c163325f..8280172c806c 100644
--- a/net/ipv6/ip6_fib.c
+++ b/net/ipv6/ip6_fib.c
@@ -191,6 +191,12 @@ void rt6_free_pcpu(struct rt6_info *non_pcpu_rt)
 }
 EXPORT_SYMBOL_GPL(rt6_free_pcpu);
 
+static void fib6_free_table(struct fib6_table *table)
+{
+   inetpeer_invalidate_tree(>tb6_peers);
+   kfree(table);
+}
+
 static void fib6_link_table(struct net *net, struct fib6_table *tb)
 {
unsigned int h;
@@ -2022,15 +2028,22 @@ static int __net_init fib6_net_init(struct net *net)
 
 static void fib6_net_exit(struct net *net)
 {
+   unsigned int i;
+
rt6_ifdown(net, NULL);
del_timer_sync(>ipv6.ip6_fib_timer);
 
-#ifdef CONFIG_IPV6_MULTIPLE_TABLES
-   inetpeer_invalidate_tree(>ipv6.fib6_local_tbl->tb6_peers);
-   kfree(net->ipv6.fib6_local_tbl);
-#endif
-   inetpeer_invalidate_tree(>ipv6.fib6_main_tbl->tb6_peers);
-   kfree(net->ipv6.fib6_main_tbl);
+   for (i = 0; i < FIB_TABLE_HASHSZ; i++) {
+   struct hlist_head *head = >ipv6.fib_table_hash[i];
+   struct hlist_node *tmp;
+   struct fib6_table *tb;
+
+   hlist_for_each_entry_safe(tb, tmp, head, tb6_hlist) {
+   hlist_del(>tb6_hlist);
+   fib6_free_table(tb);
+   }
+   }
+
kfree(net->ipv6.fib_table_hash);
kfree(net->ipv6.rt6_stats);
fib6_notifier_exit(net);
-- 
2.14.1

Re: [PATCH RFC 5/5] Add KSZ8795 SPI driver

[Pavel Machek]

Hi!


> +static int ksz_spi_read(struct ksz_device *dev, u32 reg, u8 *data,
> + unsigned int len)
> +{
> + struct spi_device *spi = dev->priv;
> +
> + return ksz_spi_read_reg(spi, reg, data, len); }
> +
> +static int ksz_spi_read8(struct ksz_device *dev, u32 reg, u8 *val) {
> + return ksz_spi_read(dev, reg, val, 1); }
> +
> +static int ksz_spi_read16(struct ksz_device *dev, u32 reg, u16 *val) {
> + int ret = ksz_spi_read(dev, reg, (u8 *)val, 2);
> +
> + if (!ret)
> + *val = be16_to_cpu(*val);
> +
> + return ret;
> +}

> +static int ksz_spi_read32(struct ksz_device *dev, u32 reg, u32 *val) {
> + int ret = ksz_spi_read(dev, reg, (u8 *)val, 4);
> +
> + if (!ret)
> + *val = be32_to_cpu(*val);
> +
> + return ret;
> +}

Please format according to CodingStyle. (Not only this.)

And this will be common for more drivers. Can it go to a header file
and be included...?


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


signature.asc
Description: Digital signature

[PATCH v5 01/10] arm64: dts: allwinner: Restore EMAC changes

[Corentin Labbe]

This patch restore arm64 DT about dwmac-sun8i
This reverts commit 87e1f5e8bb4b ("arm64: dts: allwinner: Revert EMAC changes")

Signed-off-by: Corentin Labbe 
---
 .../boot/dts/allwinner/sun50i-a64-bananapi-m64.dts   | 16 
 .../boot/dts/allwinner/sun50i-a64-pine64-plus.dts| 15 +++
 arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts  | 17 +
 .../dts/allwinner/sun50i-a64-sopine-baseboard.dts| 16 
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi| 20 
 .../boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts | 17 +
 .../boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts| 17 +
 .../boot/dts/allwinner/sun50i-h5-orangepi-prime.dts  | 17 +
 8 files changed, 135 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts 
b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
index d347f52e27f6..45bdbfb96126 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-bananapi-m64.dts
@@ -51,6 +51,7 @@
compatible = "sinovoip,bananapi-m64", "allwinner,sun50i-a64";
 
aliases {
+   ethernet0 = 
serial0 = 
serial1 = 
};
@@ -69,6 +70,14 @@
status = "okay";
 };
 
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins>;
+   phy-mode = "rgmii";
+   phy-handle = <_rgmii_phy>;
+   status = "okay";
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins>;
@@ -79,6 +88,13 @@
bias-pull-up;
 };
 
+ {
+   ext_rgmii_phy: ethernet-phy@1 {
+   compatible = "ethernet-phy-ieee802.3-c22";
+   reg = <1>;
+   };
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts 
b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
index f82ccf332c0f..24f1aac366d6 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64-plus.dts
@@ -48,3 +48,18 @@
 
/* TODO: Camera, touchscreen, etc. */
 };
+
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins>;
+   phy-mode = "rgmii";
+   phy-handle = <_rgmii_phy>;
+   status = "okay";
+};
+
+ {
+   ext_rgmii_phy: ethernet-phy@1 {
+   compatible = "ethernet-phy-ieee802.3-c22";
+   reg = <1>;
+   };
+};
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts 
b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
index caf8b6fbe5e3..6f209bb10a2f 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-pine64.dts
@@ -51,6 +51,7 @@
compatible = "pine64,pine64", "allwinner,sun50i-a64";
 
aliases {
+   ethernet0 = 
serial0 = 
serial1 = 
serial2 = 
@@ -78,6 +79,15 @@
status = "okay";
 };
 
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins>;
+   phy-mode = "rmii";
+   phy-handle = <_rmii_phy1>;
+   status = "okay";
+
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins>;
@@ -88,6 +98,13 @@
bias-pull-up;
 };
 
+ {
+   ext_rmii_phy1: ethernet-phy@1 {
+   compatible = "ethernet-phy-ieee802.3-c22";
+   reg = <1>;
+   };
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts 
b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
index 17ccc12b58df..0eb2acedf8c3 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
@@ -53,6 +53,7 @@
 "allwinner,sun50i-a64";
 
aliases {
+   ethernet0 = 
serial0 = 
};
 
@@ -76,6 +77,21 @@
status = "okay";
 };
 
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins>;
+   phy-mode = "rgmii";
+   phy-handle = <_rgmii_phy>;
+   status = "okay";
+};
+
+ {
+   ext_rgmii_phy: ethernet-phy@1 {
+   compatible = "ethernet-phy-ieee802.3-c22";
+   reg = <1>;
+   };
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins>;
diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi 
b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 75e8d3182535..4dd9ffef0d80 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -461,6 +461,26 @@
#size-cells = <0>;
};
 
+   emac: ethernet@1c3 {
+   compatible = "allwinner,sun50i-a64-emac";
+   syscon = <>;
+   reg = 

[PATCH v5 00/10] net: stmmac: dwmac-sun8i: Handle integrated PHY

[Corentin Labbe]

Hello

The current way to find if the PHY is internal is to compare DT phy-mode
and emac_variant/internal_phy.
But it will negate a possible future SoC where an external PHY use the
same phy mode than the integrated one.

This patchs series adds a new way to find if the PHY is integrated, via
the phy-is-integrated DT property.

Since it exists both integrated and external ethernet-phy@1, they are merged in
the final DTB and so share all properties.
For avoiding this, and better represent the reality, we use a MDIO mux.

The first try was to create a new MDIO mux "mdio-mux-syscon".
mdio-mux-syscon working the same way than mdio-mux-mmioreg with the exception
that the register is used via syscon/regmap.
But this solution does not work for two reason:
- changing the MDIO selection need the reset of MAC which cannot be done by the
mdio-mux-syscon driver
- There were driver loading order problem:
- mdio-mux-syscon needing that stmmac register the parent MDIO
- stmmac needing that child MDIO was registered just after registering 
parent MDIO

So we cannot use any external MDIO-mux.

The final solution was to represent a mdio-mux and let the MAC handle all 
things.
Note that phy-is-integrated is still needed (even if we use a MDIO mux) since
some properties apply only on integrated PHY and we need to know the final MDIO
bus in mdio_mux_syscon_switch_fn().

Since DT bits was reverted in 4.13, this patch series include the revert of the 
revert.
So
- the first four patchs bring back DT/stmmac stuff that was in 4.13 (and 
reverted)
- fifth patch document how DT MDIO mux is implemented
- patch 6 and 7 modify DT
- patch 8, 9, 10 Modify stmmac according to the new bindings

I have let patch splited for easy review. (for seeing what's new)
But the final serie could have some patch squashed if someone want.
Like squashing patch and 2 and 5 (documentation)

Since DT worked well in 4.13, could it be targeted for 4.14 ?
If necessary I could split this serie in two:
- bring back A64/A83T (patchs 1, 2, 4, 7, 9)
- add MXIO-mux and H3 (patchs 3, 4, 5, 6, 8, 10)

Regards

Changes since v4:
- Update documentation for new bindings
- Added 4 patchs for bring back reverted stuff of 4.13
- dwmac-sun8i now handle mdio-mux
- MDIO use now compatible = "snps,dwmac-mdio";

Changes since v3:
- Added a patch for handling fixed-link
- Updated documentation

Changes since v2:
- Add a MDIO mux for creating distinction between integrated and external MDIO.
- phy-is-integrated is not set in dtsi.

Changes since v1:
- Dropped phy-is-integrated documentation patch since another same patch was 
already merged
- Moved phy-is-integrated from SoC dtsi to final board DT.


Corentin Labbe (10):
  arm64: dts: allwinner: Restore EMAC changes
  dt-bindings: net: Restore sun8i dwmac binding
  arm: dts: sunxi: Restore EMAC changes
  net: stmmac: sun8i: Restore the compatibles
  dt-bindings: net: dwmac-sun8i: update documentation about integrated
PHY
  ARM: dts: sunxi: h3/h5: represent the mdio switch used by
sun8i-h3-emac
  arm64: dts: allwinner: add snps,dwmac-mdio compatible to emac/mdio
  net: stmmac: dwmac-sun8i: choose internal PHY via phy-is-integrated
  net: stmmac: snps,dwmac-mdio MDIOs are automatically registered
  net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

 .../devicetree/bindings/net/dwmac-sun8i.txt| 197 +
 arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts  |   9 +
 arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts|  19 ++
 arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts  |   7 +
 arch/arm/boot/dts/sun8i-h3-orangepi-2.dts  |   8 +
 arch/arm/boot/dts/sun8i-h3-orangepi-one.dts|   8 +
 arch/arm/boot/dts/sun8i-h3-orangepi-pc-plus.dts|   5 +
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts |   8 +
 arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts   |  22 +++
 arch/arm/boot/dts/sun8i-h3-orangepi-plus2e.dts |  16 ++
 arch/arm/boot/dts/sunxi-h3-h5.dtsi |  46 +
 .../boot/dts/allwinner/sun50i-a64-bananapi-m64.dts |  16 ++
 .../boot/dts/allwinner/sun50i-a64-pine64-plus.dts  |  15 ++
 .../arm64/boot/dts/allwinner/sun50i-a64-pine64.dts |  17 ++
 .../dts/allwinner/sun50i-a64-sopine-baseboard.dts  |  16 ++
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi  |  21 +++
 .../boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts   |  17 ++
 .../boot/dts/allwinner/sun50i-h5-orangepi-pc2.dts  |  17 ++
 .../dts/allwinner/sun50i-h5-orangepi-prime.dts |  17 ++
 drivers/net/ethernet/stmicro/stmmac/Kconfig|   1 +
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c  | 140 ---
 .../net/ethernet/stmicro/stmmac/stmmac_platform.c  |   4 -
 22 files changed, 601 insertions(+), 25 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/dwmac-sun8i.txt

-- 
2.13.5

Re: [V2 PATCH net-next 1/2] xdp: implement xdp_redirect_map for generic XDP

[Jesper Dangaard Brouer]

On Thu, 07 Sep 2017 16:09:56 +0200
Daniel Borkmann  wrote:

> On 09/07/2017 02:33 PM, Jesper Dangaard Brouer wrote:
> > Using bpf_redirect_map is allowed for generic XDP programs, but the
> > appropriate map lookup was never performed in xdp_do_generic_redirect().
> >
> > Instead the map-index is directly used as the ifindex.  For the
> > xdp_redirect_map sample in SKB-mode '-S', this resulted in trying
> > sending on ifindex 0 which isn't valid, resulting in getting SKB
> > packets dropped.  Thus, the reported performance numbers are wrong in
> > commit 24251c264798 ("samples/bpf: add option for native and skb mode
> > for redirect apps") for the 'xdp_redirect_map -S' case.
> >
> > It might seem innocent this was lacking, but it can actually crash the
> > kernel.  The potential crash is caused by not consuming redirect_info->map.
> > The bpf_redirect_map helper will set this_cpu_ptr(_info)->map
> > pointer, which will survive even after unloading the xdp bpf_prog and
> > deallocating the devmap data-structure.  This leaves a dead map
> > pointer around.  The kernel will crash when loading the xdp_redirect
> > sample (in native XDP mode) as it doesn't reset map (via bpf_redirect)
> > and returns XDP_REDIRECT, which will cause it to dereference the map
> > pointer.
> >
> > Fixes: 6103aa96ec07 ("net: implement XDP_REDIRECT for xdp generic")
> > Fixes: 24251c264798 ("samples/bpf: add option for native and skb mode for 
> > redirect apps")
> > Signed-off-by: Jesper Dangaard Brouer 
> > ---
> >   include/trace/events/xdp.h |4 ++--
> >   net/core/filter.c  |   14 +++---
> >   2 files changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/trace/events/xdp.h b/include/trace/events/xdp.h
> > index 862575ac8da9..4e16c43fba10 100644
> > --- a/include/trace/events/xdp.h
> > +++ b/include/trace/events/xdp.h
> > @@ -138,11 +138,11 @@ DEFINE_EVENT_PRINT(xdp_redirect_template, 
> > xdp_redirect_map_err,
> >
> >   #define _trace_xdp_redirect_map(dev, xdp, fwd, map, idx)  \
> >  trace_xdp_redirect_map(dev, xdp, fwd ? fwd->ifindex : 0,   \
> > -   0, map, idx);
> > +   0, map, idx)
> >
> >   #define _trace_xdp_redirect_map_err(dev, xdp, fwd, map, idx, err) \
> >  trace_xdp_redirect_map_err(dev, xdp, fwd ? fwd->ifindex : 0,   \
> > -   err, map, idx);
> > +   err, map, idx)
> >
> >   #endif /* _TRACE_XDP_H */
> >
> > diff --git a/net/core/filter.c b/net/core/filter.c
> > index 5912c738a7b2..3767470cab6c 100644
> > --- a/net/core/filter.c
> > +++ b/net/core/filter.c
> > @@ -2566,13 +2566,19 @@ int xdp_do_generic_redirect(struct net_device *dev, 
> > struct sk_buff *skb,
> > struct bpf_prog *xdp_prog)
> >   {
> > struct redirect_info *ri = this_cpu_ptr(_info);
> > +   struct bpf_map *map = ri->map;
> > u32 index = ri->ifindex;
> > struct net_device *fwd;
> > unsigned int len;
> > int err = 0;
> >
> > -   fwd = dev_get_by_index_rcu(dev_net(dev), index);
> > ri->ifindex = 0;
> > +   ri->map = NULL;
> > +
> > +   if (map)
> > +   fwd = __dev_map_lookup_elem(map, index);
> > +   else
> > +   fwd = dev_get_by_index_rcu(dev_net(dev), index);
> > if (unlikely(!fwd)) {
> > err = -EINVAL;
> > goto err;
> > @@ -2590,10 +2596,12 @@ int xdp_do_generic_redirect(struct net_device *dev, 
> > struct sk_buff *skb,
> > }
> >
> > skb->dev = fwd;  
> 
> Looks much better above, thanks!
> 
> > -   _trace_xdp_redirect(dev, xdp_prog, index);
> > +   map ? _trace_xdp_redirect_map(dev, xdp_prog, fwd, map, index)
> > +   : _trace_xdp_redirect(dev, xdp_prog, index);  
> 
> Could we rather make this in a way such that when the two
> tracepoints are disabled and thus patched out, that we can
> also omit the extra conditional which has no purpose then?

First of all I don't think it make much of a difference, I measured the
impact of the full patch to "cost" 1.62 nanosec (which is arguably
below the accuracy level of the system under test)

Secondly, I plan to optimize the map case for generic XDP later, where
I would naturally split this into two functions (as V1, and as
native-XDP), thus this extra conditional would go away.  As I've shown
offlist (to you, John and Andy) I demonstrated a 24% speedup via a
xmit_more hack for generic XDP.


> Perhaps just a consolidated _trace_xdp_generic_redirect_map()
> would be better to avoid this altogether given we have twice
> the same anyway, here and in err path.

I do want separate tracepoints for xdp_redirect and xdp_redirect_map,
as it makes it more clear for users of the tracepoint (and attached
bpf_prog's can be faster, knowing the context).

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer

[PATCH net] phy: mvebu-cp110: checking for NULL instead of IS_ERR()

[Dan Carpenter]

devm_ioremap_resource() never returns NULL, it only returns error
pointers so this test needs to be changed.

Fixes: d0438bd6aa09 ("phy: add the mvebu cp110 comphy driver")
Signed-off-by: Dan Carpenter 
---
This driver apparently is going through the net tree, but netdev isn't
listed as handling it in MAINTAINERS.  Kishon, do you know what's up
with that?

diff --git a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c 
b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
index 73ebad6634a7..24578bd68ddc 100644
--- a/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
+++ b/drivers/phy/marvell/phy-mvebu-cp110-comphy.c
@@ -576,8 +576,8 @@ static int mvebu_comphy_probe(struct platform_device *pdev)
return PTR_ERR(priv->regmap);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
priv->base = devm_ioremap_resource(>dev, res);
-   if (!priv->base)
-   return -ENOMEM;
+   if (IS_ERR(priv->base))
+   return PTR_ERR(priv->base);
 
for_each_available_child_of_node(pdev->dev.of_node, child) {
struct mvebu_comphy_lane *lane;

Re: [PATCH v5 01/10] arm64: dts: allwinner: Restore EMAC changes

[Corentin Labbe]

On Fri, Sep 08, 2017 at 09:19:54AM +0200, Maxime Ripard wrote:
> On Fri, Sep 08, 2017 at 09:11:47AM +0200, Corentin Labbe wrote:
> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts 
> > b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
> > index 1c2387bd5df6..968908761194 100644
> > --- a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
> > @@ -50,6 +50,7 @@
> > compatible = "friendlyarm,nanopi-neo2", "allwinner,sun50i-h5";
> >  
> > aliases {
> > +   ethernet0 = 
> > serial0 = 
> > };
> >  
> > @@ -108,6 +109,22 @@
> > status = "okay";
> >  };
> >  
> > + {
> > +   pinctrl-names = "default";
> > +   pinctrl-0 = <_rgmii_pins>;
> > +   phy-supply = <_gmac_3v3>;
> > +   phy-handle = <_rgmii_phy>;
> > +   phy-mode = "rgmii";
> > +   status = "okay";
> > +};
> > +
> > + {
> > +   ext_rgmii_phy: ethernet-phy@7 {
> > +   compatible = "ethernet-phy-ieee802.3-c22";
> > +   reg = <7>;
> > +   };
> > +};
> > +
> 
> This won't compile, you don't have that node in the H5 DTSI.
> 

Since H5 DTSI include arm/sunxi-h3-h5.dtsi it compiles.
Furthermore, I restested just now and confirm, it compiles fine.

Regards

[PATCH v5 02/10] dt-bindings: net: Restore sun8i dwmac binding

[Corentin Labbe]

This patch restore dt-bindings documentation about dwmac-sun8i
This reverts commit 8aa33ec2f481 ("dt-bindings: net: Revert sun8i dwmac 
binding")

Signed-off-by: Corentin Labbe 
---
 .../devicetree/bindings/net/dwmac-sun8i.txt| 84 ++
 1 file changed, 84 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/dwmac-sun8i.txt

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
new file mode 100644
index ..725f3b187886
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -0,0 +1,84 @@
+* Allwinner sun8i GMAC ethernet controller
+
+This device is a platform glue layer for stmmac.
+Please see stmmac.txt for the other unchanged properties.
+
+Required properties:
+- compatible: should be one of the following string:
+   "allwinner,sun8i-a83t-emac"
+   "allwinner,sun8i-h3-emac"
+   "allwinner,sun8i-v3s-emac"
+   "allwinner,sun50i-a64-emac"
+- reg: address and length of the register for the device.
+- interrupts: interrupt for the device
+- interrupt-names: should be "macirq"
+- clocks: A phandle to the reference clock for this device
+- clock-names: should be "stmmaceth"
+- resets: A phandle to the reset control for this device
+- reset-names: should be "stmmaceth"
+- phy-mode: See ethernet.txt
+- phy-handle: See ethernet.txt
+- #address-cells: shall be 1
+- #size-cells: shall be 0
+- syscon: A phandle to the syscon of the SoC with one of the following
+ compatible string:
+  - allwinner,sun8i-h3-system-controller
+  - allwinner,sun8i-v3s-system-controller
+  - allwinner,sun50i-a64-system-controller
+  - allwinner,sun8i-a83t-system-controller
+
+Optional properties:
+- allwinner,tx-delay-ps: TX clock delay chain value in ps. Range value is 
0-700. Default is 0)
+- allwinner,rx-delay-ps: RX clock delay chain value in ps. Range value is 
0-3100. Default is 0)
+Both delay properties need to be a multiple of 100. They control the delay for
+external PHY.
+
+Optional properties for the following compatibles:
+  - "allwinner,sun8i-h3-emac",
+  - "allwinner,sun8i-v3s-emac":
+- allwinner,leds-active-low: EPHY LEDs are active low
+
+Required child node of emac:
+- mdio bus node: should be named mdio
+
+Required properties of the mdio node:
+- #address-cells: shall be 1
+- #size-cells: shall be 0
+
+The device node referenced by "phy" or "phy-handle" should be a child node
+of the mdio node. See phy.txt for the generic PHY bindings.
+
+Required properties of the phy node with the following compatibles:
+  - "allwinner,sun8i-h3-emac",
+  - "allwinner,sun8i-v3s-emac":
+- clocks: a phandle to the reference clock for the EPHY
+- resets: a phandle to the reset control for the EPHY
+
+Example:
+
+emac: ethernet@1c0b000 {
+   compatible = "allwinner,sun8i-h3-emac";
+   syscon = <>;
+   reg = <0x01c0b000 0x104>;
+   interrupts = ;
+   interrupt-names = "macirq";
+   resets = < RST_BUS_EMAC>;
+   reset-names = "stmmaceth";
+   clocks = < CLK_BUS_EMAC>;
+   clock-names = "stmmaceth";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   phy-handle = <_mii_phy>;
+   phy-mode = "mii";
+   allwinner,leds-active-low;
+   mdio: mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   int_mii_phy: ethernet-phy@1 {
+   reg = <1>;
+   clocks = < CLK_BUS_EPHY>;
+   resets = < RST_BUS_EPHY>;
+   };
+   };
+};
-- 
2.13.5

[PATCH v5 03/10] arm: dts: sunxi: Restore EMAC changes

[Corentin Labbe]

This patch restore arm DT about dwmac-sun8i
This reverts commit fe45174b72ae ("arm: dts: sunxi: Revert EMAC changes")

Signed-off-by: Corentin Labbe 
---
 arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts |  9 
 arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts   | 19 +
 arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts |  7 ++
 arch/arm/boot/dts/sun8i-h3-orangepi-2.dts |  8 +++
 arch/arm/boot/dts/sun8i-h3-orangepi-one.dts   |  8 +++
 arch/arm/boot/dts/sun8i-h3-orangepi-pc-plus.dts   |  5 +
 arch/arm/boot/dts/sun8i-h3-orangepi-pc.dts|  8 +++
 arch/arm/boot/dts/sun8i-h3-orangepi-plus.dts  | 22 +++
 arch/arm/boot/dts/sun8i-h3-orangepi-plus2e.dts| 16 ++
 arch/arm/boot/dts/sunxi-h3-h5.dtsi| 26 +++
 10 files changed, 128 insertions(+)

diff --git a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts 
b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
index b1502df7b509..6713d0f2b3f4 100644
--- a/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
+++ b/arch/arm/boot/dts/sun8i-h2-plus-orangepi-zero.dts
@@ -56,6 +56,8 @@
 
aliases {
serial0 = 
+   /* ethernet0 is the H3 emac, defined in sun8i-h3.dtsi */
+   ethernet0 = 
ethernet1 = 
};
 
@@ -102,6 +104,13 @@
status = "okay";
 };
 
+ {
+   phy-handle = <_mii_phy>;
+   phy-mode = "mii";
+   allwinner,leds-active-low;
+   status = "okay";
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins_a>;
diff --git a/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts 
b/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
index a337af1de322..d756ff825116 100644
--- a/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
+++ b/arch/arm/boot/dts/sun8i-h3-bananapi-m2-plus.dts
@@ -52,6 +52,7 @@
compatible = "sinovoip,bpi-m2-plus", "allwinner,sun8i-h3";
 
aliases {
+   ethernet0 = 
serial0 = 
serial1 = 
};
@@ -114,12 +115,30 @@
status = "okay";
 };
 
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_rgmii_pins>;
+   phy-supply = <_gmac_3v3>;
+   phy-handle = <_rgmii_phy>;
+   phy-mode = "rgmii";
+
+   allwinner,leds-active-low;
+   status = "okay";
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins_a>;
status = "okay";
 };
 
+ {
+   ext_rgmii_phy: ethernet-phy@1 {
+   compatible = "ethernet-phy-ieee802.3-c22";
+   reg = <0>;
+   };
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins_a>, <_cd_pin>;
diff --git a/arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts 
b/arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts
index 8d2cc6e9a03f..78f6c24952dd 100644
--- a/arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts
+++ b/arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts
@@ -46,3 +46,10 @@
model = "FriendlyARM NanoPi NEO";
compatible = "friendlyarm,nanopi-neo", "allwinner,sun8i-h3";
 };
+
+ {
+   phy-handle = <_mii_phy>;
+   phy-mode = "mii";
+   allwinner,leds-active-low;
+   status = "okay";
+};
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts 
b/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
index 8ff71b1bb45b..17cdeae19c6f 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-2.dts
@@ -54,6 +54,7 @@
aliases {
serial0 = 
/* ethernet0 is the H3 emac, defined in sun8i-h3.dtsi */
+   ethernet0 = 
ethernet1 = 
};
 
@@ -117,6 +118,13 @@
status = "okay";
 };
 
+ {
+   phy-handle = <_mii_phy>;
+   phy-mode = "mii";
+   allwinner,leds-active-low;
+   status = "okay";
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins_a>;
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts 
b/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
index 5fea430e0eb1..6880268e8b87 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-one.dts
@@ -52,6 +52,7 @@
compatible = "xunlong,orangepi-one", "allwinner,sun8i-h3";
 
aliases {
+   ethernet0 = 
serial0 = 
};
 
@@ -97,6 +98,13 @@
status = "okay";
 };
 
+ {
+   phy-handle = <_mii_phy>;
+   phy-mode = "mii";
+   allwinner,leds-active-low;
+   status = "okay";
+};
+
  {
pinctrl-names = "default";
pinctrl-0 = <_pins_a>, <_cd_pin>;
diff --git a/arch/arm/boot/dts/sun8i-h3-orangepi-pc-plus.dts 
b/arch/arm/boot/dts/sun8i-h3-orangepi-pc-plus.dts
index 8b93f5c781a7..a10281b455f5 100644
--- a/arch/arm/boot/dts/sun8i-h3-orangepi-pc-plus.dts
+++ b/arch/arm/boot/dts/sun8i-h3-orangepi-pc-plus.dts
@@ -53,6 +53,11 @@
};
 };
 
+ {
+   /* LEDs changed to active high on the plus */
+   /delete-property/ allwinner,leds-active-low;
+};
+
  {
 

[PATCH v5 04/10] net: stmmac: sun8i: Restore the compatibles

[Corentin Labbe]

This patch restore compatibles about dwmac-sun8i
This reverts commit ad4540cc5aa3 ("net: stmmac: sun8i: Remove the compatibles")

Signed-off-by: Corentin Labbe 
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 39c2122a4f26..fffd6d5fc907 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -979,6 +979,14 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id sun8i_dwmac_match[] = {
+   { .compatible = "allwinner,sun8i-h3-emac",
+   .data = _variant_h3 },
+   { .compatible = "allwinner,sun8i-v3s-emac",
+   .data = _variant_v3s },
+   { .compatible = "allwinner,sun8i-a83t-emac",
+   .data = _variant_a83t },
+   { .compatible = "allwinner,sun50i-a64-emac",
+   .data = _variant_a64 },
{ }
 };
 MODULE_DEVICE_TABLE(of, sun8i_dwmac_match);
-- 
2.13.5

[PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Corentin Labbe]

This patch add documentation about the MDIO switch used on sun8i-h3-emac
for integrated PHY.

Signed-off-by: Corentin Labbe 
---
 .../devicetree/bindings/net/dwmac-sun8i.txt| 127 +++--
 1 file changed, 120 insertions(+), 7 deletions(-)

diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
index 725f3b187886..3fa0e54825ea 100644
--- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
+++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
@@ -39,7 +39,7 @@ Optional properties for the following compatibles:
 - allwinner,leds-active-low: EPHY LEDs are active low
 
 Required child node of emac:
-- mdio bus node: should be named mdio
+- mdio bus node: should be labelled mdio
 
 Required properties of the mdio node:
 - #address-cells: shall be 1
@@ -48,14 +48,28 @@ Required properties of the mdio node:
 The device node referenced by "phy" or "phy-handle" should be a child node
 of the mdio node. See phy.txt for the generic PHY bindings.
 
-Required properties of the phy node with the following compatibles:
+The following compatibles require an mdio-mux node called "mdio-mux":
+  - "allwinner,sun8i-h3-emac"
+  - "allwinner,sun8i-v3s-emac":
+Required properties for the mdio-mux node:
+  - compatible = "mdio-mux"
+  - one child mdio for the integrated mdio
+  - one child mdio for the external mdio if present (V3s have none)
+Required properties for the mdio-mux children node:
+  - reg: 0 for internal MDIO bus, 1 for external MDIO bus
+
+The following compatibles require a PHY node representing the integrated
+PHY, under the integrated MDIO bus node if an mdio-mux node is used:
   - "allwinner,sun8i-h3-emac",
   - "allwinner,sun8i-v3s-emac":
+
+Required properties of the integrated phy node:
 - clocks: a phandle to the reference clock for the EPHY
 - resets: a phandle to the reset control for the EPHY
+- phy-is-integrated
+- Should be a child of the integrated mdio
 
-Example:
-
+Example with integrated PHY:
 emac: ethernet@1c0b000 {
compatible = "allwinner,sun8i-h3-emac";
syscon = <>;
@@ -72,13 +86,112 @@ emac: ethernet@1c0b000 {
phy-handle = <_mii_phy>;
phy-mode = "mii";
allwinner,leds-active-low;
+
+   mdio0: mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   compatible = "snps,dwmac-mdio";
+   };
+
+   mdio-mux {
+   compatible = "mdio-mux";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   int_mdio: mdio@1 {
+   reg = <0>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   int_mii_phy: ethernet-phy@1 {
+   reg = <1>;
+   clocks = < CLK_BUS_EPHY>;
+   resets = < RST_BUS_EPHY>;
+   phy-is-integrated
+   };
+   };
+   ext_mdio: mdio@0 {
+   reg = <1>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   };
+   };
+};
+
+Example with external PHY:
+emac: ethernet@1c0b000 {
+   compatible = "allwinner,sun8i-h3-emac";
+   syscon = <>;
+   reg = <0x01c0b000 0x104>;
+   interrupts = ;
+   interrupt-names = "macirq";
+   resets = < RST_BUS_EMAC>;
+   reset-names = "stmmaceth";
+   clocks = < CLK_BUS_EMAC>;
+   clock-names = "stmmaceth";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   phy-handle = <_rgmii_phy>;
+   phy-mode = "rgmii";
+   allwinner,leds-active-low;
+
+   mdio0: mdio {
+   #address-cells = <1>;
+   #size-cells = <0>;
+   compatible = "snps,dwmac-mdio";
+   };
+
+   mdio-mux {
+   compatible = "mdio-mux";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   int_mdio: mdio@1 {
+   reg = <0>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   int_mii_phy: ethernet-phy@1 {
+   reg = <1>;
+   clocks = < CLK_BUS_EPHY>;
+   resets = < RST_BUS_EPHY>;
+   phy-is-integrated
+   };
+   };
+   ext_mdio: mdio@0 {
+   reg = <1>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   ext_rgmii_phy: ethernet-phy@1 {
+   reg = <1>;
+   };
+   };
+   };
+};
+
+Example with SoC without integrated PHY
+
+emac: ethernet@1c0b000 {
+   compatible = "allwinner,sun8i-a83t-emac";
+   syscon = <>;
+   reg 

Re: [PATCH v5 01/10] arm64: dts: allwinner: Restore EMAC changes

[Chen-Yu Tsai]

On Fri, Sep 8, 2017 at 3:36 PM, Corentin Labbe
 wrote:
> On Fri, Sep 08, 2017 at 09:19:54AM +0200, Maxime Ripard wrote:
>> On Fri, Sep 08, 2017 at 09:11:47AM +0200, Corentin Labbe wrote:
>> > diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts 
>> > b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
>> > index 1c2387bd5df6..968908761194 100644
>> > --- a/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
>> > +++ b/arch/arm64/boot/dts/allwinner/sun50i-h5-nanopi-neo2.dts
>> > @@ -50,6 +50,7 @@
>> > compatible = "friendlyarm,nanopi-neo2", "allwinner,sun50i-h5";
>> >
>> > aliases {
>> > +   ethernet0 = 
>> > serial0 = 
>> > };
>> >
>> > @@ -108,6 +109,22 @@
>> > status = "okay";
>> >  };
>> >
>> > + {
>> > +   pinctrl-names = "default";
>> > +   pinctrl-0 = <_rgmii_pins>;
>> > +   phy-supply = <_gmac_3v3>;
>> > +   phy-handle = <_rgmii_phy>;
>> > +   phy-mode = "rgmii";
>> > +   status = "okay";
>> > +};
>> > +
>> > + {
>> > +   ext_rgmii_phy: ethernet-phy@7 {
>> > +   compatible = "ethernet-phy-ieee802.3-c22";
>> > +   reg = <7>;
>> > +   };
>> > +};
>> > +
>>
>> This won't compile, you don't have that node in the H5 DTSI.
>>
>
> Since H5 DTSI include arm/sunxi-h3-h5.dtsi it compiles.
> Furthermore, I restested just now and confirm, it compiles fine.

The order of your patches are wrong. No individual patch should
introduce build failures, not just the whole patch series.

ChenYu

Re: [PATCH RFC 0/6] Modify KSZ9477 DSA driver in preparation to add other KSZ switch drivers.

[Pavel Machek]

Hi!

> From: Tristram Ha 
> 
> This series of patches is to modify the original KSZ9477 DSA driver so that 
> other KSZ switch drivers can be added and use the common code.
> 

Please wrap the lines from time to time...


> This patch set is against net-next.
> 
>  drivers/net/dsa/microchip/Makefile |2 +-
>  drivers/net/dsa/microchip/ksz9477.c| 1317 
> 

We already have ksz_9477_reg.h. So should this be ksz_9477.c for consistency?

>  drivers/net/dsa/microchip/ksz_common.c | 1156 +---
>  drivers/net/dsa/microchip/ksz_priv.h   |  105 ++-
>  drivers/net/dsa/microchip/ksz_spi.c|   13 +-
>  net/dsa/tag_ksz.c  |   40 +-

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


signature.asc
Description: Digital signature

RE: [PATCH net] smsc95xx: Configure pause time to 0xffff when tx flow control enabled

[Nisar.Sayed]

> On Thu, Sep 07, 2017 at 06:51:37AM +, nisar.sa...@microchip.com
> wrote:
> > From: Nisar Sayed 
> >
> > Configure pause time to 0x when tx flow control enabled
> 
> Hi Nisar
> 
> You should explain the 'Why' in the commit message. Why do we want a
> pause time of 0x?
> 
>   Andrew

Thanks Andrew,

I shall update the description and submit next version.

- Nisar

RE: [PATCH 3/3] lan78xx: Use default value loaded from EEPROM/OTP when resetting

[Nisar.Sayed]

> On Thu, Sep 07, 2017 at 07:11:50AM +, nisar.sa...@microchip.com
> wrote:
> > From: Nisar Sayed 
> >
> > Use default value loaded from EEPROM/OTP when resetting
> 
> Hi Nisar
> 
> Subject: [PATCH 3/3]
> 
> Is this a fix for net, or further development for net-next?
> 
> Why do we want the default values?
> 
> Andrew

Thanks Andrew,

Yes it is for "net", sorry missed to include it, will update in next version of 
submit.

These bits are "reset protected" and should not be modified and must use the
Must be configured from EEPROM only. Will update the description.

- Nisar

Re: [PATCH RFC 4/5] Add KSZ8795 register definitions

[Pavel Machek]

On Thu 2017-09-07 21:17:27, tristram...@microchip.com wrote:
> From: Tristram Ha 
> 
> Add KSZ8795 switch support with register definitions.
> 
> Signed-off-by: Tristram Ha 
> ---
> diff --git a/drivers/net/dsa/microchip/ksz8795.h 
> b/drivers/net/dsa/microchip/ksz8795.h
> new file mode 100644
> index 000..005eda5
> --- /dev/null
> +++ b/drivers/net/dsa/microchip/ksz8795.h

We already have ksz_9477_reg.h. Please make it consistent.

> @@ -0,0 +1,1015 @@
> +/**
> + * Microchip KSZ8795 definition file
> + *
> + * Copyright (c) 2017 Microchip Technology Inc.
> + *   Tristram Ha 
> + *
> + * Permission to use, copy, modify, and/or distribute this software for any
> + * purpose with or without fee is hereby granted, provided that the above
> + * copyright notice and this permission notice appear in all copies.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
> + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
> + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
> + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
> + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
> + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
> + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> + */

-ENOTGPL.
Pavel

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


signature.asc
Description: Digital signature

Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs

[Nikolay Aleksandrov]

On 08/09/17 05:06, Kosuke Tatsukawa wrote:
> Hi,
> 
>> On  7.09.2017 01:47, Kosuke Tatsukawa wrote:
>>> Commit cbf5ecb30560 ("net: bonding: Fix transmit load balancing in
>>> balance-alb mode") tried to fix transmit dynamic load balancing in
>>> balance-alb mode, which wasn't working after commit 8b426dc54cf4
>>> ("bonding: remove hardcoded value").
>>>
>>> It turned out that my previous patch only fixed the case when
>>> balance-alb was specified as bonding module parameter, and not when
>>> balance-alb mode was set using /sys/class/net/*/bonding/mode (the most
>>> common usage).  In the latter case, tlb_dynamic_lb was set up according
>>> to the default mode of the bonding interface, which happens to be
>>> balance-rr.
>>>
>>> This additional patch addresses this issue by setting up tlb_dynamic_lb
>>> to 1 if "mode" is set to balance-alb through the sysfs interface.
>>>
>>> I didn't add code to change tlb_balance_lb back to the default value for
>>> other modes, because "mode" is usually set up only once during
>>> initialization, and it's not worthwhile to change the static variable
>>> bonding_defaults in bond_main.c to a global variable just for this
>>> purpose.
>>>
>>> Commit 8b426dc54cf4 also changes the value of tlb_dynamic_lb for
>>> balance-tlb mode if it is set up using the sysfs interface.  I didn't
>>> change that behavior, because the value of tlb_balance_lb can be changed
>>> using the sysfs interface for balance-tlb, and I didn't like changing
>>> the default value back and forth for balance-tlb.
>>>
>>> As for balance-alb, /sys/class/net/*/bonding/tlb_balance_lb cannot be
>>> written to.  However, I think balance-alb with tlb_dynamic_lb set to 0
>>> is not an intended usage, so there is little use making it writable at
>>> this moment.
>>>
>>> Fixes: 8b426dc54cf4 ("bonding: remove hardcoded value")
>>> Reported-by: Reinis Rozitis 
>>> Signed-off-by: Kosuke Tatsukawa 
>>> Cc: sta...@vger.kernel.org  # v4.12+
>>> ---
>>>  drivers/net/bonding/bond_options.c |3 +++
>>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>>
>>
>> I don't believe this to be the right solution, hardcoding it like this
>> changes user-visible behaviour. The issue is that if someone configured
>> it to be 0 in tlb mode, suddenly it will become 1 and will silently
>> override their config if they switch the mode to alb. Also it robs users
>> from their choice.
>>
>> If you think this should be settable in ALB mode, then IMO you should
>> edit tlb_dynamic_lb option's unsuppmodes and allow it to be set in ALB.
>> That would also be consistent with how it's handled in TLB mode.
> 
> No, I don't think tlb_dynamic_lb should be settable in balance-alb at
> this point.  All the current commits regarding tlb_dynamic_lb are for
> balance-tlb mode, so I don't think balance-alb with tlb_dynamic_lb set
> to 0 is an intended usage.
> 
> 
>> Going back and looking at your previous fix I'd argue that it is also
>> wrong, you should've removed the mode check altogether to return the
>> original behaviour where the dynamic_lb is set to 1 (enabled) by
>> default and then ALB mode would've had it, of course that would've left
>> the case of setting it to 0 in TLB mode and switching to ALB, but that
>> is a different issue.
> 
> Maybe balance-alb shouldn't be dependent on tlb_dynamic_lb.
> tlb_dynamic_lb is referenced in the following functions.
> 
>  + bond_do_alb_xmit()  -- Used by both balance-tlb and balance-alb
>  + bond_tlb_xmit()  -- Only used by balance-tlb
>  + bond_open()  -- Used by both balance-tlb and balance-alb
>  + bond_check_params()  -- Used during module initialization
>  + bond_fill_info()  -- Used to get/set value
>  + bond_option_tlb_dynamic_lb_set()  -- Used to get/set value
>  + bonding_show_tlb_dynamic_lb()  -- Used to get/set value
>  + bond_is_nondyn_tlb()  -- Only referenced if balance-tlb mode
> 
> The following untested patch adds code to make balance-alb work as if
> tlb_dynamic_lb==1 for the functions which affect balance-alb mode.  It
> also reverts my previous patch.
> 
> What do you think about this approach?
> ---
> Kosuke TATSUKAWA  | 1st Platform Software Division
>   | NEC Solution Innovators
>   | ta...@ab.jp.nec.com
> 

Logically the approach looks good, that being said it adds unnecessary tests in
the fast path, why not just something like the patch below ? That only leaves 
the
problem if it is zeroed in TLB and switched to ALB mode, and that is a one line
fix to unsuppmodes just allow it to be set for that specific case. The below
returns the default behaviour before the commit in your Fixes tag.


diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index fc63992ab0e0..c99dc59d729b 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4289,7 +4289,7 @@ static int bond_check_params(struct bond_params *params)
int bond_mode   = BOND_MODE_ROUNDROBIN;

[PATCH net] net: qualcomm: rmnet: Fix a double free

[Dan Carpenter]

This is called from rmnet_map_ingress_handler().  When the
rmnet_map_deaggregate() returns NULL then the caller calls
consume_skb(skb) which frees the skb.  The kfree_skb() on this error
path leads to a double free.

Fixes: ceed73a2cf4a ("drivers: net: ethernet: qualcomm: rmnet: Initial 
implementation")
Signed-off-by: Dan Carpenter 
---
This is from static analysis and not tested.

diff --git a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c 
b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
index 557c9bf1a469..0335fce54201 100644
--- a/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
+++ b/drivers/net/ethernet/qualcomm/rmnet/rmnet_map_data.c
@@ -95,10 +95,8 @@ struct sk_buff *rmnet_map_deaggregate(struct sk_buff *skb)
skb_pull(skb, packet_len);
 
/* Some hardware can send us empty frames. Catch them */
-   if (ntohs(maph->pkt_len) == 0) {
-   kfree_skb(skb);
+   if (ntohs(maph->pkt_len) == 0)
return NULL;
-   }
 
return skbn;
 }

[PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

[Corentin Labbe]

The Allwinner H3 SoC have two distinct MDIO bus, only one could be
active at the same time.
The selection of the active MDIO bus are done via some bits in the EMAC
register of the system controller.

This patch implement this MDIO switch via a custom MDIO-mux.

Signed-off-by: Corentin Labbe 
---
 drivers/net/ethernet/stmicro/stmmac/Kconfig   |   1 +
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 116 +++---
 2 files changed, 104 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig 
b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 97035766c291..e28c0d2c58e9 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -159,6 +159,7 @@ config DWMAC_SUN8I
tristate "Allwinner sun8i GMAC support"
default ARCH_SUNXI
depends on OF && (ARCH_SUNXI || COMPILE_TEST)
+   select MDIO_BUS_MUX
---help---
  Support for Allwinner H3 A83T A64 EMAC ethernet controllers.
 
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index 672553b652bd..ddd5695886ac 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -71,6 +72,7 @@ struct sunxi_priv_data {
const struct emac_variant *variant;
struct regmap *regmap;
bool use_internal_phy;
+   void *mux_handle;
 };
 
 static const struct emac_variant emac_variant_h3 = {
@@ -195,6 +197,9 @@ static const struct emac_variant emac_variant_a64 = {
 #define H3_EPHY_LED_POLBIT(17) /* 1: active low, 0: active 
high */
 #define H3_EPHY_SHUTDOWN   BIT(16) /* 1: shutdown, 0: power up */
 #define H3_EPHY_SELECT BIT(15) /* 1: internal PHY, 0: external PHY */
+#define H3_EPHY_MUX_MASK   (H3_EPHY_SHUTDOWN | H3_EPHY_SELECT)
+#define DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID   0
+#define DWMAC_sUN8I_MDIO_MUX_EXTERNAL_ID   1
 
 /* H3/A64 specific bits */
 #define SYSCON_RMII_EN BIT(13) /* 1: enable RMII (overrides EPIT) */
@@ -634,6 +639,76 @@ static int sun8i_dwmac_reset(struct stmmac_priv *priv)
return 0;
 }
 
+/* MDIO multiplexing switch function
+ * This function is called by the mdio-mux layer when it thinks the mdio bus
+ * multiplexer needs to switch.
+ * 'current_child' is the current value of the mux register
+ * 'desired_child' is the value of the 'reg' property of the target child MDIO
+ * node.
+ * The first time this function is called, current_child == -1.
+ * If current_child == desired_child, then the mux is already set to the
+ * correct bus.
+ *
+ * Note that we do not use reg/mask like mdio-mux-mmioreg because we need to
+ * know easily which bus is used (reset must be done only for desired bus).
+ */
+static int mdio_mux_syscon_switch_fn(int current_child, int desired_child,
+void *data)
+{
+   struct stmmac_priv *priv = data;
+   struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
+   u32 reg, val;
+   int ret = 0;
+   bool need_reset = false;
+
+   if (current_child ^ desired_child) {
+   regmap_read(gmac->regmap, SYSCON_EMAC_REG, );
+   switch (desired_child) {
+   case DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID:
+   dev_info(priv->device, "Switch mux to internal PHY");
+   val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SELECT;
+   if (gmac->use_internal_phy)
+   need_reset = true;
+   break;
+   case DWMAC_sUN8I_MDIO_MUX_EXTERNAL_ID:
+   dev_info(priv->device, "Switch mux to external PHY");
+   val = (reg & ~H3_EPHY_MUX_MASK) | H3_EPHY_SHUTDOWN;
+   if (!gmac->use_internal_phy)
+   need_reset = true;
+   break;
+   default:
+   dev_err(priv->device, "Invalid child id %x\n", 
desired_child);
+   return -EINVAL;
+   }
+   regmap_write(gmac->regmap, SYSCON_EMAC_REG, val);
+   /* After changing syscon value, the MAC need reset or it will 
use
+* the last value (and so the last PHY set).
+* Reset is necessary only when we reach the needed MDIO,
+* it timeout in other case.
+*/
+   if (need_reset)
+   ret = sun8i_dwmac_reset(priv);
+   else
+   dev_dbg(priv->device, "skipped reset\n");
+   }
+   return ret;
+}
+
+static int sun8i_dwmac_register_mdio_mux(struct stmmac_priv *priv)
+{
+   int ret;
+   struct device_node *mdio_mux;
+   struct sunxi_priv_data *gmac = 

[PATCH v5 08/10] net: stmmac: dwmac-sun8i: choose internal PHY via phy-is-integrated

[Corentin Labbe]

The current way to find if the phy is internal is to compare DT phy-mode
and emac_variant/internal_phy.
But it will negate a possible future SoC where an external PHY use the
same phy mode than the internal one.

This patch adds a new way to find if the PHY is internal, via
the phy-is-integrated property.

Since the internal_phy variable does not need anymore to contain the xMII mode
used by the internal PHY, it is still used for knowing the presence of an
internal PHY, so it is modified to a boolean soc_has_internal_phy.

Signed-off-by: Corentin Labbe 
Acked-by: Chen-Yu Tsai 
Reviewed-by: Florian Fainelli 
---
 drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 16 
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c 
b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
index fffd6d5fc907..672553b652bd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c
@@ -41,14 +41,14 @@
  * This value is used for disabling properly EMAC
  * and used as a good starting value in case of the
  * boot process(uboot) leave some stuff.
- * @internal_phy:  Does the MAC embed an internal PHY
+ * @soc_has_internal_phy:  Does the MAC embed an internal PHY
  * @support_mii:   Does the MAC handle MII
  * @support_rmii:  Does the MAC handle RMII
  * @support_rgmii: Does the MAC handle RGMII
  */
 struct emac_variant {
u32 default_syscon_value;
-   int internal_phy;
+   bool soc_has_internal_phy;
bool support_mii;
bool support_rmii;
bool support_rgmii;
@@ -75,7 +75,7 @@ struct sunxi_priv_data {
 
 static const struct emac_variant emac_variant_h3 = {
.default_syscon_value = 0x58000,
-   .internal_phy = PHY_INTERFACE_MODE_MII,
+   .soc_has_internal_phy = true,
.support_mii = true,
.support_rmii = true,
.support_rgmii = true
@@ -83,20 +83,20 @@ static const struct emac_variant emac_variant_h3 = {
 
 static const struct emac_variant emac_variant_v3s = {
.default_syscon_value = 0x38000,
-   .internal_phy = PHY_INTERFACE_MODE_MII,
+   .soc_has_internal_phy = true,
.support_mii = true
 };
 
 static const struct emac_variant emac_variant_a83t = {
.default_syscon_value = 0,
-   .internal_phy = 0,
+   .soc_has_internal_phy = false,
.support_mii = true,
.support_rgmii = true
 };
 
 static const struct emac_variant emac_variant_a64 = {
.default_syscon_value = 0,
-   .internal_phy = 0,
+   .soc_has_internal_phy = false,
.support_mii = true,
.support_rmii = true,
.support_rgmii = true
@@ -648,7 +648,7 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv)
 "Current syscon value is not the default %x (expect 
%x)\n",
 val, reg);
 
-   if (gmac->variant->internal_phy) {
+   if (gmac->variant->soc_has_internal_phy) {
if (!gmac->use_internal_phy) {
/* switch to external PHY interface */
reg &= ~H3_EPHY_SELECT;
@@ -932,7 +932,7 @@ static int sun8i_dwmac_probe(struct platform_device *pdev)
}
 
plat_dat->interface = of_get_phy_mode(dev->of_node);
-   if (plat_dat->interface == gmac->variant->internal_phy) {
+   if (of_property_read_bool(plat_dat->phy_node, "phy-is-integrated")) {
dev_info(>dev, "Will use internal PHY\n");
gmac->use_internal_phy = true;
gmac->ephy_clk = of_clk_get(plat_dat->phy_node, 0);
-- 
2.13.5

[PATCH v5 07/10] arm64: dts: allwinner: add snps,dwmac-mdio compatible to emac/mdio

[Corentin Labbe]

stmmac bindings docs said that its mdio node must have
compatible = "snps,dwmac-mdio";
Since dwmac-sun8i does not have any good reasons to not doing it, all
their MDIO node must have it.

Signed-off-by: Corentin Labbe 
---
 arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi 
b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
index 4dd9ffef0d80..5dceebd81f09 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi
@@ -476,6 +476,7 @@
#size-cells = <0>;
 
mdio: mdio {
+   compatible = "snps,dwmac-mdio";
#address-cells = <1>;
#size-cells = <0>;
};
-- 
2.13.5

[PATCH v5 06/10] ARM: dts: sunxi: h3/h5: represent the mdio switch used by sun8i-h3-emac

[Corentin Labbe]

Since dwmac-sun8i could use either an integrated PHY or an external PHY
(which could be at same MDIO address), we need to represent this selection
by a MDIO switch.

Signed-off-by: Corentin Labbe 
---
 arch/arm/boot/dts/sunxi-h3-h5.dtsi | 30 +-
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi 
b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 4b599b5d26f6..e137377b312d 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -417,14 +417,34 @@
#size-cells = <0>;
status = "disabled";
 
-   mdio: mdio {
+   mdio0: mdio {
#address-cells = <1>;
#size-cells = <0>;
-   int_mii_phy: ethernet-phy@1 {
-   compatible = 
"ethernet-phy-ieee802.3-c22";
+   compatible = "snps,dwmac-mdio";
+   };
+
+   mdio-mux {
+   compatible = "mdio-mux";
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   /* Only one MDIO is usable at the time */
+   internal_mdio: mdio@1 {
+   reg = <0>;
+   #address-cells = <1>;
+   #size-cells = <0>;
+   int_mii_phy: ethernet-phy@1 {
+   compatible = 
"ethernet-phy-ieee802.3-c22";
+   reg = <1>;
+   clocks = < CLK_BUS_EPHY>;
+   resets = < RST_BUS_EPHY>;
+   phy-is-integrated;
+   };
+   };
+   mdio: mdio@0 {
reg = <1>;
-   clocks = < CLK_BUS_EPHY>;
-   resets = < RST_BUS_EPHY>;
+   #address-cells = <1>;
+   #size-cells = <0>;
};
};
};
-- 
2.13.5

[PATCH v5 09/10] net: stmmac: snps,dwmac-mdio MDIOs are automatically registered

[Corentin Labbe]

stmmac bindings docs said that its mdio node must have
compatible = "snps,dwmac-mdio";
Since dwmac-sun8i does not have any good reasons to not doing it, all
their MDIO node must have it.

Since these compatible is automatically registered, dwmac-sun8i compatible
does not need to be in need_mdio_ids.

Signed-off-by: Corentin Labbe 
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 4 
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c 
b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index a366b3747eeb..3de5501e34fe 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -311,10 +311,6 @@ static int stmmac_dt_phy(struct plat_stmmacenet_data *plat,
bool mdio = true;
static const struct of_device_id need_mdio_ids[] = {
{ .compatible = "snps,dwc-qos-ethernet-4.10" },
-   { .compatible = "allwinner,sun8i-a83t-emac" },
-   { .compatible = "allwinner,sun8i-h3-emac" },
-   { .compatible = "allwinner,sun8i-v3s-emac" },
-   { .compatible = "allwinner,sun50i-a64-emac" },
};
 
/* If phy-handle property is passed from DT, use it as the PHY */
-- 
2.13.5

Re: [Intel-wired-lan] [PATCH] e1000e: changed some expensive calls of udelay to usleep_range

[Pavel Machek]

On Thu 2017-09-07 22:19:47, Brown, Aaron F wrote:
> > From: Intel-wired-lan [mailto:intel-wired-lan-boun...@osuosl.org] On
> > Behalf Of Pavel Machek
> > Sent: Monday, September 4, 2017 9:26 AM
> > To: Matthew Tan 
> > Cc: michael.kardo...@nxp.com; Williams, Mitch A
> > ; linux-ker...@vger.kernel.org;
> > john.ronc...@intel.com; intel-wired-...@lists.osuosl.org;
> > netdev@vger.kernel.org
> > Subject: Re: [Intel-wired-lan] [PATCH] e1000e: changed some expensive calls
> > of udelay to usleep_range
> > 
> > Hi!
> > 
> > > @@ -183,7 +183,7 @@ s32 e1000e_read_phy_reg_mdic(struct e1000_hw
> > *hw, u32 offset, u16 *data)
> > >* reading duplicate data in the next MDIC transaction.
> > >*/
> > >   if (hw->mac.type == e1000_pch2lan)
> > > - udelay(100);
> > > + usleep_range(90, 100);
> > >
> > >   return 0;
> > >  }
> > 
> > Can you explain why shortening the delay is acceptable here?
> 
> Maybe it's not.
> 
> This patch is causing speed / duplex tests to fail on several of my test 
> systems.  Specifically a Lenova laptop with an 82577 and a NUC with an i218 
> (though that does not mean it is limited to those or that it's not related to 
> the individual link partner.)
>

Ok, this should be quite easy to verify -- just adjust all the ranges
to be >= original ones.

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


signature.asc
Description: Digital signature

Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs

[Nikolay Aleksandrov]

On 08/09/17 13:10, Nikolay Aleksandrov wrote:
> On 08/09/17 05:06, Kosuke Tatsukawa wrote:
>> Hi,
>>
>>> On  7.09.2017 01:47, Kosuke Tatsukawa wrote:
 Commit cbf5ecb30560 ("net: bonding: Fix transmit load balancing in
 balance-alb mode") tried to fix transmit dynamic load balancing in
 balance-alb mode, which wasn't working after commit 8b426dc54cf4
 ("bonding: remove hardcoded value").

 It turned out that my previous patch only fixed the case when
 balance-alb was specified as bonding module parameter, and not when
 balance-alb mode was set using /sys/class/net/*/bonding/mode (the most
 common usage).  In the latter case, tlb_dynamic_lb was set up according
 to the default mode of the bonding interface, which happens to be
 balance-rr.

 This additional patch addresses this issue by setting up tlb_dynamic_lb
 to 1 if "mode" is set to balance-alb through the sysfs interface.

 I didn't add code to change tlb_balance_lb back to the default value for
 other modes, because "mode" is usually set up only once during
 initialization, and it's not worthwhile to change the static variable
 bonding_defaults in bond_main.c to a global variable just for this
 purpose.

 Commit 8b426dc54cf4 also changes the value of tlb_dynamic_lb for
 balance-tlb mode if it is set up using the sysfs interface.  I didn't
 change that behavior, because the value of tlb_balance_lb can be changed
 using the sysfs interface for balance-tlb, and I didn't like changing
 the default value back and forth for balance-tlb.

 As for balance-alb, /sys/class/net/*/bonding/tlb_balance_lb cannot be
 written to.  However, I think balance-alb with tlb_dynamic_lb set to 0
 is not an intended usage, so there is little use making it writable at
 this moment.

 Fixes: 8b426dc54cf4 ("bonding: remove hardcoded value")
 Reported-by: Reinis Rozitis 
 Signed-off-by: Kosuke Tatsukawa 
 Cc: sta...@vger.kernel.org  # v4.12+
 ---
  drivers/net/bonding/bond_options.c |3 +++
  1 files changed, 3 insertions(+), 0 deletions(-)

>>>
>>> I don't believe this to be the right solution, hardcoding it like this
>>> changes user-visible behaviour. The issue is that if someone configured
>>> it to be 0 in tlb mode, suddenly it will become 1 and will silently
>>> override their config if they switch the mode to alb. Also it robs users
>>> from their choice.
>>>
>>> If you think this should be settable in ALB mode, then IMO you should
>>> edit tlb_dynamic_lb option's unsuppmodes and allow it to be set in ALB.
>>> That would also be consistent with how it's handled in TLB mode.
>>
>> No, I don't think tlb_dynamic_lb should be settable in balance-alb at
>> this point.  All the current commits regarding tlb_dynamic_lb are for
>> balance-tlb mode, so I don't think balance-alb with tlb_dynamic_lb set
>> to 0 is an intended usage.
>>
>>
>>> Going back and looking at your previous fix I'd argue that it is also
>>> wrong, you should've removed the mode check altogether to return the
>>> original behaviour where the dynamic_lb is set to 1 (enabled) by
>>> default and then ALB mode would've had it, of course that would've left
>>> the case of setting it to 0 in TLB mode and switching to ALB, but that
>>> is a different issue.
>>
>> Maybe balance-alb shouldn't be dependent on tlb_dynamic_lb.
>> tlb_dynamic_lb is referenced in the following functions.
>>
>>  + bond_do_alb_xmit()  -- Used by both balance-tlb and balance-alb
>>  + bond_tlb_xmit()  -- Only used by balance-tlb
>>  + bond_open()  -- Used by both balance-tlb and balance-alb
>>  + bond_check_params()  -- Used during module initialization
>>  + bond_fill_info()  -- Used to get/set value
>>  + bond_option_tlb_dynamic_lb_set()  -- Used to get/set value
>>  + bonding_show_tlb_dynamic_lb()  -- Used to get/set value
>>  + bond_is_nondyn_tlb()  -- Only referenced if balance-tlb mode
>>
>> The following untested patch adds code to make balance-alb work as if
>> tlb_dynamic_lb==1 for the functions which affect balance-alb mode.  It
>> also reverts my previous patch.
>>
>> What do you think about this approach?
>> ---
>> Kosuke TATSUKAWA  | 1st Platform Software Division
>>   | NEC Solution Innovators
>>   | ta...@ab.jp.nec.com
>>
> 
> Logically the approach looks good, that being said it adds unnecessary tests 
> in
> the fast path, why not just something like the patch below ? That only leaves 
> the
> problem if it is zeroed in TLB and switched to ALB mode, and that is a one 
> line
> fix to unsuppmodes just allow it to be set for that specific case. The below
> returns the default behaviour before the commit in your Fixes tag.
> 
> 

Actually I'm fine with your approach, too. It will fix this regardless of the
value of tlb_dynamic_lb which sounds good to me for the price of a test in
the fast path.

Re: [PATCH net] bpf: don't select potentially stale ri->map from buggy xdp progs

[Daniel Borkmann]

On 09/08/2017 07:06 AM, Jesper Dangaard Brouer wrote:

On Fri,  8 Sep 2017 00:14:51 +0200
Daniel Borkmann  wrote:


+   /* This is really only caused by a deliberately crappy
+* BPF program, normally we would never hit that case,
+* so no need to inform someone via tracepoints either,
+* just bail out.
+*/
+   if (unlikely(map_owner != xdp_prog))
+   return -EINVAL;


IMHO we do need to call the tracepoint here.  It is not just crappy
BPF-progs that cause this situation, it is also drivers not implementing
XDP_REDIRECT yet (which is all but ixgbe).  Due to the level XDP
operates at, tracepoints are the only way users can runtime troubleshoot
their XDP programs.


Drivers not implementing XDP_REDIRECT don't even get there in
the first place. What they will do is to hit the 'default' case
when they check for the action code from the BPF program. Then
call into bpf_warn_invalid_xdp_action(act), and fall-through
to hit the tracepoint at trace_xdp_exception() which is also
triggered by XDP_ABORTED usually. So when that happens we do
complain loudly and call a tracepoint already. We should probably
tweak the bpf_warn_invalid_xdp_action() message a little to make
it clear that the action could also just be unsupported by the
driver instead of being illegal.

Re: [PATCH v5 05/10] dt-bindings: net: dwmac-sun8i: update documentation about integrated PHY

[Maxime Ripard]

On Fri, Sep 08, 2017 at 09:11:51AM +0200, Corentin Labbe wrote:
> This patch add documentation about the MDIO switch used on sun8i-h3-emac
> for integrated PHY.
> 
> Signed-off-by: Corentin Labbe 
> ---
>  .../devicetree/bindings/net/dwmac-sun8i.txt| 127 
> +++--
>  1 file changed, 120 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt 
> b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> index 725f3b187886..3fa0e54825ea 100644
> --- a/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> +++ b/Documentation/devicetree/bindings/net/dwmac-sun8i.txt
> @@ -39,7 +39,7 @@ Optional properties for the following compatibles:
>  - allwinner,leds-active-low: EPHY LEDs are active low
>  
>  Required child node of emac:
> -- mdio bus node: should be named mdio
> +- mdio bus node: should be labelled mdio

labels do not end up in the final DT (while the names do) so why are
you making this change?

>  
>  Required properties of the mdio node:
>  - #address-cells: shall be 1
> @@ -48,14 +48,28 @@ Required properties of the mdio node:
>  The device node referenced by "phy" or "phy-handle" should be a child node
>  of the mdio node. See phy.txt for the generic PHY bindings.
>  
> -Required properties of the phy node with the following compatibles:
> +The following compatibles require an mdio-mux node called "mdio-mux":
> +  - "allwinner,sun8i-h3-emac"
> +  - "allwinner,sun8i-v3s-emac":
> +Required properties for the mdio-mux node:
> +  - compatible = "mdio-mux"
> +  - one child mdio for the integrated mdio
> +  - one child mdio for the external mdio if present (V3s have none)
> +Required properties for the mdio-mux children node:
> +  - reg: 0 for internal MDIO bus, 1 for external MDIO bus
> +
> +The following compatibles require a PHY node representing the integrated
> +PHY, under the integrated MDIO bus node if an mdio-mux node is used:
>- "allwinner,sun8i-h3-emac",
>- "allwinner,sun8i-v3s-emac":
> +
> +Required properties of the integrated phy node:
>  - clocks: a phandle to the reference clock for the EPHY
>  - resets: a phandle to the reset control for the EPHY
> +- phy-is-integrated
> +- Should be a child of the integrated mdio

I'm not sure what you mean by that, you ask that it should (so not
required?) be a child of the integrated mdio...

>  
> -Example:
> -
> +Example with integrated PHY:
>  emac: ethernet@1c0b000 {
>   compatible = "allwinner,sun8i-h3-emac";
>   syscon = <>;
> @@ -72,13 +86,112 @@ emac: ethernet@1c0b000 {
>   phy-handle = <_mii_phy>;
>   phy-mode = "mii";
>   allwinner,leds-active-low;
> +
> + mdio0: mdio {

(You don't label it mdio here, unlike what was asked before)

> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "snps,dwmac-mdio";
> + };

I think Rob wanted that node gone?

> + mdio-mux {
> + compatible = "mdio-mux";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + int_mdio: mdio@1 {
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + int_mii_phy: ethernet-phy@1 {
> + reg = <1>;
> + clocks = < CLK_BUS_EPHY>;
> + resets = < RST_BUS_EPHY>;
> + phy-is-integrated
> + };
> + };

... And in your example it's a child of the mdio mux?

> + ext_mdio: mdio@0 {
> + reg = <1>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + };
> + };
> +};
> +
> +Example with external PHY:
> +emac: ethernet@1c0b000 {
> + compatible = "allwinner,sun8i-h3-emac";
> + syscon = <>;
> + reg = <0x01c0b000 0x104>;
> + interrupts = ;
> + interrupt-names = "macirq";
> + resets = < RST_BUS_EMAC>;
> + reset-names = "stmmaceth";
> + clocks = < CLK_BUS_EMAC>;
> + clock-names = "stmmaceth";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + phy-handle = <_rgmii_phy>;
> + phy-mode = "rgmii";
> + allwinner,leds-active-low;
> +
> + mdio0: mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + compatible = "snps,dwmac-mdio";
> + };
> +
> + mdio-mux {
> + compatible = "mdio-mux";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + int_mdio: mdio@1 {
> + reg = <0>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + int_mii_phy: ethernet-phy@1 {
> + reg = <1>;
> + clocks = < CLK_BUS_EPHY>;
> + resets = < RST_BUS_EPHY>;
> +

Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

[Corentin Labbe]

On Fri, Sep 08, 2017 at 04:00:20PM +0200, Andrew Lunn wrote:
> > > > +static int mdio_mux_syscon_switch_fn(int current_child, int 
> > > > desired_child,
> > > > +void *data)
> > > > +{
> > > > +   struct stmmac_priv *priv = data;
> > > > +   struct sunxi_priv_data *gmac = priv->plat->bsp_priv;
> > > > +   u32 reg, val;
> > > > +   int ret = 0;
> > > > +   bool need_reset = false;
> > > > +
> > > > +   if (current_child ^ desired_child) {
> > > > +   regmap_read(gmac->regmap, SYSCON_EMAC_REG, );
> > > > +   switch (desired_child) {
> > > > +   case DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID:
> > > > +   dev_info(priv->device, "Switch mux to internal 
> > > > PHY");
> > > > +   val = (reg & ~H3_EPHY_MUX_MASK) | 
> > > > H3_EPHY_SELECT;
> > > > +   if (gmac->use_internal_phy)
> > > > +   need_reset = true;
> > > > +   break;
> > > 
> > > This i don't get. Why do you need use_internal_phy? Isn't that
> > > implicit from DWMAC_sUN8I_MDIO_MUX_INTERNAL_ID? Is it even possible to
> > > use an external PHY on the internal MDIO bus?
> > > 
> > 
> > On my H3 box with external PHY, the MDIO mux library first select (for scan 
> > ?) the internal MDIO.
> > Without use_internal_phy usage, this board will launch a reset to use the 
> > internal MDIO... and this reset timeout/fail.
> 
> Do you know why the reset times out/fails?
> 

Because there are nothing connected to it.
I got also reset timeout on integrated MDIO when the integrated PHY is not 
powered.

[PATCH net] sctp: fix missing wake ups in some situations

[Marcelo Ricardo Leitner]

Commit fb586f25300f ("sctp: delay calls to sk_data_ready() as much as
possible") minimized the number of wake ups that are triggered in case
the association receives a packet with multiple data chunks on it and/or
when io_events are enabled and then commit 0970f5b36659 ("sctp: signal
sk_data_ready earlier on data chunks reception") moved the wake up to as
soon as possible. It thus relies on the state machine running later to
clean the flag that the event was already generated.

The issue is that there are 2 call paths that calls
sctp_ulpq_tail_event() outside of the state machine, causing the flag to
linger and possibly omitting a needed wake up in the sequence.

One of the call paths is when enabling SCTP_SENDER_DRY_EVENTS via
setsockopt(SCTP_EVENTS), as noticed by Harald Welte. The other is when
partial reliability triggers removal of chunks from the send queue when
the application calls sendmsg().

This commit fixes it by not setting the flag in case the socket is not
owned by the user, as it won't be cleaned later. This works for
user-initiated calls and also for rx path processing.

Fixes: fb586f25300f ("sctp: delay calls to sk_data_ready() as much as possible")
Reported-by: Harald Welte 
Signed-off-by: Marcelo Ricardo Leitner 
---

Hi. Please consider this one for -stable. Thanks

 net/sctp/ulpqueue.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/sctp/ulpqueue.c b/net/sctp/ulpqueue.c
index 
0225d62a869f8deff10565c4625df0a10464ce87..a71be33f3afeb0aaaef174ee082c4c547aab1e2d
 100644
--- a/net/sctp/ulpqueue.c
+++ b/net/sctp/ulpqueue.c
@@ -265,7 +265,8 @@ int sctp_ulpq_tail_event(struct sctp_ulpq *ulpq, struct 
sctp_ulpevent *event)
sctp_ulpq_clear_pd(ulpq);
 
if (queue == >sk_receive_queue && !sp->data_ready_signalled) {
-   sp->data_ready_signalled = 1;
+   if (!sock_owned_by_user(sk))
+   sp->data_ready_signalled = 1;
sk->sk_data_ready(sk);
}
return 1;
-- 
2.13.5

IFA_F_OPTIMISTIC is not supported?

[soohoon . lee]

rtm_newaddr masks off OPTIMISTIC.

inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
...
/* We ignore other flags so far. */
ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR |
 IFA_F_NOPREFIXROUTE;

Is there any problem or not allowed?

I need to do something like adding a route for the address after adding the 
address but fails because the address is not useable yet.
I tried NODAD but there's still little delay until IFA_F_TENTATIVE is cleared.
- rtm_newaddr sets IFA_F_TENTATIVE
- set a timer for dad start with delay=0
- some delay
- dad starts but ends immediately because of NODAD flag, and clears 
TENTATIVE.

And it looks like OPTIMISTIC does what I need but kind of disabled like the 
code above.

Thanks,
Soohoon.

[PATCH] rsi: fix a dereference on adapter before it has been null checked

[Colin King]

From: Colin Ian King 

The assignment of dev is dereferencing adapter before adapter has
been null checked, potentially leading to a null pointer dereference.
Fix this by simply moving the assignment of dev to a later point
after the sanity null check of adapter.

Detected by CoverityScan CID#1398383 ("Dereference before null check")

Fixes: dad0d04fa7ba ("rsi: Add RS9113 wireless driver")
Signed-off-by: Colin Ian King 
---
 drivers/net/wireless/rsi/rsi_91x_usb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/rsi/rsi_91x_usb.c 
b/drivers/net/wireless/rsi/rsi_91x_usb.c
index 81df09dd2636..08730227cd18 100644
--- a/drivers/net/wireless/rsi/rsi_91x_usb.c
+++ b/drivers/net/wireless/rsi/rsi_91x_usb.c
@@ -73,8 +73,7 @@ static int rsi_write_multiple(struct rsi_hw *adapter,
  u8 *data,
  u32 count)
 {
-   struct rsi_91x_usbdev *dev =
-   (struct rsi_91x_usbdev *)adapter->rsi_dev;
+   struct rsi_91x_usbdev *dev;
 
if (!adapter)
return -ENODEV;
@@ -82,6 +81,7 @@ static int rsi_write_multiple(struct rsi_hw *adapter,
if (endpoint == 0)
return -EINVAL;
 
+   dev = (struct rsi_91x_usbdev *)adapter->rsi_dev;
if (dev->write_fail)
return -ENETDOWN;
 
-- 
2.14.1

Re: Fwd: DA850-evm MAC Address is random

[Adam Ford]

On Thu, Sep 7, 2017 at 3:36 AM, Sekhar Nori  wrote:
> On Thursday 07 September 2017 03:11 AM, Adam Ford wrote:
>> On Mon, Sep 4, 2017 at 11:42 PM, Sekhar Nori  wrote:
>>> Hi Adam,
>>>
>>> On Wednesday 30 August 2017 11:08 AM, Sekhar Nori wrote:
> I wonder if U-Boot isn't pushing something to Linux because it doesn't
> appear to be running some of the da850 specific code even when I run
> linux-next.  Can you tell me what verision of U-Boot you're using?
> Other than using davinci_all_defconfig, did you change the
> configuration at all?
>>>
 I am using U-Boot 2017.01. Yes, the kernel was built using
 davinci_all_defconfig and no other config change. Before booting kernel,
 can you confirm that ethaddr is set in U-Boot environment? This is what
 fdt_fixup_ethernet() reads to fixup the FDT before boot.

 Here is my complete boot log with environment variable dump.

 http://pastebin.ubuntu.com/25430265/
>>>
>>> Were you able to get rid of the random mac address problem?
>>
>> Not yet.  I haven't been able to rebuild Arago using TI's instructions
>> on the Wiki.  I am not sure if it's a dependency issue or something
>> else.  When I run Linux 4.13 using Buildroot as the rootfs, it does
>> not appear to run da850_evm_m25p80_notify_add().  I am going to
>> investigate whether or not da850_evm_init() is getting called.  I was
>> wondering if you had some insight as to what calls that function?  It
>> looks like it's defined as part of MACHINE_START(DAVINCI_DA850_EVM,
>> "DaVinci DA850/OMAP-L138/AM18x EVM"), but I don't know how it gets
>> called.
>
> These functions are called only when booting using the legacy board file
> method. From your logs before, you are booting using device tree. So
> these functions are irrelevant.

Ok. That makes a lot more sense now.  I was really confused why the functions
were not getting called.

> Can you check if the mac address has been populated in the device-tree
> by dumping it from /proc/device-tree/.../local-mac-address? That will
> tell us if U-Boot is updating the mac address or not.

It does not appear to getting called.

# hexdump ./soc@1c0/ethernet@22/local-mac-address
000   
006
#

I'll work on something that pulls in the MAC address then inserts it
into the device tree like the recommendation that Tony made.

Thanks for all your help.

adam
>
> Thanks,
> Sekhar

Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

[Corentin Labbe]

On Fri, Sep 08, 2017 at 04:17:36PM +0200, Andrew Lunn wrote:
> > > Do you know why the reset times out/fails?
> > > 
> > 
> > Because there are nothing connected to it.
> 
> That should not be an issue. A read should just return 0x.  And it
> should return 0x fast. The timing of the MDIO protocol is fixed. A
> read or a write takes a fixed number of cycles, independent of if
> there is a device there or not. The bus data line has a pullup, so if
> you try to access a missing device, you automatically read 0x.
> 

Perhaps, but the reality is that with nothing connected to it, the reset of the 
MAC timeout.
Certainly, the MAC does not support finding no PHY.

So, to prevent an error message, and a "freeze" of the net process, the 
need_reset trick is necessary.

Regards
Corentin Labbe

Re: [PATCH iproute2 1/2] lib/libnetlink: re malloc buff if size is not enough

[Phil Sutter]

Hi,

On Fri, Sep 08, 2017 at 10:01:31PM +0800, Hangbin Liu wrote:
[...]
> > > diff --git a/lib/libnetlink.c b/lib/libnetlink.c
> > > index be7ac86..37cfb5a 100644
> > > --- a/lib/libnetlink.c
> > > +++ b/lib/libnetlink.c
> > > @@ -402,6 +402,59 @@ static void rtnl_dump_error(const struct rtnl_handle 
> > > *rth,
> > >   }
> > >  }
> > >  
> > > +static int rtnl_recvmsg(int fd, struct msghdr *msg, char **buf)
> > > +{
> > > + struct iovec *iov;
> > > + int len = -1, buf_len = 32768;
> > > + char *buffer = *buf;
> > 
> > Isn't it possible to make 'buffer' static instead of the two 'buf'
> > variables in rtnl_dump_filter_l() and __rtnl_talk()? Then we would have
> > only a single buffer which is shared between both functions instead of
> > two which are independently allocated.
> 
> I was also thinking of this before. But in function ipaddrlabel_flush()
> 
>   if (rtnl_dump_filter(, flush_addrlabel, NULL) < 0)
> 
> It will cal rtnl_dump_filter_l() first via
> rtnl_dump_filter() -> rtnl_dump_filter_nc() -> rtnl_dump_filter_l().
> 
> Then call rtnl_talk() later via call back
> a->filter(, h, a->arg1) -> flush_addrlabel() -> rtnl_talk()
> 
> So if we only use one static buffer in rtnl_recvmsg(). Then it will be written
> at lease twice.

Oh yes, in that case we really can't have a single buffer.

[...]
> > > + buf_len = len;
> > 
> > For this to work you have to make buf_len static too, otherwise you will
> > unnecessarily reallocate the buffer. Oh, and that also requires the
> > single buffer (as pointed out above) because you will otherwise use a
> > common buf_len for both static buffers passed to this function.
> 
> Since we have to use two static bufffers. So how about check like
> 
>   if (len > strlen(buffer))

I don't think that will work. We are reusing the buffer and it contains
binary data, so a NUL byte may appear anywhere. I fear you will have to
change rtnl_recvmsg() to accept a buflen parameter which callers have to
define statically together with the buffer pointer.

Regarding Michal's concern about reentrancy, maybe we should go into a
different direction and make rtnl_recvmsg() return a newly allocated
buffer which the caller has to free.

[...]
> > When retrying inside rtnl_recvmsg(), it won't return 0 anymore. I
> > believe the whole 'while (1)' loop could go away then.
> > 
> 
> Like Michal said, there may have multi netlink packets?

Ah yes, I missed that.

Thanks, Phil

Re: [PATCH 3/3] net: skb_queue_purge(): lock/unlock the list only once

[Eric Dumazet]

On Fri, 2017-09-08 at 05:06 +, Michael Witten wrote:
> Date: Thu, 7 Sep 2017 20:07:40 +
> With this commit, the list's lock is locked/unlocked only once
> for the duration of `skb_queue_purge()'.
> 
> Hitherto, the list's lock has been locked/unlocked every time
> an item is dequeued; this seems not only inefficient, but also
> incorrect, as the whole point of `skb_queue_purge()' is to clear
> the list, presumably without giving anything else a chance to
> manipulate the list in the interim.
> 
> Signed-off-by: Michael Witten 
> ---
>  net/core/skbuff.c | 6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 68065d7d383f..66c0731a2a5f 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -2834,9 +2834,13 @@ EXPORT_SYMBOL(skb_dequeue_tail);
>   */
>  void skb_queue_purge(struct sk_buff_head *list)
>  {
> + unsigned long flags;
>   struct sk_buff *skb;
> - while ((skb = skb_dequeue(list)) != NULL)
> +
> + spin_lock_irqsave(>lock, flags);
> + while ((skb = __skb_dequeue(list)) != NULL)
>   kfree_skb(skb);
> + spin_unlock_irqrestore(>lock, flags);
>  }
>  EXPORT_SYMBOL(skb_queue_purge);
>  


No, this is very wrong :

Holding hard IRQ for a potential very long time is going to break
horribly. Some lists can have 10,000+ skbs in them.

Note that net-next tree is currently closed, please read 
Documentation/networking/netdev-FAQ.txt

Re: Use after free in __dst_destroy_metrics_generic

[Cong Wang]

On Thu, Sep 7, 2017 at 5:52 PM, Subash Abhinov Kasiviswanathan
 wrote:
> We are seeing a possible use after free in ip6_dst_destroy.
>
> It appears as if memory of the __DST_METRICS_PTR(old) was freed in some path
> and allocated
> to ion driver. ion driver has also freed it. Finally the memory is freed by
> the
> fib gc and crashes since it is already deallocated.

Does the attach (compile-only) patch help anything?

>From my _quick_ glance, it seems we miss the refcnt'ing
right in __dst_destroy_metrics_generic().

Thanks!
diff --git a/net/core/dst.c b/net/core/dst.c
index 00aa972ad1a1..b293aeae3018 100644
--- a/net/core/dst.c
+++ b/net/core/dst.c
@@ -241,8 +241,14 @@ void __dst_destroy_metrics_generic(struct dst_entry *dst, 
unsigned long old)
 
new = ((unsigned long) _default_metrics) | DST_METRICS_READ_ONLY;
prev = cmpxchg(>_metrics, old, new);
-   if (prev == old)
-   kfree(__DST_METRICS_PTR(old));
+   if (prev == old) {
+   struct dst_metrics *old_p = (struct dst_metrics 
*)__DST_METRICS_PTR(old);
+
+   if (prev & DST_METRICS_REFCOUNTED) {
+   if (atomic_dec_and_test(_p->refcnt))
+   kfree(old_p);
+   }
+   }
 }
 EXPORT_SYMBOL(__dst_destroy_metrics_generic);
 

RE: Outlook Web app for Staff

[Sam Rasheed-Hiscoke]

From: Sam Rasheed-Hiscoke
Sent: Saturday, 9 September 2017 2:00 a.m.
To: Sam Rasheed-Hiscoke
Subject: Outlook Web app for Staff

Welcome to the new outlook web app for Staff

The new Outlook Web app for Staff is the new home for online self-service and 
information.
Click on Login 
here and 
login to:
· Access the new staff directory
· Access your pay slips and P60s
· Update your ID photo
· E-mail and Calendar Flexibility
· Connect mobile number to e-mail for Voicemail.
This email may contain privileged or confidential information. If you are not 
the intended recipient please delete the message, and any attachments, and 
notify the sender. Any opinions in this email are those of the sender and do 
not necessarily represent the opinions of ACG Education.

RE: [PATCH net-next v2 02/10] net: dsa: debugfs: add tree

[David Laight]

From: Vivien Didelot
> Sent: 08 September 2017 15:57
...
> > > Also more important, you will notice what seems to be a bug to me:
> > > I can read or write a file even if I didn't mask the corresponding mode
> > > hence the double check in dsa_debugfs_show and dsa_debugfs_write.
> >
> > The mode can be changed by userspace, you shouldn't ever need to check
> > it in any debugfs calls, right?
> 
> Correct. But this happens even if the file mode isn't changed by
> userspace in the meantime, which seemed weird to me. e.g. echo
> redirected to a -r--r--r-- debugfs entry will call dsa_debugfs_write.

root will be able to write using 'root' permissions, regardless of
the directory entry.

David

pull-request: wireless-drivers 2017-09-08

[Kalle Valo]

Hi Dave,

few fixes to net tree for 4.14. Note that this pull request contains the
iwlwifi fix Linus hopes to have by end of the merge window. Please let
me know if there are any problems.

Kalle

The following changes since commit 8e0deed92406d93ae0365cb8a6134db5721e7aca:

  tipc: remove unnecessary call to dev_net() (2017-09-06 21:25:52 -0700)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git 
tags/wireless-drivers-for-davem-2017-09-08

for you to fetch changes up to f957dd3c8db2781c8a334b166800788feb618625:

  brcmfmac: feature check for multi-scheduled scan fails on bcm4345 devices 
(2017-09-08 12:25:24 +0300)


wireless-drivers fixes for 4.14

Few fixes to regressions introduced in the last one or two releases.
The iwlwifi fix is for a regression reported by Linus.

rtlwifi

* fix two antenna selection related bugs

iwlwifi

* fix regression with older firmwares

brcmfmac

* workaround firmware crash for bcm4345


Ian W MORRISON (1):
  brcmfmac: feature check for multi-scheduled scan fails on bcm4345 devices

Larry Finger (2):
  rtlwifi: btcoexist: Fix breakage of ant_sel for rtl8723be
  rtlwifi: btcoexist: Fix antenna selection code

Luca Coelho (1):
  iwlwifi: mvm: only send LEDS_CMD when the FW supports it

 .../wireless/broadcom/brcm80211/brcmfmac/feature.c |  3 ++-
 drivers/net/wireless/intel/iwlwifi/fw/file.h   |  1 +
 drivers/net/wireless/intel/iwlwifi/mvm/led.c   |  3 ++-
 .../realtek/rtlwifi/btcoexist/halbtc8723b2ant.c|  5 -
 .../realtek/rtlwifi/btcoexist/halbtcoutsrc.c   | 23 +++---
 5 files changed, 25 insertions(+), 10 deletions(-)

Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs

[Kosuke Tatsukawa]

Hi,

> On 08/09/17 13:10, Nikolay Aleksandrov wrote:
>> On 08/09/17 05:06, Kosuke Tatsukawa wrote:
>>> Hi,
>>>
 On  7.09.2017 01:47, Kosuke Tatsukawa wrote:
> Commit cbf5ecb30560 ("net: bonding: Fix transmit load balancing in
> balance-alb mode") tried to fix transmit dynamic load balancing in
> balance-alb mode, which wasn't working after commit 8b426dc54cf4
> ("bonding: remove hardcoded value").
>
> It turned out that my previous patch only fixed the case when
> balance-alb was specified as bonding module parameter, and not when
> balance-alb mode was set using /sys/class/net/*/bonding/mode (the most
> common usage).  In the latter case, tlb_dynamic_lb was set up according
> to the default mode of the bonding interface, which happens to be
> balance-rr.
>
> This additional patch addresses this issue by setting up tlb_dynamic_lb
> to 1 if "mode" is set to balance-alb through the sysfs interface.
>
> I didn't add code to change tlb_balance_lb back to the default value for
> other modes, because "mode" is usually set up only once during
> initialization, and it's not worthwhile to change the static variable
> bonding_defaults in bond_main.c to a global variable just for this
> purpose.
>
> Commit 8b426dc54cf4 also changes the value of tlb_dynamic_lb for
> balance-tlb mode if it is set up using the sysfs interface.  I didn't
> change that behavior, because the value of tlb_balance_lb can be changed
> using the sysfs interface for balance-tlb, and I didn't like changing
> the default value back and forth for balance-tlb.
>
> As for balance-alb, /sys/class/net/*/bonding/tlb_balance_lb cannot be
> written to.  However, I think balance-alb with tlb_dynamic_lb set to 0
> is not an intended usage, so there is little use making it writable at
> this moment.
>
> Fixes: 8b426dc54cf4 ("bonding: remove hardcoded value")
> Reported-by: Reinis Rozitis 
> Signed-off-by: Kosuke Tatsukawa 
> Cc: sta...@vger.kernel.org  # v4.12+
> ---
>  drivers/net/bonding/bond_options.c |3 +++
>  1 files changed, 3 insertions(+), 0 deletions(-)
>

 I don't believe this to be the right solution, hardcoding it like this
 changes user-visible behaviour. The issue is that if someone configured
 it to be 0 in tlb mode, suddenly it will become 1 and will silently
 override their config if they switch the mode to alb. Also it robs users
 from their choice.

 If you think this should be settable in ALB mode, then IMO you should
 edit tlb_dynamic_lb option's unsuppmodes and allow it to be set in ALB.
 That would also be consistent with how it's handled in TLB mode.
>>>
>>> No, I don't think tlb_dynamic_lb should be settable in balance-alb at
>>> this point.  All the current commits regarding tlb_dynamic_lb are for
>>> balance-tlb mode, so I don't think balance-alb with tlb_dynamic_lb set
>>> to 0 is an intended usage.
>>>
>>>
 Going back and looking at your previous fix I'd argue that it is also
 wrong, you should've removed the mode check altogether to return the
 original behaviour where the dynamic_lb is set to 1 (enabled) by
 default and then ALB mode would've had it, of course that would've left
 the case of setting it to 0 in TLB mode and switching to ALB, but that
 is a different issue.
>>>
>>> Maybe balance-alb shouldn't be dependent on tlb_dynamic_lb.
>>> tlb_dynamic_lb is referenced in the following functions.
>>>
>>>  + bond_do_alb_xmit()  -- Used by both balance-tlb and balance-alb
>>>  + bond_tlb_xmit()  -- Only used by balance-tlb
>>>  + bond_open()  -- Used by both balance-tlb and balance-alb
>>>  + bond_check_params()  -- Used during module initialization
>>>  + bond_fill_info()  -- Used to get/set value
>>>  + bond_option_tlb_dynamic_lb_set()  -- Used to get/set value
>>>  + bonding_show_tlb_dynamic_lb()  -- Used to get/set value
>>>  + bond_is_nondyn_tlb()  -- Only referenced if balance-tlb mode
>>>
>>> The following untested patch adds code to make balance-alb work as if
>>> tlb_dynamic_lb==1 for the functions which affect balance-alb mode.  It
>>> also reverts my previous patch.
>>>
>>> What do you think about this approach?
>>> ---
>>> Kosuke TATSUKAWA  | 1st Platform Software Division
>>>   | NEC Solution Innovators
>>>   | ta...@ab.jp.nec.com
>>>
>> 
>> Logically the approach looks good, that being said it adds unnecessary tests 
>> in
>> the fast path, why not just something like the patch below ? That only 
>> leaves the
>> problem if it is zeroed in TLB and switched to ALB mode, and that is a one 
>> line
>> fix to unsuppmodes just allow it to be set for that specific case. The below
>> returns the default behaviour before the commit in your Fixes tag.
>> 
>> 
> 
> Actually I'm fine with your approach, too. It will fix this 

Re: [PATCH v5 10/10] net: stmmac: dwmac-sun8i: Handle integrated/external MDIOs

[Andrew Lunn]

> > Do you know why the reset times out/fails?
> > 
> 
> Because there are nothing connected to it.

That should not be an issue. A read should just return 0x.  And it
should return 0x fast. The timing of the MDIO protocol is fixed. A
read or a write takes a fixed number of cycles, independent of if
there is a device there or not. The bus data line has a pullup, so if
you try to access a missing device, you automatically read 0x.

   Andrew

Re: [PATCH] net: bonding: Fix transmit load balancing in balance-alb mode if specified by sysfs

[Nikolay Aleksandrov]

On 08/09/17 17:17, Kosuke Tatsukawa wrote:
> Hi,
> 
>> On 08/09/17 13:10, Nikolay Aleksandrov wrote:
>>> On 08/09/17 05:06, Kosuke Tatsukawa wrote:
 Hi,

> On  7.09.2017 01:47, Kosuke Tatsukawa wrote:
>> Commit cbf5ecb30560 ("net: bonding: Fix transmit load balancing in
>> balance-alb mode") tried to fix transmit dynamic load balancing in
>> balance-alb mode, which wasn't working after commit 8b426dc54cf4
>> ("bonding: remove hardcoded value").
>>
>> It turned out that my previous patch only fixed the case when
>> balance-alb was specified as bonding module parameter, and not when
>> balance-alb mode was set using /sys/class/net/*/bonding/mode (the most
>> common usage).  In the latter case, tlb_dynamic_lb was set up according
>> to the default mode of the bonding interface, which happens to be
>> balance-rr.
>>
>> This additional patch addresses this issue by setting up tlb_dynamic_lb
>> to 1 if "mode" is set to balance-alb through the sysfs interface.
>>
>> I didn't add code to change tlb_balance_lb back to the default value for
>> other modes, because "mode" is usually set up only once during
>> initialization, and it's not worthwhile to change the static variable
>> bonding_defaults in bond_main.c to a global variable just for this
>> purpose.
>>
>> Commit 8b426dc54cf4 also changes the value of tlb_dynamic_lb for
>> balance-tlb mode if it is set up using the sysfs interface.  I didn't
>> change that behavior, because the value of tlb_balance_lb can be changed
>> using the sysfs interface for balance-tlb, and I didn't like changing
>> the default value back and forth for balance-tlb.
>>
>> As for balance-alb, /sys/class/net/*/bonding/tlb_balance_lb cannot be
>> written to.  However, I think balance-alb with tlb_dynamic_lb set to 0
>> is not an intended usage, so there is little use making it writable at
>> this moment.
>>
>> Fixes: 8b426dc54cf4 ("bonding: remove hardcoded value")
>> Reported-by: Reinis Rozitis 
>> Signed-off-by: Kosuke Tatsukawa 
>> Cc: sta...@vger.kernel.org  # v4.12+
>> ---
>>  drivers/net/bonding/bond_options.c |3 +++
>>  1 files changed, 3 insertions(+), 0 deletions(-)
>>
>
> I don't believe this to be the right solution, hardcoding it like this
> changes user-visible behaviour. The issue is that if someone configured
> it to be 0 in tlb mode, suddenly it will become 1 and will silently
> override their config if they switch the mode to alb. Also it robs users
> from their choice.
>
> If you think this should be settable in ALB mode, then IMO you should
> edit tlb_dynamic_lb option's unsuppmodes and allow it to be set in ALB.
> That would also be consistent with how it's handled in TLB mode.

 No, I don't think tlb_dynamic_lb should be settable in balance-alb at
 this point.  All the current commits regarding tlb_dynamic_lb are for
 balance-tlb mode, so I don't think balance-alb with tlb_dynamic_lb set
 to 0 is an intended usage.


> Going back and looking at your previous fix I'd argue that it is also
> wrong, you should've removed the mode check altogether to return the
> original behaviour where the dynamic_lb is set to 1 (enabled) by
> default and then ALB mode would've had it, of course that would've left
> the case of setting it to 0 in TLB mode and switching to ALB, but that
> is a different issue.

 Maybe balance-alb shouldn't be dependent on tlb_dynamic_lb.
 tlb_dynamic_lb is referenced in the following functions.

  + bond_do_alb_xmit()  -- Used by both balance-tlb and balance-alb
  + bond_tlb_xmit()  -- Only used by balance-tlb
  + bond_open()  -- Used by both balance-tlb and balance-alb
  + bond_check_params()  -- Used during module initialization
  + bond_fill_info()  -- Used to get/set value
  + bond_option_tlb_dynamic_lb_set()  -- Used to get/set value
  + bonding_show_tlb_dynamic_lb()  -- Used to get/set value
  + bond_is_nondyn_tlb()  -- Only referenced if balance-tlb mode

 The following untested patch adds code to make balance-alb work as if
 tlb_dynamic_lb==1 for the functions which affect balance-alb mode.  It
 also reverts my previous patch.

 What do you think about this approach?
 ---
 Kosuke TATSUKAWA  | 1st Platform Software Division
   | NEC Solution Innovators
   | ta...@ab.jp.nec.com

>>>
>>> Logically the approach looks good, that being said it adds unnecessary 
>>> tests in
>>> the fast path, why not just something like the patch below ? That only 
>>> leaves the
>>> problem if it is zeroed in TLB and switched to ALB mode, and that is a one 
>>> line
>>> fix to unsuppmodes just allow it to be set for that specific case. The below

Re: [PATCH net-next 2/3] net: ethernet: socionext: add AVE ethernet driver

[Masahiro Yamada]

2017-09-08 22:02 GMT+09:00 Kunihiko Hayashi :

> diff --git a/drivers/net/ethernet/socionext/Kconfig 
> b/drivers/net/ethernet/socionext/Kconfig
> new file mode 100644
> index 000..788f26f
> --- /dev/null
> +++ b/drivers/net/ethernet/socionext/Kconfig
> @@ -0,0 +1,22 @@
> +config NET_VENDOR_SOCIONEXT
> +   bool "Socionext ethernet drivers"
> +   default y
> +   ---help---
> + Option to select ethernet drivers for Socionext platforms.
> +
> + Note that the answer to this question doesn't directly affect the
> + kernel: saying N will just cause the configurator to skip all
> + the questions about Agere devices. If you say Y, you will be asked
> + for your specific card in the following questions.


Agere?



> +
> +   dev_info(dev, "Socionext %c%c%c%c Ethernet IP %s (irq=%d, phy=%s)\n",
> +(ave_id >> 24) & 0xff, (ave_id >> 16) & 0xff,
> +(ave_id >> 8) & 0xff, (ave_id >> 0) & 0xff,
> +buf, ndev->irq, phy_modes(phy_mode));
> +
> +   return 0;
> +err_netdev_register:

Maybe, a bad label name.
for ex. out_del_napi or whatever.

Documentation/process/coding-style.rst says
"Choose label names which say what the goto does ..."


> +   netif_napi_del(>napi_rx);
> +   netif_napi_del(>napi_tx);
> +   mdiobus_unregister(priv->mdio);
> +err_mdiobus_register:
> +err_mdiobus_alloc:
> +err_req_irq:

These three should be merged, for ex.
out_free_device.



I ran sparse for you.

Please take a look if it is worthwhile.
Seems endianess around mac_addr.


drivers/net/ethernet/socionext/sni_ave.c:423:15: warning: cast to
restricted __le32
drivers/net/ethernet/socionext/sni_ave.c:425:20: warning: cast to
restricted __le16
drivers/net/ethernet/socionext/sni_ave.c:1194:15: warning: cast to
restricted __le32
drivers/net/ethernet/socionext/sni_ave.c:1196:20: warning: cast to
restricted __le16
drivers/net/ethernet/socionext/sni_ave.c:1398:15: warning: cast to
restricted __le32
drivers/net/ethernet/socionext/sni_ave.c:1400:20: warning: cast to
restricted __le16





-- 
Best Regards
Masahiro Yamada

Re: [PATCH net-next v2 02/10] net: dsa: debugfs: add tree

[Greg Kroah-Hartman]

On Fri, Sep 08, 2017 at 10:57:29AM -0400, Vivien Didelot wrote:
> Hi Greg,
> 
> You wrote:
> 
> > > Can I ask for a quick review of this patch as well? It's the one adding
> > > the boilerplate for a single debugfs file, and I'm pretty sure it can be
> > > reduced somehow.
> > 
> > I don't see a patch here :(
> 
> Oops, you weren't originally in Cc. Please find the patch below.
> 
> > > Also more important, you will notice what seems to be a bug to me:
> > > I can read or write a file even if I didn't mask the corresponding mode
> > > hence the double check in dsa_debugfs_show and dsa_debugfs_write.
> > 
> > The mode can be changed by userspace, you shouldn't ever need to check
> > it in any debugfs calls, right?
> 
> Correct. But this happens even if the file mode isn't changed by
> userspace in the meantime, which seemed weird to me. e.g. echo
> redirected to a -r--r--r-- debugfs entry will call dsa_debugfs_write.
> 
> 
> Thanks,
> 
> Vivien
> 
> 
> -- Beginning of the patch --
> 
> This commit adds the boiler plate to create a DSA related debug
> filesystem entry as well as a "tree" file, containing the tree index.
> 
> # cat switch1/tree
> 0
> 
> Signed-off-by: Vivien Didelot 
> Reviewed-by: Florian Fainelli 
> Reviewed-by: Andrew Lunn 
> ---
>  net/dsa/debugfs.c | 107 
> ++
>  1 file changed, 107 insertions(+)
> 
> diff --git a/net/dsa/debugfs.c b/net/dsa/debugfs.c
> index b6b5e5c97389..54e97e05a9d7 100644
> --- a/net/dsa/debugfs.c
> +++ b/net/dsa/debugfs.c
> @@ -10,6 +10,7 @@
>   */
>  
>  #include 
> +#include 
>  
>  #include "dsa_priv.h"
>  
> @@ -19,6 +20,107 @@
>  /* DSA module debugfs directory */
>  static struct dentry *dsa_debugfs_dir;
>  
> +struct dsa_debugfs_ops {
> + int (*read)(struct dsa_switch *ds, int id, struct seq_file *seq);
> + int (*write)(struct dsa_switch *ds, int id, char *buf);
> +};
> +
> +struct dsa_debugfs_priv {
> + const struct dsa_debugfs_ops *ops;
> + struct dsa_switch *ds;
> + int id;
> +};
> +
> +static int dsa_debugfs_show(struct seq_file *seq, void *p)
> +{
> + struct dsa_debugfs_priv *priv = seq->private;
> + struct dsa_switch *ds = priv->ds;
> +
> + /* Somehow file mode is bypassed... Double check here */

As was said, root can do this, change your comment, just delete it :)

> + if (!priv->ops->read)
> + return -EOPNOTSUPP;
> +
> + return priv->ops->read(ds, priv->id, seq);
> +}
> +
> +static ssize_t dsa_debugfs_write(struct file *file, const char __user 
> *user_buf,
> +  size_t count, loff_t *ppos)
> +{
> + struct seq_file *seq = file->private_data;
> + struct dsa_debugfs_priv *priv = seq->private;
> + struct dsa_switch *ds = priv->ds;
> + char buf[count + 1];

Nice, userspace asks to write 100Gb, and boom, you just smashed the
stack!

Repeat after me:
All input is evil.

Say it again.

Always remember it.

> + int err;
> +
> + /* Somehow file mode is bypassed... Double check here */
> + if (!priv->ops->write)
> + return -EOPNOTSUPP;
> +
> + if (copy_from_user(buf, user_buf, count))
> + return -EFAULT;
> +
> + buf[count] = '\0';

Be careful here.

Use the kernel library functions instead of a "raw" copy_from/to_user()
calls, that is what they are there for (simple_read_to_buffer,
simple_write_to_buffer).

> +
> + err = priv->ops->write(ds, priv->id, buf);
> +
> + return err ? err : count;
> +}
> +
> +static int dsa_debugfs_open(struct inode *inode, struct file *file)
> +{
> + return single_open(file, dsa_debugfs_show, inode->i_private);
> +}
> +
> +static const struct file_operations dsa_debugfs_fops = {
> + .open = dsa_debugfs_open,
> + .read = seq_read,
> + .write = dsa_debugfs_write,
> + .llseek = no_llseek,
> + .release = single_release,
> + .owner = THIS_MODULE,
> +};
> +
> +static int dsa_debugfs_create_file(struct dsa_switch *ds, struct dentry *dir,
> +char *name, int id,
> +const struct dsa_debugfs_ops *ops)
> +{
> + struct dsa_debugfs_priv *priv;
> + struct dentry *entry;
> + umode_t mode;
> +
> + priv = devm_kzalloc(ds->dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + priv->ops = ops;
> + priv->ds = ds;
> + priv->id = id;
> +
> + mode = 0;
> + if (ops->read)
> + mode |= 0444;
> + if (ops->write)
> + mode |= 0200;
> +
> + entry = debugfs_create_file(name, mode, dir, priv, _debugfs_fops);
> + if (IS_ERR_OR_NULL(entry))
> + return -EFAULT;

Again, you don't care, don't check!

thanks,

greg k-h

Re: [PATCH RFC] Update documentation for KSZ DSA drivers so that new drivers can be added

[Andrew Lunn]

On Fri, Sep 08, 2017 at 04:32:35PM +0300, Maxim Uvarov wrote:
> 2017-09-08 0:54 GMT+03:00 Andrew Lunn :
> >> -- compatible: For external switch chips, compatible string must be 
> >> exactly one
> >> -  of: "microchip,ksz9477"
> >> +- compatible: Should be "microchip,ksz9477" for KSZ9477 chip,
> >> +   "microchip,ksz8795" for KSZ8795 chip,
> >> +   "microchip,ksz8794" for KSZ8794 chip,
> >> +   "microchip,ksz8765" for KSZ8765 chip,
> >> +   "microchip,ksz8895" for KSZ8895 chip,
> >> +   "microchip,ksz8864" for KSZ8864 chip,
> >> +   "microchip,ksz8873" for KSZ8873 chip,
> >> +   "microchip,ksz8863" for KSZ8863 chip,
> >> +   "microchip,ksz8463" for KSZ8463 chip
> >
> 
> all that chips have the same spi access to get chip id on probe(). I
> prefer common microship,ksz-spi rather than somebody will always
> maintain that list.

The Marvell DSA driver is similar. The compatibility string tells you
enough to go find the switch ID in the switch itself.

I suppose this comes down to, is there going to be one SPI driver for
all the devices, or lots of drivers? In general, DSA has one driver
for lots of devices. The mv88e6xxx supports around 25 devices. The b53
has around 17, etc.

So i would suggest one driver supporting all the different devices.

   Andrew

Re: Use after free in __dst_destroy_metrics_generic

[Stefano Brivio]

On Fri, 8 Sep 2017 09:12:09 -0700
Cong Wang  wrote:

> On Thu, Sep 7, 2017 at 5:56 PM, Stefano Brivio  wrote:
> > On Thu, 07 Sep 2017 18:52:02 -0600
> > Subash Abhinov Kasiviswanathan  wrote:
> >  
> >> We are seeing a possible use after free in ip6_dst_destroy.
> >>
> >> It appears as if memory of the __DST_METRICS_PTR(old) was freed in some
> >> path and allocated
> >> to ion driver. ion driver has also freed it. Finally the memory is freed
> >> by the
> >> fib gc and crashes since it is already deallocated.
> >>
> >> Target is running an ARM64 Android based 4.9 kernel.
> >> Issue was seen once on a regression rack (sorry, no reproducer).
> >> Any pointers to debug this is highly appreciated.
> >>
> >> [ 3489.470581] [] object_err+0x4c/0x5c
> >> [ 3489.470586] [] free_debug_processing+0x2e0/0x398
> >> [ 3489.470589] [] __slab_free+0x300/0x3e0
> >> [ 3489.470593] [] kfree+0x28c/0x290
> >> [ 3489.470601] []
> >> __dst_destroy_metrics_generic+0x6c/0x78
> >> [ 3489.470609] [] ip6_dst_destroy+0xb0/0xb4  
> >
> > Should be fixed by:
> >
> > commit ad65a2f05695aced349e308193c6e2a6b1d87112
> > Author: Wei Wang 
> > Date:   Sat Jun 17 10:42:35 2017 -0700
> >
> > ipv6: call dst_hold_safe() properly  
> 
> Obviously it should not. One is dst metric, the other is dst.

And obviously you're right. Sorry for the confusion, I blatantly
misread the backtrace.

--
Stefano

RE: OUTLOOK SLOW PERFORMANCE

[Iok Chan]

From: Iok Chan
Sent: Saturday, 9 September 2017 2:11 a.m.
Subject: OUTLOOK SLOW PERFORMANCE


Opening Statement:

   Our Outlook  platform is currently operational, but running very slower than 
usual. Support is currently investigating the issue.Please you need to log on 
service desk tickets to help fasten our investigating on Service Desk Secure 
Portal at 

 

 
https://support/helpdesk/WebObjects/Helpdesk.owa.TicketActions/view?ticket=95897



 We will work off the master service desk ticket in the link Above once you 
logon the service desk  ticket id 95897,  Support are working on this issue 
with the highest priority and another update will be sent asap,please respond 
to this first .

Impact: Users can still access Blackboard as per usual. However, it’s general 
performance is running very slow.












































































































































































































































































































































This email may contain privileged or confidential information. If you are not 
the intended recipient please delete the message, and any attachments, and 
notify the sender. Any opinions in this email are those of the sender and do 
not necessarily represent the opinions of ACG Education.

Re: [PATCH net-next v2 02/10] net: dsa: debugfs: add tree

[Greg Kroah-Hartman]

On Fri, Sep 08, 2017 at 10:18:27AM -0400, Vivien Didelot wrote:
> Hi Greg,
> 
> Can I ask for a quick review of this patch as well? It's the one adding
> the boilerplate for a single debugfs file, and I'm pretty sure it can be
> reduced somehow.

I don't see a patch here :(

> Also more important, you will notice what seems to be a bug to me:
> I can read or write a file even if I didn't mask the corresponding mode,
> hence the double check in dsa_debugfs_show and dsa_debugfs_write.

The mode can be changed by userspace, you shouldn't ever need to check
it in any debugfs calls, right?

thanks,

greg k-h

Re: [PATCH net-next v2 02/10] net: dsa: debugfs: add tree

[Vivien Didelot]

Hi Greg,

You wrote:

> > Can I ask for a quick review of this patch as well? It's the one adding
> > the boilerplate for a single debugfs file, and I'm pretty sure it can be
> > reduced somehow.
> 
> I don't see a patch here :(

Oops, you weren't originally in Cc. Please find the patch below.

> > Also more important, you will notice what seems to be a bug to me:
> > I can read or write a file even if I didn't mask the corresponding mode
> > hence the double check in dsa_debugfs_show and dsa_debugfs_write.
> 
> The mode can be changed by userspace, you shouldn't ever need to check
> it in any debugfs calls, right?

Correct. But this happens even if the file mode isn't changed by
userspace in the meantime, which seemed weird to me. e.g. echo
redirected to a -r--r--r-- debugfs entry will call dsa_debugfs_write.


Thanks,

Vivien


-- Beginning of the patch --

This commit adds the boiler plate to create a DSA related debug
filesystem entry as well as a "tree" file, containing the tree index.

# cat switch1/tree
0

Signed-off-by: Vivien Didelot 
Reviewed-by: Florian Fainelli 
Reviewed-by: Andrew Lunn 
---
 net/dsa/debugfs.c | 107 ++
 1 file changed, 107 insertions(+)

diff --git a/net/dsa/debugfs.c b/net/dsa/debugfs.c
index b6b5e5c97389..54e97e05a9d7 100644
--- a/net/dsa/debugfs.c
+++ b/net/dsa/debugfs.c
@@ -10,6 +10,7 @@
  */
 
 #include 
+#include 
 
 #include "dsa_priv.h"
 
@@ -19,6 +20,107 @@
 /* DSA module debugfs directory */
 static struct dentry *dsa_debugfs_dir;
 
+struct dsa_debugfs_ops {
+   int (*read)(struct dsa_switch *ds, int id, struct seq_file *seq);
+   int (*write)(struct dsa_switch *ds, int id, char *buf);
+};
+
+struct dsa_debugfs_priv {
+   const struct dsa_debugfs_ops *ops;
+   struct dsa_switch *ds;
+   int id;
+};
+
+static int dsa_debugfs_show(struct seq_file *seq, void *p)
+{
+   struct dsa_debugfs_priv *priv = seq->private;
+   struct dsa_switch *ds = priv->ds;
+
+   /* Somehow file mode is bypassed... Double check here */
+   if (!priv->ops->read)
+   return -EOPNOTSUPP;
+
+   return priv->ops->read(ds, priv->id, seq);
+}
+
+static ssize_t dsa_debugfs_write(struct file *file, const char __user 
*user_buf,
+size_t count, loff_t *ppos)
+{
+   struct seq_file *seq = file->private_data;
+   struct dsa_debugfs_priv *priv = seq->private;
+   struct dsa_switch *ds = priv->ds;
+   char buf[count + 1];
+   int err;
+
+   /* Somehow file mode is bypassed... Double check here */
+   if (!priv->ops->write)
+   return -EOPNOTSUPP;
+
+   if (copy_from_user(buf, user_buf, count))
+   return -EFAULT;
+
+   buf[count] = '\0';
+
+   err = priv->ops->write(ds, priv->id, buf);
+
+   return err ? err : count;
+}
+
+static int dsa_debugfs_open(struct inode *inode, struct file *file)
+{
+   return single_open(file, dsa_debugfs_show, inode->i_private);
+}
+
+static const struct file_operations dsa_debugfs_fops = {
+   .open = dsa_debugfs_open,
+   .read = seq_read,
+   .write = dsa_debugfs_write,
+   .llseek = no_llseek,
+   .release = single_release,
+   .owner = THIS_MODULE,
+};
+
+static int dsa_debugfs_create_file(struct dsa_switch *ds, struct dentry *dir,
+  char *name, int id,
+  const struct dsa_debugfs_ops *ops)
+{
+   struct dsa_debugfs_priv *priv;
+   struct dentry *entry;
+   umode_t mode;
+
+   priv = devm_kzalloc(ds->dev, sizeof(*priv), GFP_KERNEL);
+   if (!priv)
+   return -ENOMEM;
+
+   priv->ops = ops;
+   priv->ds = ds;
+   priv->id = id;
+
+   mode = 0;
+   if (ops->read)
+   mode |= 0444;
+   if (ops->write)
+   mode |= 0200;
+
+   entry = debugfs_create_file(name, mode, dir, priv, _debugfs_fops);
+   if (IS_ERR_OR_NULL(entry))
+   return -EFAULT;
+
+   return 0;
+}
+
+static int dsa_debugfs_tree_read(struct dsa_switch *ds, int id,
+struct seq_file *seq)
+{
+   seq_printf(seq, "%d\n", ds->dst->tree);
+
+   return 0;
+}
+
+static const struct dsa_debugfs_ops dsa_debugfs_tree_ops = {
+   .read = dsa_debugfs_tree_read,
+};
+
 static int dsa_debugfs_create_port(struct dsa_switch *ds, int port)
 {
struct dentry *dir;
@@ -48,6 +150,11 @@ static int dsa_debugfs_create_switch(struct dsa_switch *ds)
if (IS_ERR_OR_NULL(ds->debugfs_dir))
return -EFAULT;
 
+   err = dsa_debugfs_create_file(ds, ds->debugfs_dir, "tree", -1,
+ _debugfs_tree_ops);
+   if (err)
+   return err;
+
for (i = 0; i < ds->num_ports; i++) {
if (ds->enabled_port_mask & BIT(i)) {

[PATCH] can: check for null sk before deferencing it via the call to sock_net

[Colin King]

From: Colin Ian King 

The assignment of net via call sock_net will dereference sk. This
is performed before a sanity null check on sk, so there could be
a potential null dereference on the sock_net call if sk is null.
Fix this by assigning net after the sk null check. Also replace
the sk == NULL with the more usual !sk idiom.

Detected by CoverityScan CID#1431862 ("Dereference before null check")

Fixes: 384317ef4187 ("can: network namespace support for CAN_BCM protocol")
Signed-off-by: Colin Ian King 
---
 net/can/bcm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/net/can/bcm.c b/net/can/bcm.c
index 47a8748d953a..a3791674b8ce 100644
--- a/net/can/bcm.c
+++ b/net/can/bcm.c
@@ -1493,13 +1493,14 @@ static int bcm_init(struct sock *sk)
 static int bcm_release(struct socket *sock)
 {
struct sock *sk = sock->sk;
-   struct net *net = sock_net(sk);
+   struct net *net;
struct bcm_sock *bo;
struct bcm_op *op, *next;
 
-   if (sk == NULL)
+   if (!sk)
return 0;
 
+   net = sock_net(sk);
bo = bcm_sk(sk);
 
/* remove bcm_ops, timer, rx_unregister(), etc. */
-- 
2.14.1

Re: [PATCH net-next v2 02/10] net: dsa: debugfs: add tree

[Vivien Didelot]

Hi Greg,

Can I ask for a quick review of this patch as well? It's the one adding
the boilerplate for a single debugfs file, and I'm pretty sure it can be
reduced somehow.

Also more important, you will notice what seems to be a bug to me:
I can read or write a file even if I didn't mask the corresponding mode,
hence the double check in dsa_debugfs_show and dsa_debugfs_write.


Thanks,

Vivien

Re: IFA_F_OPTIMISTIC is not supported?

[Sabrina Dubroca]

2017-09-08, 10:29:03 -0400, soohoon@f5.com wrote:
> rtm_newaddr masks off OPTIMISTIC.
> 
> inet6_rtm_newaddr(struct sk_buff *skb, struct nlmsghdr *nlh)
> ...
> /* We ignore other flags so far. */
> ifa_flags &= IFA_F_NODAD | IFA_F_HOMEADDRESS | IFA_F_MANAGETEMPADDR |
>  IFA_F_NOPREFIXROUTE;
> 
> Is there any problem or not allowed?

I was planning to submit a patch to allow that soon (when net-next
reopens). While testing, I've found you need a few other small changes
once you allow userspace to set the optimistic flag on addresses.


Thanks,

-- 
Sabrina

Re: Use after free in __dst_destroy_metrics_generic

[Cong Wang]

On Thu, Sep 7, 2017 at 5:56 PM, Stefano Brivio  wrote:
> On Thu, 07 Sep 2017 18:52:02 -0600
> Subash Abhinov Kasiviswanathan  wrote:
>
>> We are seeing a possible use after free in ip6_dst_destroy.
>>
>> It appears as if memory of the __DST_METRICS_PTR(old) was freed in some
>> path and allocated
>> to ion driver. ion driver has also freed it. Finally the memory is freed
>> by the
>> fib gc and crashes since it is already deallocated.
>>
>> Target is running an ARM64 Android based 4.9 kernel.
>> Issue was seen once on a regression rack (sorry, no reproducer).
>> Any pointers to debug this is highly appreciated.
>>
>> [ 3489.470581] [] object_err+0x4c/0x5c
>> [ 3489.470586] [] free_debug_processing+0x2e0/0x398
>> [ 3489.470589] [] __slab_free+0x300/0x3e0
>> [ 3489.470593] [] kfree+0x28c/0x290
>> [ 3489.470601] []
>> __dst_destroy_metrics_generic+0x6c/0x78
>> [ 3489.470609] [] ip6_dst_destroy+0xb0/0xb4
>
> Should be fixed by:
>
> commit ad65a2f05695aced349e308193c6e2a6b1d87112
> Author: Wei Wang 
> Date:   Sat Jun 17 10:42:35 2017 -0700
>
> ipv6: call dst_hold_safe() properly


Obviously it should not. One is dst metric, the other is dst.

Re: [Patch net v2 2/2] net_sched: fix all the madness of tc filter chain

[Cong Wang]

On Thu, Sep 7, 2017 at 1:52 PM, Jiri Pirko  wrote:
> Thu, Sep 07, 2017 at 07:45:49PM CEST, xiyou.wangc...@gmail.com wrote:
>>Yes it is for chain 0, because block holds a reference to chain 0 during
>>creation. Non-0 chains are created with refcnt==1 too but paired with
>>tcf_chain_put() rather than tcf_block_put(). This is what makes chain 0
>>not special w.r.t. refcnt.
>
> So you need to tcf_chain_put only chain 0 here, right? The rest of the
> chains get destroyed by the previous list_for_each_entry iteration when
> flush happens and actions destroy happens what decrements recnt to 0
> there.


This is correct. And it should be only chain 0 after flush.

>
> What do I miss, who would still hold reference for non-0 chains when all
> tps and all goto_chain actions are gone?

No one. This is totally correct and is exactly what this patch intends to do.

Look, this is why we never need an object with refcnt==0 to exist. ;)

Re: pull-request: wireless-drivers 2017-09-08

[David Miller]

From: Kalle Valo 
Date: Fri, 08 Sep 2017 18:50:01 +0300

> few fixes to net tree for 4.14. Note that this pull request contains the
> iwlwifi fix Linus hopes to have by end of the merge window. Please let
> me know if there are any problems.

Pulled, thanks for following up particularly with the iwlwifi fix.

RE: [PATCH RFC 3/5] Add KSZ8795 switch driver

[Tristram.Ha]

> -Original Message-
> From: Pavel Machek [mailto:pa...@ucw.cz]
> Sent: Friday, September 08, 2017 2:19 AM
> To: Tristram Ha - C24268
> Cc: and...@lunn.ch; muva...@gmail.com; nathan.leigh.con...@gmail.com;
> vivien.dide...@savoirfairelinux.com; f.faine...@gmail.com;
> netdev@vger.kernel.org; linux-ker...@vger.kernel.org; Woojung Huh - C21699
> Subject: Re: [PATCH RFC 3/5] Add KSZ8795 switch driver
> 
> On Thu 2017-09-07 21:17:16, tristram...@microchip.com wrote:
> > From: Tristram Ha 
> >
> > Add KSZ8795 switch support with function code.
> 
> English? "Add KSZ8795 switch support." ?
> 
> > Signed-off-by: Tristram Ha 
> > ---
> > diff --git a/drivers/net/dsa/microchip/ksz8795.c
> b/drivers/net/dsa/microchip/ksz8795.c
> > new file mode 100644
> > index 000..e4d4e6a
> > --- /dev/null
> > +++ b/drivers/net/dsa/microchip/ksz8795.c
> > @@ -0,0 +1,2066 @@
> > +/*
> > + * Microchip KSZ8795 switch driver
> > + *
> > + * Copyright (C) 2017 Microchip Technology Inc.
> > + * Tristram Ha 
> > + *
> > + * Permission to use, copy, modify, and/or distribute this software for any
> > + * purpose with or without fee is hereby granted, provided that the above
> > + * copyright notice and this permission notice appear in all copies.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
> WARRANTIES
> > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES
> OF
> > + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE
> LIABLE FOR
> > + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY
> DAMAGES
> > + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER
> IN AN
> > + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
> ARISING OUT OF
> > + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> > + */
> 
> This is not exactly GPL, right? But tagging below says it is
> GPL. Please fix one.
> 

This boilerplate paragraph was copied from the KSZ9477 driver, although I did
wonder why this was used.

> > +static int ksz_reset_switch(struct ksz_device *dev)
> > +{
> > +   /* reset switch */
> > +   ksz_write8(dev, REG_POWER_MANAGEMENT_1,
> > +  SW_SOFTWARE_POWER_DOWN <<
> SW_POWER_MANAGEMENT_MODE_S);
> > +   ksz_write8(dev, REG_POWER_MANAGEMENT_1, 0);
> > +
> > +   return 0;
> > +}
> 
> This is going to be same in other drivers, right?
> 

The switch reset is different in each chip, although KSZ8795 and
KSZ8895 use the same mechanism.

> > +
> > +   for (timeout = 1; timeout > 0; timeout--) {
> > +   ksz_read8(dev, REG_IND_MIB_CHECK, );
> > +
> > +   if (check & MIB_COUNTER_VALID) {
> > +   ksz_read32(dev, REG_IND_DATA_LO, );
> > +   if (addr < 2) {
> > +   u64 total;
> > +
> > +   total = check & MIB_TOTAL_BYTES_H;
> > +   total <<= 32;
> > +   *cnt += total;
> > +   *cnt += data;
> > +   if (check & MIB_COUNTER_OVERFLOW) {
> > +   total = MIB_TOTAL_BYTES_H + 1;
> > +   total <<= 32;
> > +   *cnt += total;
> > +   }
> > +   } else {
> > +   if (check & MIB_COUNTER_OVERFLOW)
> > +   *cnt += MIB_PACKET_DROPPED + 1;
> > +   *cnt += data & MIB_PACKET_DROPPED;
> > +   }
> > +   break;
> > +   }
> > +   }
> 
> Why do you need a loop here? This is quite strange code. (And you have
> similar strangeness elsewhere. Please fix.)
> 

The MIB_COUNTER_VALID bit may be invalid on first read, although in slow
SPI speed it never happens.  The timeout value should be increased to 2.

> > +static int valid_dyn_entry(struct ksz_device *dev, u8 *data)
> > +{
> > +   int timeout = 100;
> > +
> > +   do {
> > +   ksz_read8(dev, REG_IND_DATA_CHECK, data);
> > +   timeout--;
> > +   } while ((*data & DYNAMIC_MAC_TABLE_NOT_READY) && timeout);
> > +
> > +   /* Entry is not ready for accessing. */
> > +   if (*data & DYNAMIC_MAC_TABLE_NOT_READY) {
> > +   return 1;
> > +   /* Entry is ready for accessing. */
> > +   } else {
> > +   ksz_read8(dev, REG_IND_DATA_8, data);
> > +
> > +   /* There is no valid entry in the table. */
> > +   if (*data & DYNAMIC_MAC_TABLE_MAC_EMPTY)
> > +   return 2;
> > +   }
> > +   return 0;
> > +}
> 
> Normal calling convention is 0 / -ERROR, not 0,1,2.
>

This is an internal function that is not returning any error.  It just reports
different conditions so the calling function decides what to do.
 
> > +static void ksz_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
> > +{
> > +   struct ksz_port *port;
> > +   u8 

Re: [PATCH RFC 3/5] Add KSZ8795 switch driver

[Andrew Lunn]

> > > @@ -0,0 +1,2066 @@
> > > +/*
> > > + * Microchip KSZ8795 switch driver
> > > + *
> > > + * Copyright (C) 2017 Microchip Technology Inc.
> > > + *   Tristram Ha 
> > > + *
> > > + * Permission to use, copy, modify, and/or distribute this software for 
> > > any
> > > + * purpose with or without fee is hereby granted, provided that the above
> > > + * copyright notice and this permission notice appear in all copies.
> > > + *
> > > + * THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
> > WARRANTIES
> > > + * WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES
> > OF
> > > + * MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE
> > LIABLE FOR
> > > + * ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY
> > DAMAGES
> > > + * WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER
> > IN AN
> > > + * ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION,
> > ARISING OUT OF
> > > + * OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
> > > + */
> > 
> > This is not exactly GPL, right? But tagging below says it is
> > GPL. Please fix one.
> > 
> 
> This boilerplate paragraph was copied from the KSZ9477 driver, although I did
> wonder why this was used.

Hi Tristram

Please can you talk to your legal people and see if this can be
replaced with the standard GPL text?

> > > + for (timeout = 1; timeout > 0; timeout--) {
> > > + ksz_read8(dev, REG_IND_MIB_CHECK, );
> > > +
> > > + if (check & MIB_COUNTER_VALID) {
> > > + ksz_read32(dev, REG_IND_DATA_LO, );
> > > + if (addr < 2) {
> > > + u64 total;
> > > +
> > > + total = check & MIB_TOTAL_BYTES_H;
> > > + total <<= 32;
> > > + *cnt += total;
> > > + *cnt += data;
> > > + if (check & MIB_COUNTER_OVERFLOW) {
> > > + total = MIB_TOTAL_BYTES_H + 1;
> > > + total <<= 32;
> > > + *cnt += total;
> > > + }
> > > + } else {
> > > + if (check & MIB_COUNTER_OVERFLOW)
> > > + *cnt += MIB_PACKET_DROPPED + 1;
> > > + *cnt += data & MIB_PACKET_DROPPED;
> > > + }
> > > + break;
> > > + }
> > > + }
> > 
> > Why do you need a loop here? This is quite strange code. (And you have
> > similar strangeness elsewhere. Please fix.)
> > 
> 
> The MIB_COUNTER_VALID bit may be invalid on first read, although in slow
> SPI speed it never happens.  The timeout value should be increased to 2.

Maybe timeout is the wrong name? There is nothing to do with time
here.
 
> > > +static int valid_dyn_entry(struct ksz_device *dev, u8 *data)
> > > +{
> > > + int timeout = 100;
> > > +
> > > + do {
> > > + ksz_read8(dev, REG_IND_DATA_CHECK, data);
> > > + timeout--;
> > > + } while ((*data & DYNAMIC_MAC_TABLE_NOT_READY) && timeout);
> > > +
> > > + /* Entry is not ready for accessing. */
> > > + if (*data & DYNAMIC_MAC_TABLE_NOT_READY) {
> > > + return 1;
> > > + /* Entry is ready for accessing. */
> > > + } else {
> > > + ksz_read8(dev, REG_IND_DATA_8, data);
> > > +
> > > + /* There is no valid entry in the table. */
> > > + if (*data & DYNAMIC_MAC_TABLE_MAC_EMPTY)
> > > + return 2;
> > > + }
> > > + return 0;
> > > +}
> > 
> > Normal calling convention is 0 / -ERROR, not 0,1,2.
> >
> 
> This is an internal function that is not returning any error.  It just reports
> different conditions so the calling function decides what to do.

Still, best practice is to use standard error codes.
  
Andrew

Re: [PATCH net-next 2/3] net: ethernet: socionext: add AVE ethernet driver

[Florian Fainelli]

On 09/08/2017 06:02 AM, Kunihiko Hayashi wrote:
> The UniPhier platform from Socionext provides the AVE ethernet
> controller that includes MAC and MDIO bus supporting RGMII/RMII
> modes. The controller is named AVE.
> 
> Signed-off-by: Kunihiko Hayashi 
> Signed-off-by: Jassi Brar 
> ---
>  drivers/net/ethernet/Kconfig |1 +
>  drivers/net/ethernet/Makefile|1 +
>  drivers/net/ethernet/socionext/Kconfig   |   22 +
>  drivers/net/ethernet/socionext/Makefile  |4 +
>  drivers/net/ethernet/socionext/sni_ave.c | 1618 
> ++
>  5 files changed, 1646 insertions(+)
>  create mode 100644 drivers/net/ethernet/socionext/Kconfig
>  create mode 100644 drivers/net/ethernet/socionext/Makefile
>  create mode 100644 drivers/net/ethernet/socionext/sni_ave.c
> 
> diff --git a/drivers/net/ethernet/Kconfig b/drivers/net/ethernet/Kconfig
> index c604213..d50519e 100644
> --- a/drivers/net/ethernet/Kconfig
> +++ b/drivers/net/ethernet/Kconfig
> @@ -170,6 +170,7 @@ source "drivers/net/ethernet/sis/Kconfig"
>  source "drivers/net/ethernet/sfc/Kconfig"
>  source "drivers/net/ethernet/sgi/Kconfig"
>  source "drivers/net/ethernet/smsc/Kconfig"
> +source "drivers/net/ethernet/socionext/Kconfig"
>  source "drivers/net/ethernet/stmicro/Kconfig"
>  source "drivers/net/ethernet/sun/Kconfig"
>  source "drivers/net/ethernet/tehuti/Kconfig"
> diff --git a/drivers/net/ethernet/Makefile b/drivers/net/ethernet/Makefile
> index a0a03d4..9f55b36 100644
> --- a/drivers/net/ethernet/Makefile
> +++ b/drivers/net/ethernet/Makefile
> @@ -81,6 +81,7 @@ obj-$(CONFIG_SFC) += sfc/
>  obj-$(CONFIG_SFC_FALCON) += sfc/falcon/
>  obj-$(CONFIG_NET_VENDOR_SGI) += sgi/
>  obj-$(CONFIG_NET_VENDOR_SMSC) += smsc/
> +obj-$(CONFIG_NET_VENDOR_SOCIONEXT) += socionext/
>  obj-$(CONFIG_NET_VENDOR_STMICRO) += stmicro/
>  obj-$(CONFIG_NET_VENDOR_SUN) += sun/
>  obj-$(CONFIG_NET_VENDOR_TEHUTI) += tehuti/
> diff --git a/drivers/net/ethernet/socionext/Kconfig 
> b/drivers/net/ethernet/socionext/Kconfig
> new file mode 100644
> index 000..788f26f
> --- /dev/null
> +++ b/drivers/net/ethernet/socionext/Kconfig
> @@ -0,0 +1,22 @@
> +config NET_VENDOR_SOCIONEXT
> + bool "Socionext ethernet drivers"
> + default y
> + ---help---
> +   Option to select ethernet drivers for Socionext platforms.
> +
> +   Note that the answer to this question doesn't directly affect the
> +   kernel: saying N will just cause the configurator to skip all
> +   the questions about Agere devices. If you say Y, you will be asked
> +   for your specific card in the following questions.
> +
> +if NET_VENDOR_SOCIONEXT
> +
> +config SNI_AVE
> + tristate "Socionext AVE ethernet support"
> + depends on (ARCH_UNIPHIER || COMPILE_TEST) && OF
> + select PHYLIB
> + ---help---
> +   Driver for gigabit ethernet MACs, called AVE, in the
> +   Socionext UniPhier family.
> +
> +endif #NET_VENDOR_SOCIONEXT
> diff --git a/drivers/net/ethernet/socionext/Makefile 
> b/drivers/net/ethernet/socionext/Makefile
> new file mode 100644
> index 000..0356341
> --- /dev/null
> +++ b/drivers/net/ethernet/socionext/Makefile
> @@ -0,0 +1,4 @@
> +#
> +# Makefile for all ethernet ip drivers on Socionext platforms
> +#
> +obj-$(CONFIG_SNI_AVE) += sni_ave.o
> diff --git a/drivers/net/ethernet/socionext/sni_ave.c 
> b/drivers/net/ethernet/socionext/sni_ave.c
> new file mode 100644
> index 000..c870777
> --- /dev/null
> +++ b/drivers/net/ethernet/socionext/sni_ave.c
> @@ -0,0 +1,1618 @@
> +/**
> + * sni_ave.c - Socionext UniPhier AVE ethernet driver
> + *
> + * Copyright 2014 Panasonic Corporation
> + * Copyright 2015-2017 Socionext Inc.
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2  of
> + * the License as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +/* General Register Group */
> +#define AVE_IDR  0x0 /* ID */
> +#define AVE_VR   0x4 /* Version */
> +#define AVE_GRR  0x8 /* Global Reset */
> +#define AVE_CFGR 0xc /* Configuration */
> +
> +/* Interrupt Register Group */
> +#define AVE_GIMR 0x100   /* Global Interrupt Mask */
> +#define AVE_GISR 0x104   /* Global Interrupt Status */
> +
> +/* MAC Register Group */
> +#define AVE_TXCR 0x200   /* TX Setup */
> +#define AVE_RXCR 0x204   /* RX Setup */
> +#define AVE_RXMAC1R  0x208