* Re: [PATCH net-next v2] bonding: check slave set command firstly
From: David Miller @ 2019-02-14 16:36 UTC (permalink / raw)
To: xiangxia.m.yue; +Cc: netdev
In-Reply-To: <1549910988-40999-1-git-send-email-xiangxia.m.yue@gmail.com>
From: xiangxia.m.yue@gmail.com
Date: Mon, 11 Feb 2019 10:49:48 -0800
> From: Tonghao Zhang <xiangxia.m.yue@gmail.com>
>
> This patch is a little improvement. If user use the
> command shown as below, we should print the info [1]
> instead of [2]. The eth0 exists actually, and it may
> confuse user.
>
> $ echo "eth0" > /sys/class/net/bond4/bonding/slaves
>
> [1] "bond4: no command found in slaves file - use +ifname or -ifname"
> [2] "write error: No such device"
>
> Signed-off-by: Tonghao Zhang <xiangxia.m.yue@gmail.com>
Applied, but would you please fix the dates on your submissions?
Because the dates in your patch postings are in the past, patchwork
puts your work at the tail of my patch queue instead of the front.
Thank you.
^ permalink raw reply
* Re: [PATCH iproute2] iplink: document XDP subcommand to force the XDP mode.
From: Stephen Hemminger @ 2019-02-14 16:29 UTC (permalink / raw)
To: Matteo Croce; +Cc: netdev, David Ahern, Stephen Hemminger, Jakub Kicinski
In-Reply-To: <CAGnkfhyYVN0_hqzrQQWb_+CW9PUr_C4JMa6GGx9Fh0qfu=Qx-A@mail.gmail.com>
On Thu, 14 Feb 2019 15:01:26 +0100
Matteo Croce <mcroce@redhat.com> wrote:
> On Wed, Feb 13, 2019 at 11:04 PM Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> >
> > On Wed, 13 Feb 2019 15:40:30 +0100
> > Matteo Croce <mcroce@redhat.com> wrote:
> >
> > > When attaching an eBPF program to a device, ip link can force the XDP mode
> > > by using the xdp{generic,drv,offload} keyword instead of just 'xdp'.
> > > Document this behaviour also in the help output.
> > >
> > > Signed-off-by: Matteo Croce <mcroce@redhat.com>
> > > Fixes: 14683814 ("bpf: add xdpdrv for requesting XDP driver mode")
> > > Fixes: 1b5e8094 ("bpf: allow requesting XDP HW offload")
> >
> > Applied, thanks.
> > The man page already has this as well.
> >
>
> Yes, I found it just after I made the patch. However, it could be nice
> to have the generic "xdp" and a command like "type" or "mode" to
> specify the XDP mode, eg.
> ip link set dev eth0 xdp mode [ auto | generic | drv | offload ]
> I was trying to add it, but unfortunately it seems that the arguments
> aren't parsed in a loop, and are required to be in the exact order.
> Would this change make sense?
>
> Regards,
>
> --
> Matteo Croce
> per aspera ad upstream
It would have made sense to start with, but the syntax is more or less fixed
by now.
^ permalink raw reply
* Re: [PATCH net-next 2/3] arm64: dts: fsl: ls1028a-rdb: Add ENETC external eth ports for the LS1028A RDB board
From: Andrew Lunn @ 2019-02-14 16:27 UTC (permalink / raw)
To: Claudiu Manoil
Cc: Shawn Guo, Leo Li, David S . Miller, devicetree@vger.kernel.org,
Alexandru Marginean, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org
In-Reply-To: <VI1PR04MB4880C9EF6C9EBFCAA2209DD596670@VI1PR04MB4880.eurprd04.prod.outlook.com>
> Hi Andrew,
>
> The extra node for mdio seems to complicate things somewhat.
> Just adding this node seems not enough. How to find out easily if a child
> of a enetc port node is a mdio node?
You copy somebody else code :-)
https://elixir.bootlin.com/linux/v5.0-rc6/source/drivers/net/dsa/mv88e6xxx/chip.c#L2765
Andrew
^ permalink raw reply
* [PATCH] ethtool: fec: add pretty dump
From: Vivien Didelot @ 2019-02-14 16:15 UTC (permalink / raw)
To: netdev; +Cc: Vivien Didelot, John W . Linville
In-Reply-To: <20190214161536.19646-1-vivien.didelot@gmail.com>
Add pretty dump for the port registers of the "fec" kernel driver. Only
offsets exposed by the driver are dumped by ethtool.
Both register dump versions are supported, version 2 which corresponds
to the kernel compilation test:
#if defined(CONFIG_M523x) || defined(CONFIG_M527x)
|| defined(CONFIG_M528x) || defined(CONFIG_M520x)
|| defined(CONFIG_M532x) || defined(CONFIG_ARM)
|| defined(CONFIG_ARM64) || defined(CONFIG_COMPILE_TEST)
and version 1 which corresponds to the opposite test. At the same time,
detail a few interesting registers of version 2.
Kernels not patched for setting this version will cause ethtool to
dump the whole set of registers as it already does today.
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
Makefile.am | 2 +-
ethtool.c | 1 +
fec.c | 219 ++++++++++++++++++++++++++++++++++++++++++++++++++++
internal.h | 3 +
4 files changed, 224 insertions(+), 1 deletion(-)
create mode 100644 fec.c
diff --git a/Makefile.am b/Makefile.am
index 468eed1fd..0a2fd2917 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -10,7 +10,7 @@ ethtool_SOURCES = ethtool.c ethtool-copy.h internal.h net_tstamp-copy.h \
if ETHTOOL_ENABLE_PRETTY_DUMP
ethtool_SOURCES += \
amd8111e.c de2104x.c dsa.c e100.c e1000.c et131x.c igb.c \
- fec_8xx.c ibm_emac.c ixgb.c ixgbe.c natsemi.c \
+ fec.c fec_8xx.c ibm_emac.c ixgb.c ixgbe.c natsemi.c \
pcnet32.c realtek.c tg3.c marvell.c vioc.c \
smsc911x.c at76c50x-usb.c sfc.c stmmac.c \
sff-common.c sff-common.h sfpid.c sfpdiag.c \
diff --git a/ethtool.c b/ethtool.c
index fb4c0886c..96953ceae 100644
--- a/ethtool.c
+++ b/ethtool.c
@@ -1167,6 +1167,7 @@ static const struct {
{ "fjes", fjes_dump_regs },
{ "lan78xx", lan78xx_dump_regs },
{ "dsa", dsa_dump_regs },
+ { "fec", fec_dump_regs },
#endif
};
diff --git a/fec.c b/fec.c
new file mode 100644
index 000000000..01b1d34cc
--- /dev/null
+++ b/fec.c
@@ -0,0 +1,219 @@
+#include <stdio.h>
+#include <string.h>
+
+#include "internal.h"
+
+/* Macros and dump functions for the 32-bit "fec" driver registers */
+
+#define REG(_reg, _name, _val) \
+ printf("0x%.03x: %-44.44s 0x%.8x\n", _reg, _name, _val)
+
+#define FIELD(_name, _fmt, ...) \
+ printf(" %-47.47s " _fmt "\n", _name, ##__VA_ARGS__)
+
+static void fec_dump_reg_v1(int reg, u32 val)
+{
+ switch (reg) {
+ case 0x000: /* FEC_ECNTRL */
+ case 0x004: /* FEC_IEVENT */
+ case 0x008: /* FEC_IMASK */
+ case 0x00c: /* FEC_IVEC */
+ case 0x010: /* FEC_R_DES_ACTIVE_0 */
+ case 0x014: /* FEC_X_DES_ACTIVE_0 */
+ case 0x040: /* FEC_MII_DATA */
+ case 0x044: /* FEC_MII_SPEED */
+ case 0x08c: /* FEC_R_BOUND */
+ case 0x090: /* FEC_R_FSTART */
+ case 0x0a4: /* FEC_X_WMRK */
+ case 0x0ac: /* FEC_X_FSTART */
+ case 0x104: /* FEC_R_CNTRL */
+ case 0x108: /* FEC_MAX_FRM_LEN */
+ case 0x144: /* FEC_X_CNTRL */
+ case 0x3c0: /* FEC_ADDR_LOW */
+ case 0x3c4: /* FEC_ADDR_HIGH */
+ case 0x3c8: /* FEC_GRP_HASH_TABLE_HIGH */
+ case 0x3cc: /* FEC_GRP_HASH_TABLE_LOW */
+ case 0x3d0: /* FEC_R_DES_START_0 */
+ case 0x3d4: /* FEC_X_DES_START_0 */
+ case 0x3d8: /* FEC_R_BUFF_SIZE_0 */
+ REG(reg, "", val);
+ break;
+ }
+}
+
+static void fec_dump_reg_v2(int reg, u32 val)
+{
+ switch (reg) {
+ case 0x084: /* FEC_R_CNTRL */
+ REG(reg, "RCR (Receive Control Register)", val);
+ FIELD("MAX_FL (Maximum frame length)", "%u", (val & 0x07ff0000) >> 16);
+ FIELD("FCE (Flow control enable)", "%u", !!(val & 0x00000020));
+ FIELD("BC_REJ (Broadcast frame reject)", "%u", !!(val & 0x00000010));
+ FIELD("PROM (Promiscuous mode)", "%u", !!(val & 0x00000008));
+ FIELD("DRT (Disable receive on transmit)", "%u", !!(val & 0x00000002));
+ FIELD("LOOP (Internal loopback)", "%u", !!(val & 0x00000001));
+ break;
+ case 0x0c4: /* FEC_X_CNTRL */
+ REG(reg, "TCR (Transmit Control Register)", val);
+ FIELD("RFC_PAUSE (Receive frame control pause)", "%u", !!(val & 0x00000010));
+ FIELD("TFC_PAUSE (Transmit frame control pause)", "%u", !!(val & 0x00000008));
+ FIELD("FDEN (Full duplex enable)", "%u", !!(val & 0x00000004));
+ FIELD("HBC (Heartbeat control)", "%u", !!(val & 0x00000002));
+ FIELD("GTS (Graceful transmit stop)", "%u", !!(val & 0x00000001));
+ break;
+ case 0x118: /* FEC_HASH_TABLE_HIGH */
+ REG(reg, "IAUR (Individual Address Upper Register)", val);
+ FIELD("IADDR1", "0x%.16llx", (u64)((u64)val) << 32);
+ break;
+ case 0x11c: /* FEC_HASH_TABLE_LOW */
+ REG(reg, "IALR (Individual Address Lower Register)", val);
+ FIELD("IADDR2", "0x%.16x", val);
+ break;
+ case 0x120: /* FEC_GRP_HASH_TABLE_HIGH */
+ REG(reg, "GAUR (Group Address Upper Register)", val);
+ FIELD("GADDR1", "0x%.16llx", (u64)((u64)val) << 32);
+ break;
+ case 0x124: /* FEC_GRP_HASH_TABLE_LOW */
+ REG(reg, "GALR (Group Address Lower Register)", val);
+ FIELD("GADDR2", "0x%.16x", val);
+ break;
+ case 0x144: /* FEC_X_WMRK */
+ REG(reg, "TFWR (Transmit FIFO Watermark Register)", val);
+ FIELD("X_WMRK", "%s",
+ (val & 0x00000003) == 0x00000000 ? "64 bytes" :
+ (val & 0x00000003) == 0x00000002 ? "128 bytes" :
+ (val & 0x00000003) == 0x00000003 ? "192 bytes" : "?");
+ break;
+ case 0x14c: /* FEC_R_BOUND */
+ REG(reg, "FRBR (FIFO Receive Bound Register)", val);
+ FIELD("R_BOUND (Highest valid FIFO RAM address)", "0x%.2x", (val & 0x000003fc) >> 2);
+ break;
+ case 0x188: /* FEC_R_BUFF_SIZE_0 */
+ REG(reg, "EMRBR (Maximum Receive Buffer Size)", val);
+ FIELD("R_BUF_SIZE (Receive buffer size)", "%u", (val & 0x000007f0) >> 4);
+ break;
+ case 0x004: /* FEC_IEVENT */
+ case 0x008: /* FEC_IMASK */
+ case 0x010: /* FEC_R_DES_ACTIVE_0 */
+ case 0x014: /* FEC_X_DES_ACTIVE_0 */
+ case 0x024: /* FEC_ECNTRL */
+ case 0x040: /* FEC_MII_DATA */
+ case 0x044: /* FEC_MII_SPEED */
+ case 0x064: /* FEC_MIB_CTRLSTAT */
+ case 0x0e4: /* FEC_ADDR_LOW */
+ case 0x0e8: /* FEC_ADDR_HIGH */
+ case 0x0ec: /* FEC_OPD */
+ case 0x0f0: /* FEC_TXIC0 */
+ case 0x0f4: /* FEC_TXIC1 */
+ case 0x0f8: /* FEC_TXIC2 */
+ case 0x100: /* FEC_RXIC0 */
+ case 0x104: /* FEC_RXIC1 */
+ case 0x108: /* FEC_RXIC2 */
+ case 0x150: /* FEC_R_FSTART */
+ case 0x160: /* FEC_R_DES_START_1 */
+ case 0x164: /* FEC_X_DES_START_1 */
+ case 0x168: /* FEC_R_BUFF_SIZE_1 */
+ case 0x16c: /* FEC_R_DES_START_2 */
+ case 0x170: /* FEC_X_DES_START_2 */
+ case 0x174: /* FEC_R_BUFF_SIZE_2 */
+ case 0x180: /* FEC_R_DES_START_0 */
+ case 0x184: /* FEC_X_DES_START_0 */
+ case 0x190: /* FEC_R_FIFO_RSFL */
+ case 0x194: /* FEC_R_FIFO_RSEM */
+ case 0x198: /* FEC_R_FIFO_RAEM */
+ case 0x19c: /* FEC_R_FIFO_RAFL */
+ case 0x1c4: /* FEC_RACC */
+ case 0x1c8: /* FEC_RCMR_1 */
+ case 0x1cc: /* FEC_RCMR_2 */
+ case 0x1d8: /* FEC_DMA_CFG_1 */
+ case 0x1dc: /* FEC_DMA_CFG_2 */
+ case 0x1e0: /* FEC_R_DES_ACTIVE_1 */
+ case 0x1e4: /* FEC_X_DES_ACTIVE_1 */
+ case 0x1e8: /* FEC_R_DES_ACTIVE_2 */
+ case 0x1ec: /* FEC_X_DES_ACTIVE_2 */
+ case 0x1f0: /* FEC_QOS_SCHEME */
+ case 0x200: /* RMON_T_DROP */
+ case 0x204: /* RMON_T_PACKETS */
+ case 0x208: /* RMON_T_BC_PKT */
+ case 0x20c: /* RMON_T_MC_PKT */
+ case 0x210: /* RMON_T_CRC_ALIGN */
+ case 0x214: /* RMON_T_UNDERSIZE */
+ case 0x218: /* RMON_T_OVERSIZE */
+ case 0x21c: /* RMON_T_FRAG */
+ case 0x220: /* RMON_T_JAB */
+ case 0x224: /* RMON_T_COL */
+ case 0x228: /* RMON_T_P64 */
+ case 0x22c: /* RMON_T_P65TO127 */
+ case 0x230: /* RMON_T_P128TO255 */
+ case 0x234: /* RMON_T_P256TO511 */
+ case 0x238: /* RMON_T_P512TO1023 */
+ case 0x23c: /* RMON_T_P1024TO2047 */
+ case 0x240: /* RMON_T_P_GTE2048 */
+ case 0x244: /* RMON_T_OCTETS */
+ case 0x248: /* IEEE_T_DROP */
+ case 0x24c: /* IEEE_T_FRAME_OK */
+ case 0x250: /* IEEE_T_1COL */
+ case 0x254: /* IEEE_T_MCOL */
+ case 0x258: /* IEEE_T_DEF */
+ case 0x25c: /* IEEE_T_LCOL */
+ case 0x260: /* IEEE_T_EXCOL */
+ case 0x264: /* IEEE_T_MACERR */
+ case 0x268: /* IEEE_T_CSERR */
+ case 0x26c: /* IEEE_T_SQE */
+ case 0x270: /* IEEE_T_FDXFC */
+ case 0x274: /* IEEE_T_OCTETS_OK */
+ case 0x284: /* RMON_R_PACKETS */
+ case 0x288: /* RMON_R_BC_PKT */
+ case 0x28c: /* RMON_R_MC_PKT */
+ case 0x290: /* RMON_R_CRC_ALIGN */
+ case 0x294: /* RMON_R_UNDERSIZE */
+ case 0x298: /* RMON_R_OVERSIZE */
+ case 0x29c: /* RMON_R_FRAG */
+ case 0x2a0: /* RMON_R_JAB */
+ case 0x2a4: /* RMON_R_RESVD_O */
+ case 0x2a8: /* RMON_R_P64 */
+ case 0x2ac: /* RMON_R_P65TO127 */
+ case 0x2b0: /* RMON_R_P128TO255 */
+ case 0x2b4: /* RMON_R_P256TO511 */
+ case 0x2b8: /* RMON_R_P512TO1023 */
+ case 0x2bc: /* RMON_R_P1024TO2047 */
+ case 0x2c0: /* RMON_R_P_GTE2048 */
+ case 0x2c4: /* RMON_R_OCTETS */
+ case 0x2c8: /* IEEE_R_DROP */
+ case 0x2cc: /* IEEE_R_FRAME_OK */
+ case 0x2d0: /* IEEE_R_CRC */
+ case 0x2d4: /* IEEE_R_ALIGN */
+ case 0x2d8: /* IEEE_R_MACERR */
+ case 0x2dc: /* IEEE_R_FDXFC */
+ case 0x2e0: /* IEEE_R_OCTETS_OK */
+ REG(reg, "", val);
+ break;
+ }
+}
+
+#undef FIELD
+#undef REG
+
+int fec_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs)
+{
+ const u32 *data = (u32 *)regs->data;
+ int offset;
+ u32 val;
+
+ for (offset = 0; offset < regs->len; offset += 4) {
+ val = data[offset / 4];
+
+ switch (regs->version) {
+ case 1:
+ fec_dump_reg_v1(offset, val);
+ break;
+ case 2:
+ fec_dump_reg_v2(offset, val);
+ break;
+ default:
+ return 1;
+ }
+ }
+
+ return 0;
+}
diff --git a/internal.h b/internal.h
index 84b0f9cef..aecf1ce60 100644
--- a/internal.h
+++ b/internal.h
@@ -357,4 +357,7 @@ int lan78xx_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs);
/* Distributed Switch Architecture */
int dsa_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs);
+/* i.MX Fast Ethernet Controller */
+int fec_dump_regs(struct ethtool_drvinfo *info, struct ethtool_regs *regs);
+
#endif /* ETHTOOL_INTERNAL_H__ */
--
2.20.1
^ permalink raw reply related
* [PATCH] net: ethernet: freescale: set FEC ethtool regs version
From: Vivien Didelot @ 2019-02-14 16:15 UTC (permalink / raw)
To: netdev; +Cc: Vivien Didelot, Fugang Duan, David S. Miller, Vivien Didelot
From: Vivien Didelot <vivien.didelot@savoirfairelinux.com>
Currently the ethtool_regs version is set to 0 for FEC devices.
Use this field to store the register dump version exposed by the
kernel. The choosen version 2 corresponds to the kernel compile test:
#if defined(CONFIG_M523x) || defined(CONFIG_M527x)
|| defined(CONFIG_M528x) || defined(CONFIG_M520x)
|| defined(CONFIG_M532x) || defined(CONFIG_ARM)
|| defined(CONFIG_ARM64) || defined(CONFIG_COMPILE_TEST)
and version 1 corresponds to the opposite. Binaries of ethtool unaware
of this version will dump the whole set as usual.
Signed-off-by: Vivien Didelot <vivien.didelot@gmail.com>
---
drivers/net/ethernet/freescale/fec_main.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 2370dc204202..697c2427f2b7 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -2098,6 +2098,7 @@ static int fec_enet_get_regs_len(struct net_device *ndev)
#if defined(CONFIG_M523x) || defined(CONFIG_M527x) || defined(CONFIG_M528x) || \
defined(CONFIG_M520x) || defined(CONFIG_M532x) || defined(CONFIG_ARM) || \
defined(CONFIG_ARM64) || defined(CONFIG_COMPILE_TEST)
+static __u32 fec_enet_register_version = 2;
static u32 fec_enet_register_offset[] = {
FEC_IEVENT, FEC_IMASK, FEC_R_DES_ACTIVE_0, FEC_X_DES_ACTIVE_0,
FEC_ECNTRL, FEC_MII_DATA, FEC_MII_SPEED, FEC_MIB_CTRLSTAT, FEC_R_CNTRL,
@@ -2128,6 +2129,7 @@ static u32 fec_enet_register_offset[] = {
IEEE_R_FDXFC, IEEE_R_OCTETS_OK
};
#else
+static __u32 fec_enet_register_version = 1;
static u32 fec_enet_register_offset[] = {
FEC_ECNTRL, FEC_IEVENT, FEC_IMASK, FEC_IVEC, FEC_R_DES_ACTIVE_0,
FEC_R_DES_ACTIVE_1, FEC_R_DES_ACTIVE_2, FEC_X_DES_ACTIVE_0,
@@ -2149,6 +2151,8 @@ static void fec_enet_get_regs(struct net_device *ndev,
u32 *buf = (u32 *)regbuf;
u32 i, off;
+ regs->version = fec_enet_register_version;
+
memset(buf, 0, regs->len);
for (i = 0; i < ARRAY_SIZE(fec_enet_register_offset); i++) {
--
2.20.1
^ permalink raw reply related
* [net-next, PATCH v2] net: stmmac: use correct define to get rx timestamp on GMAC4
From: Alexandre Torgue @ 2019-02-14 16:03 UTC (permalink / raw)
To: Giuseppe Cavallaro, Jose Abreu, davem
Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel,
Alexandre Torgue
In dwmac4_wrback_get_rx_timestamp_status we looking for a RX timestamp.
For that receive descriptors are handled and so we should use defines
related to receive descriptors. It'll no change the functional behavior
as RDES3_RDES1_VALID=TDES3_RS1V=BIT(26) but it makes code easier to read.
Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
---
changes since v1:
*use le32_to_cpu to handle endianess
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
index 20299f6..90045ff 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
@@ -268,7 +268,7 @@ static int dwmac4_wrback_get_rx_timestamp_status(void *desc, void *next_desc,
int ret = -EINVAL;
/* Get the status from normal w/b descriptor */
- if (likely(p->des3 & TDES3_RS1V)) {
+ if (likely(le32_to_cpu(p->des3) & RDES3_RDES1_VALID)) {
if (likely(le32_to_cpu(p->des1) & RDES1_TIMESTAMP_AVAILABLE)) {
int i = 0;
--
2.7.4
^ permalink raw reply related
* [PATCH net v2] mm: page_alloc: fix ref bias in page_frag_alloc() for 1-byte allocs
From: Jann Horn @ 2019-02-14 16:03 UTC (permalink / raw)
To: David S. Miller, netdev, jannh
Cc: linux-mm, linux-kernel, Michal Hocko, Vlastimil Babka,
Pavel Tatashin, Oscar Salvador, Mel Gorman, alexander.duyck
The basic idea behind ->pagecnt_bias is: If we pre-allocate the maximum
number of references that we might need to create in the fastpath later,
the bump-allocation fastpath only has to modify the non-atomic bias value
that tracks the number of extra references we hold instead of the atomic
refcount. The maximum number of allocations we can serve (under the
assumption that no allocation is made with size 0) is nc->size, so that's
the bias used.
However, even when all memory in the allocation has been given away, a
reference to the page is still held; and in the `offset < 0` slowpath, the
page may be reused if everyone else has dropped their references.
This means that the necessary number of references is actually
`nc->size+1`.
Per Alexander Duyck's request, use PAGE_FRAG_CACHE_MAX_SIZE instead of
nc->size for the bias in the hope of making the generated code slightly
faster.
Luckily, from a quick grep, it looks like the only path that can call
page_frag_alloc(fragsz=1) is TAP with the IFF_NAPI_FRAGS flag, which
requires CAP_NET_ADMIN in the init namespace and is only intended to be
used for kernel testing and fuzzing.
To test for this issue, put a `WARN_ON(page_ref_count(page) == 0)` in the
`offset < 0` path, below the virt_to_page() call, and then repeatedly call
writev() on a TAP device with IFF_TAP|IFF_NO_PI|IFF_NAPI_FRAGS|IFF_NAPI,
with a vector consisting of 15 elements containing 1 byte each.
Signed-off-by: Jann Horn <jannh@google.com>
---
sending to davem as specified by akpm
changed in v2:
- use PAGE_FRAG_CACHE_MAX_SIZE instead of nc->size for refcount bias
(Alexander Duyck)
mm/page_alloc.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 35fdde041f5c..7f79b78bc829 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -4675,11 +4675,11 @@ void *page_frag_alloc(struct page_frag_cache *nc,
/* Even if we own the page, we do not use atomic_set().
* This would break get_page_unless_zero() users.
*/
- page_ref_add(page, size - 1);
+ page_ref_add(page, PAGE_FRAG_CACHE_MAX_SIZE);
/* reset page count bias and offset to start of new frag */
nc->pfmemalloc = page_is_pfmemalloc(page);
- nc->pagecnt_bias = size;
+ nc->pagecnt_bias = PAGE_FRAG_CACHE_MAX_SIZE + 1;
nc->offset = size;
}
@@ -4695,10 +4695,10 @@ void *page_frag_alloc(struct page_frag_cache *nc,
size = nc->size;
#endif
/* OK, page count is 0, we can safely set it */
- set_page_count(page, size);
+ set_page_count(page, PAGE_FRAG_CACHE_MAX_SIZE + 1);
/* reset page count bias and offset to start of new frag */
- nc->pagecnt_bias = size;
+ nc->pagecnt_bias = PAGE_FRAG_CACHE_MAX_SIZE + 1;
offset = size - fragsz;
}
--
2.21.0.rc0.258.g878e2cd30e-goog
^ permalink raw reply related
* dead code in vhost.c
From: Stephen Hemminger @ 2019-02-14 16:03 UTC (permalink / raw)
To: mst, jasowang; +Cc: netdev
Coverity found this obvious bug
________________________________________________________________________________________________________
*** CID 1442593: Control flow issues (DEADCODE)
/drivers/vhost/vhost.c: 1795 in log_used()
1789 ret = translate_desc(vq, (uintptr_t)vq->used + used_offset,
1790 len, iov, 64, VHOST_ACCESS_WO);
1791 if (ret)
1792 return ret;
1793
1794 for (i = 0; i < ret; i++) {
>>> CID 1442593: Control flow issues (DEADCODE)
>>> Execution cannot reach this statement: "ret = log_write_hva(vq, (ui...".
1795 ret = log_write_hva(vq, (uintptr_t)iov[i].iov_base,
1796 iov[i].iov_len);
1797 if (ret)
1798 return ret;
1799 }
1800
^ permalink raw reply
* Re: [PATCH] libertas_tf: remove set but not used variable 'flags'
From: Steve deRosier @ 2019-02-14 15:58 UTC (permalink / raw)
To: YueHaibing
Cc: Kalle Valo, Colin Ian King, linux-wireless, kernel-janitors,
Network Development
In-Reply-To: <20190213014917.164506-1-yuehaibing@huawei.com>
On Tue, Feb 12, 2019 at 5:35 PM YueHaibing <yuehaibing@huawei.com> wrote:
>
> Fixes gcc '-Wunused-but-set-variable' warning:
>
> drivers/net/wireless/marvell/libertas_tf/main.c: In function 'lbtf_rx':
> drivers/net/wireless/marvell/libertas_tf/main.c:554:15: warning:
> variable 'flags' set but not used [-Wunused-but-set-variable]
>
> It never used and can be removed.
>
> Signed-off-by: YueHaibing <yuehaibing@huawei.com>
Acked-by: Steve deRosier <derosier@cal-sierra.com>
^ permalink raw reply
* Re: [net-next, PATCH] net: stmmac: use correct define to get rx timestamp on GMAC4
From: Alexandre Torgue @ 2019-02-14 15:56 UTC (permalink / raw)
To: Jose Abreu, Giuseppe Cavallaro, davem
Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <478975ee-2254-96f2-be45-ab4e9539ad08@synopsys.com>
On 2/14/19 4:30 PM, Jose Abreu wrote:
> On 2/14/2019 3:00 PM, Alexandre Torgue wrote:
>> Hi Jose
>>
>> On 2/14/19 3:18 PM, Jose Abreu wrote:
>>> Hi Alexandre,
>>>
>>> On 2/14/2019 2:12 PM, Alexandre Torgue wrote:
>>>> In dwmac4_wrback_get_rx_timestamp_status we looking for a RX
>>>> timestamp.
>>>> For that receive descriptors are handled and so we should use
>>>> defines
>>>> related to receive descriptors. It'll no change the functional
>>>> behavior
>>>> as RDES3_RDES1_VALID=TDES3_RS1V=BIT(26) but it makes code
>>>> easier to read.
>>>>
>>>> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
>>>>
>>>> diff --git
>>>> a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
>>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
>>>> index 20299f6..9f062b3 100644
>>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
>>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
>>>> @@ -268,7 +268,7 @@ static int
>>>> dwmac4_wrback_get_rx_timestamp_status(void *desc, void
>>>> *next_desc,
>>>> int ret = -EINVAL;
>>>> /* Get the status from normal w/b descriptor */
>>>> - if (likely(p->des3 & TDES3_RS1V)) {
>>>> + if (likely(p->des3 & RDES3_RDES1_VALID)) {
>>>
>>> Shouldn't this also use le32_to_cpu() like bellow ?
>>
>> I agree. I focused on cosmetic but yes you are right, we have to
>> take car about endianness as this IP is used by different
>> processors (using different endianness). I gonna send a v2.
>> I think dwmac4_rx_check_timestamp have the same kind of issue.
>> Another patch should be sent for it. no ?
>
> Yeah. Maybe you can send all of that in this v2 patch also ?
I'll send another one separately, in order to have a correct commit
header. I send it now.
>
> Thanks,
> Jose Miguel Abreu
>
>>
>> regards
>> Alex
>>
>>
>>
>>>
>>> Thanks,
>>> Jose Miguel Abreu
>>>
>>>> if (likely(le32_to_cpu(p->des1) &
>>>> RDES1_TIMESTAMP_AVAILABLE)) {
>>>> int i = 0;
>>>>
^ permalink raw reply
* Re: [PATCH 2/8] net: ethernet: stmmac: update to support all PHY config for stm32mp157c.
From: Christophe ROULLIER @ 2019-02-14 15:51 UTC (permalink / raw)
To: Andrew Lunn
Cc: robh@kernel.org, davem@davemloft.net, joabreu@synopsys.com,
mark.rutland@arm.com, mcoquelin.stm32@gmail.com, Alexandre TORGUE,
Peppe CAVALLARO, linux-stm32@st-md-mailman.stormreply.com,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org
In-Reply-To: <20190214151540.GG708@lunn.ch>
On 2/14/19 4:15 PM, Andrew Lunn wrote:
>> Sorry, I've misunderstood your question ;-)
>>
>> And you spoke about :
>>
>> case PHY_INTERFACE_MODE_RGMII:
>> case PHY_INTERFACE_MODE_RGMII_ID:
>> case PHY_INTERFACE_MODE_RGMII_RXID:
>> case PHY_INTERFACE_MODE_RGMII_TXID:
>>
>> So in my setup I've only RGMII interface, so I've never tested 3 others
>> interfaces (_ID, _RXID, _TXID).
>>
>> So do I need to add cases in my driver ?
>
> Yes. They all indicate the MAC should be using RGMII.
>
> This appears to be an old issue, but now would be a good time to fix
> it.
>
> Andrew
>
Ok Andrew, I will update with these
Thanks
Christophe
^ permalink raw reply
* Re: [rdma-rc PATCH 1/2] cxgb4: export sge_host_page_size to ulds
From: Leon Romanovsky @ 2019-02-14 15:51 UTC (permalink / raw)
To: Raju Rangoju; +Cc: jgg, davem, linux-rdma, netdev, swise
In-Reply-To: <20190214121054.11693-2-rajur@chelsio.com>
[-- Attachment #1: Type: text/plain, Size: 334 bytes --]
On Thu, Feb 14, 2019 at 05:40:53PM +0530, Raju Rangoju wrote:
> Export the sge_host_page_size field to ULDs via
> cxgb4_lld_info, so that iw_cxgb4 can make use of
> this in calculating the correct qp/cq mask.
>
> Fixes: 2391b0030e ("cxgb4: Remove SGE_HOST_PAGE_SIZE dependency on page
> size")
Please don't break Fixes line.
Thanks
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* RE: [PATCH net-next 3/3] enetc: Add ENETC PF level external MDIO support
From: Claudiu Manoil @ 2019-02-14 15:48 UTC (permalink / raw)
To: Andrew Lunn
Cc: Shawn Guo, Leo Li, David S . Miller, devicetree@vger.kernel.org,
Alexandru Marginean, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org
In-Reply-To: <20190213181323.GA708@lunn.ch>
>-----Original Message-----
>From: Andrew Lunn <andrew@lunn.ch>
>Sent: Wednesday, February 13, 2019 8:13 PM
>To: Claudiu Manoil <claudiu.manoil@nxp.com>
>Cc: Shawn Guo <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; David S .
>Miller <davem@davemloft.net>; devicetree@vger.kernel.org; Alexandru
>Marginean <alexandru.marginean@nxp.com>; linux-kernel@vger.kernel.org;
>linux-arm-kernel@lists.infradead.org; netdev@vger.kernel.org
>Subject: Re: [PATCH net-next 3/3] enetc: Add ENETC PF level external MDIO
>support
>
[...]
>> +static int enetc_mdio_write(struct mii_bus *bus, int phy_id, int regnum,
>> + u16 value)
>> +{
>> + struct enetc_mdio_regs __iomem *regs = bus_to_enetc_regs(bus);
>> + u32 mdio_ctl, mdio_cfg;
>> + u16 dev_addr;
>> + int ret;
>> +
>> + mdio_cfg = MDIO_CFG_CLKDIV(ENETC_MDC_DIV) | MDIO_CFG_NEG;
>
>What does MDIO_CFG_NEG mean?
>
I'll add a comment in the code to clarify this define.
It means the MDIO is driven by master on MDC negative edge, which is how
the external mdio busses work on this hardware. For internal MDIO CFG_NEG is 0.
[...]
>
>Maybe MDIO_CFG_ENC could be called MDIO_CFG_C45? Assuming that is what
>it actually means?
>
You're right, in the hardware manual this bit is actually called "ENC45", so
I'll change the define name to that. Should be more clear like this.
[...]
>
>It is a good idea to have an mdio node which contains the list of
>PHYs. You can get into odd situations if you don't do that.
>
Hopefully this doesn't complicate things since these are not platform devices.
It's not as easy as adding new platform device driver for mdio...
Ok for the rest of the comments too.
Thanks for the review.
Claudiu
^ permalink raw reply
* Re: [rdma-rc PATCH 2/2] iw_cxgb4: cq/qp mask depends on bar2 pages in a host page
From: Jason Gunthorpe @ 2019-02-14 15:41 UTC (permalink / raw)
To: Raju Rangoju
Cc: davem@davemloft.net, linux-rdma@vger.kernel.org,
netdev@vger.kernel.org, swise@opengridcomputing.com
In-Reply-To: <20190214121054.11693-3-rajur@chelsio.com>
On Thu, Feb 14, 2019 at 05:40:54PM +0530, Raju Rangoju wrote:
> Adjust the cq/qp mask based on no.of bar2 pages in a host page.
>
> For user-mode rdma, the granularity of the BAR2 memory mapped
> to a user rdma process during queue allocation must be based
> on the host page size. The lld attributes udb_density and
> ucq_density are used to figure out how many sge contexts are
> in a bar2 page. So the rdev->qpmask and rdev->cqmask in
> iw_cxgb4 need to now be adjusted based on how many sge bar2
> pages are in a host page.
Why is this rc? Do certain arches fail to work or something?
Jason
^ permalink raw reply
* Re: [PATCH] mm: page_alloc: fix ref bias in page_frag_alloc() for 1-byte allocs
From: Alexander Duyck @ 2019-02-14 15:37 UTC (permalink / raw)
To: Jann Horn
Cc: linux-mm, Andrew Morton, LKML, Michal Hocko, Vlastimil Babka,
Pavel Tatashin, Oscar Salvador, Mel Gorman, Aaron Lu, Netdev,
Alexander Duyck
In-Reply-To: <CAG48ez0v7QwtCKDs5vgRJht8yfZR5nudEpkMOLaDX-=47WeFqA@mail.gmail.com>
On Thu, Feb 14, 2019 at 7:13 AM Jann Horn <jannh@google.com> wrote:
>
> On Wed, Feb 13, 2019 at 11:42 PM Alexander Duyck
> <alexander.duyck@gmail.com> wrote:
> > On Wed, Feb 13, 2019 at 12:42 PM Jann Horn <jannh@google.com> wrote:
> > > The basic idea behind ->pagecnt_bias is: If we pre-allocate the maximum
> > > number of references that we might need to create in the fastpath later,
> > > the bump-allocation fastpath only has to modify the non-atomic bias value
> > > that tracks the number of extra references we hold instead of the atomic
> > > refcount. The maximum number of allocations we can serve (under the
> > > assumption that no allocation is made with size 0) is nc->size, so that's
> > > the bias used.
> > >
> > > However, even when all memory in the allocation has been given away, a
> > > reference to the page is still held; and in the `offset < 0` slowpath, the
> > > page may be reused if everyone else has dropped their references.
> > > This means that the necessary number of references is actually
> > > `nc->size+1`.
> > >
> > > Luckily, from a quick grep, it looks like the only path that can call
> > > page_frag_alloc(fragsz=1) is TAP with the IFF_NAPI_FRAGS flag, which
> > > requires CAP_NET_ADMIN in the init namespace and is only intended to be
> > > used for kernel testing and fuzzing.
> >
> > Actually that has me somewhat concerned. I wouldn't be surprised if
> > most drivers expect the netdev_alloc_frags call to at least output an
> > SKB_DATA_ALIGN sized value.
> >
> > We probably should update __netdev_alloc_frag and __napi_alloc_frag so
> > that they will pass fragsz through SKB_DATA_ALIGN.
>
> Do you want to do a separate patch for that? I'd like to not mix
> logically separate changes in a single patch, and I also don't have a
> good understanding of the alignment concerns here.
You could just include it as a separate patch with your work.
Otherwise I will get to it when I have time.
The point is the issue you pointed out will actually cause other
issues if the behavior is maintained since you shouldn't be getting
unaligned blocks out of the frags API anyway.
> > > To test for this issue, put a `WARN_ON(page_ref_count(page) == 0)` in the
> > > `offset < 0` path, below the virt_to_page() call, and then repeatedly call
> > > writev() on a TAP device with IFF_TAP|IFF_NO_PI|IFF_NAPI_FRAGS|IFF_NAPI,
> > > with a vector consisting of 15 elements containing 1 byte each.
> > >
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Jann Horn <jannh@google.com>
> > > ---
> > > mm/page_alloc.c | 8 ++++----
> > > 1 file changed, 4 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > > index 35fdde041f5c..46285d28e43b 100644
> > > --- a/mm/page_alloc.c
> > > +++ b/mm/page_alloc.c
> > > @@ -4675,11 +4675,11 @@ void *page_frag_alloc(struct page_frag_cache *nc,
> > > /* Even if we own the page, we do not use atomic_set().
> > > * This would break get_page_unless_zero() users.
> > > */
> > > - page_ref_add(page, size - 1);
> > > + page_ref_add(page, size);
> > >
> > > /* reset page count bias and offset to start of new frag */
> > > nc->pfmemalloc = page_is_pfmemalloc(page);
> > > - nc->pagecnt_bias = size;
> > > + nc->pagecnt_bias = size + 1;
> > > nc->offset = size;
> > > }
> > >
> > > @@ -4695,10 +4695,10 @@ void *page_frag_alloc(struct page_frag_cache *nc,
> > > size = nc->size;
> > > #endif
> > > /* OK, page count is 0, we can safely set it */
> > > - set_page_count(page, size);
> > > + set_page_count(page, size + 1);
> > >
> > > /* reset page count bias and offset to start of new frag */
> > > - nc->pagecnt_bias = size;
> > > + nc->pagecnt_bias = size + 1;
> > > offset = size - fragsz;
> > > }
> >
> > If we already have to add a constant it might be better to just use
> > PAGE_FRAG_CACHE_MAX_SIZE + 1 in all these spots where you are having
> > to use "size + 1" instead of "size". That way we can avoid having to
> > add a constant to a register value and then program that value.
> > instead we can just assign the constant value right from the start.
>
> I doubt that these few instructions make a difference, but sure, I can
> send a v2 with that changed.
You would be surprised. They all end up adding up over time.
^ permalink raw reply
* RE: [PATCH net-next 2/3] arm64: dts: fsl: ls1028a-rdb: Add ENETC external eth ports for the LS1028A RDB board
From: Claudiu Manoil @ 2019-02-14 15:33 UTC (permalink / raw)
To: Andrew Lunn
Cc: Shawn Guo, Leo Li, David S . Miller, devicetree@vger.kernel.org,
Alexandru Marginean, linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org
In-Reply-To: <20190213181554.GB708@lunn.ch>
>-----Original Message-----
>From: Andrew Lunn <andrew@lunn.ch>
>Sent: Wednesday, February 13, 2019 8:16 PM
>To: Claudiu Manoil <claudiu.manoil@nxp.com>
>Cc: Shawn Guo <shawnguo@kernel.org>; Leo Li <leoyang.li@nxp.com>; David S .
>Miller <davem@davemloft.net>; devicetree@vger.kernel.org; Alexandru
>Marginean <alexandru.marginean@nxp.com>; linux-kernel@vger.kernel.org;
>linux-arm-kernel@lists.infradead.org; netdev@vger.kernel.org
>Subject: Re: [PATCH net-next 2/3] arm64: dts: fsl: ls1028a-rdb: Add ENETC
>external eth ports for the LS1028A RDB board
>
>On Wed, Feb 13, 2019 at 01:02:22PM +0200, Claudiu Manoil wrote:
>> The LS1028A RDB board features an Atheros PHY connected over SGMII to
>> the ENETC PF0 (or Port0). ENETC Port1 (PF1) has no external
>> connection on this board, so it can be disabled for now.
>>
>> Signed-off-by: Alex Marginean <alexandru.marginean@nxp.com>
>> Signed-off-by: Claudiu Manoil <claudiu.manoil@nxp.com>
>> ---
>> arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts | 15
>> +++++++++++++++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
>> b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
>> index fdeb417..c8487893 100644
>> --- a/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
>> +++ b/arch/arm64/boot/dts/freescale/fsl-ls1028a-rdb.dts
>> @@ -71,3 +71,18 @@
>> &duart1 {
>> status = "okay";
>> };
>> +
>> +&enetc_port0 {
>> + phy-handle = <&sgmii_phy0>;
>> + phy-connection-type = "sgmii";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + sgmii_phy0: ethernet-phy@2 {
>> + reg = <0x2>;
>> + };
>> +};
>> +
>
>Hi Claudiu
>
>It is better to use:
>
>&enetc_port0 {
> phy-handle = <&sgmii_phy0>;
> phy-connection-type = "sgmii";
> #address-cells = <1>;
> #size-cells = <0>;
>
> mdio {
> sgmii_phy0: ethernet-phy@2 {
> reg = <0x2>;
> };
> };
>};
Hi Andrew,
The extra node for mdio seems to complicate things somewhat.
Just adding this node seems not enough. How to find out easily if a child
of a enetc port node is a mdio node? I was thinking to use device_type
for that, like this:
&enetc_port0 {
phy-handle = <&sgmii_phy0>;
phy-connection-type = "sgmii";
mdio {
#address-cells = <1>;
#size-cells = <0>;
device_type = "mdio";
sgmii_phy0: ethernet-phy@2 {
reg = <0x2>;
};
};
};
Would you agree with this?
Thanks,
Claudiu
^ permalink raw reply
* Re: [net-next, PATCH] net: stmmac: use correct define to get rx timestamp on GMAC4
From: Jose Abreu @ 2019-02-14 15:30 UTC (permalink / raw)
To: Alexandre Torgue, Jose Abreu, Giuseppe Cavallaro, davem
Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <a62c35de-7afa-14ec-5ca6-05bd61b6d09d@st.com>
On 2/14/2019 3:00 PM, Alexandre Torgue wrote:
> Hi Jose
>
> On 2/14/19 3:18 PM, Jose Abreu wrote:
>> Hi Alexandre,
>>
>> On 2/14/2019 2:12 PM, Alexandre Torgue wrote:
>>> In dwmac4_wrback_get_rx_timestamp_status we looking for a RX
>>> timestamp.
>>> For that receive descriptors are handled and so we should use
>>> defines
>>> related to receive descriptors. It'll no change the functional
>>> behavior
>>> as RDES3_RDES1_VALID=TDES3_RS1V=BIT(26) but it makes code
>>> easier to read.
>>>
>>> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
>>>
>>> diff --git
>>> a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
>>> b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
>>> index 20299f6..9f062b3 100644
>>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
>>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
>>> @@ -268,7 +268,7 @@ static int
>>> dwmac4_wrback_get_rx_timestamp_status(void *desc, void
>>> *next_desc,
>>> int ret = -EINVAL;
>>> /* Get the status from normal w/b descriptor */
>>> - if (likely(p->des3 & TDES3_RS1V)) {
>>> + if (likely(p->des3 & RDES3_RDES1_VALID)) {
>>
>> Shouldn't this also use le32_to_cpu() like bellow ?
>
> I agree. I focused on cosmetic but yes you are right, we have to
> take car about endianness as this IP is used by different
> processors (using different endianness). I gonna send a v2.
> I think dwmac4_rx_check_timestamp have the same kind of issue.
> Another patch should be sent for it. no ?
Yeah. Maybe you can send all of that in this v2 patch also ?
Thanks,
Jose Miguel Abreu
>
> regards
> Alex
>
>
>
>>
>> Thanks,
>> Jose Miguel Abreu
>>
>>> if (likely(le32_to_cpu(p->des1) &
>>> RDES1_TIMESTAMP_AVAILABLE)) {
>>> int i = 0;
>>>
^ permalink raw reply
* Re: KASAN: use-after-free Read in sctp_outq_tail
From: Marcelo Ricardo Leitner @ 2019-02-14 15:27 UTC (permalink / raw)
To: Xin Long
Cc: syzbot, davem, LKML, linux-sctp, network dev, Neil Horman,
syzkaller-bugs, Vlad Yasevich
In-Reply-To: <CADvbK_ctRtcV3Fz6CwAoiTtVH3h5u7gj=6BsiZSR3Qdq+2rDMA@mail.gmail.com>
On Thu, Feb 14, 2019 at 10:52:00PM +0800, Xin Long wrote:
> On Thu, Feb 14, 2019 at 10:39 PM Marcelo Ricardo Leitner
> <marcelo.leitner@gmail.com> wrote:
> >
> > On Thu, Feb 14, 2019 at 12:17:45PM -0200, Marcelo Ricardo Leitner wrote:
> > > On Thu, Feb 14, 2019 at 10:03:30PM +0800, Xin Long wrote:
> > > > On Wed, Feb 13, 2019 at 9:52 PM Marcelo Ricardo Leitner
> > > > <marcelo.leitner@gmail.com> wrote:
> > > > >
> > > > > On Wed, Feb 13, 2019 at 12:35:56PM +0800, Xin Long wrote:
> > > > > > On Wed, Feb 13, 2019 at 4:00 AM syzbot
> > > > > > <syzbot+7823fa3f3e2d69341ea8@syzkaller.appspotmail.com> wrote:
> > > > > > >
> > > > > > > Hello,
> > > > > > >
> > > > > > > syzbot found the following crash on:
> > > > > > >
> > > > > > > HEAD commit: d4104460aec1 Add linux-next specific files for 20190211
> > > > > > > git tree: linux-next
> > > > > > > console output: https://syzkaller.appspot.com/x/log.txt?x=14140124c00000
> > > > > > > kernel config: https://syzkaller.appspot.com/x/.config?x=c8a112d3b0d6719b
> > > > > > > dashboard link: https://syzkaller.appspot.com/bug?extid=7823fa3f3e2d69341ea8
> > > > > > > compiler: gcc (GCC) 9.0.0 20181231 (experimental)
> > > > > > >
> > > > > > > Unfortunately, I don't have any reproducer for this crash yet.
> > > > > > >
> > > > > > > IMPORTANT: if you fix the bug, please add the following tag to the commit:
> > > > > > > Reported-by: syzbot+7823fa3f3e2d69341ea8@syzkaller.appspotmail.com
> > > > > > >
> > > > > > > ==================================================================
> > > > > > > BUG: KASAN: use-after-free in list_add_tail include/linux/list.h:93 [inline]
> > > > > > > BUG: KASAN: use-after-free in sctp_outq_tail_data net/sctp/outqueue.c:105
> > > > > > > [inline]
> > > > > > > BUG: KASAN: use-after-free in sctp_outq_tail+0x816/0x930
> > > > > > > net/sctp/outqueue.c:313
> > > > > > > Read of size 8 at addr ffff88807b19a7b8 by task syz-executor.0/30745
> > > > > > I think https://patchwork.ozlabs.org/patch/1040500/ will fix this.
> > > > >
> > > > > I don't think so. Seems it will switch from use-after-free to NULL deref
> > > > > instead with that patch.
> > > > It will allocate ext for the stream if its ext is NULL in
> > > > sctp_sendmsg_to_asoc(), the new data will work fine. As for
> > >
> > > Ehh, right!
> > >
> > > > the old data on the surplus streams, it has been dropped in
> > > > sctp_stream_outq_migrate().
> > >
> > > Yep.
> > >
> > > >
> > > > >
> > > > > > > CPU: 1 PID: 30745 Comm: syz-executor.0 Not tainted 5.0.0-rc5-next-20190211
> > > > > > > #32
> > > > > > > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
> > > > > > > Google 01/01/2011
> > > > > > > Call Trace:
> > > > > > > __dump_stack lib/dump_stack.c:77 [inline]
> > > > > > > dump_stack+0x172/0x1f0 lib/dump_stack.c:113
> > > > > > > print_address_description.cold+0x7c/0x20d mm/kasan/report.c:187
> > > > > > > kasan_report.cold+0x1b/0x40 mm/kasan/report.c:317
> > > > > > > __asan_report_load8_noabort+0x14/0x20 mm/kasan/generic_report.c:132
> > > > > > > list_add_tail include/linux/list.h:93 [inline]
> > > > > > > sctp_outq_tail_data net/sctp/outqueue.c:105 [inline]
> > > > > > > sctp_outq_tail+0x816/0x930 net/sctp/outqueue.c:313
> > > > > > > sctp_cmd_send_msg net/sctp/sm_sideeffect.c:1109 [inline]
> > > > > > > sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1784 [inline]
> > > > > > > sctp_side_effects net/sctp/sm_sideeffect.c:1220 [inline]
> > > > > > > sctp_do_sm+0x68e/0x5380 net/sctp/sm_sideeffect.c:1191
> > > > > > > sctp_primitive_SEND+0xa0/0xd0 net/sctp/primitive.c:178
> > > > > > > sctp_sendmsg_to_asoc+0xa63/0x17b0 net/sctp/socket.c:1955
> > > > >
> > > > > sctp_sendmsg_to_asoc()
> > > > > ...
> > > > > if (sinfo->sinfo_stream >= asoc->stream.outcnt) {
> > > > > err = -EINVAL;
> > > > > goto err;
> > > > > }
> > > > >
> > > > > if (unlikely(!SCTP_SO(&asoc->stream, sinfo->sinfo_stream)->ext)) {
> > > > > ...
> > > > >
> > > > > It should have aborted even if an old ->ext was still there because
> > > > > outcnt is correctly updated. So somehow outcnt was wrong here.
> > > > >
> > > > > sctp_stream_init()
> > > > > ...
> > > > > /* Filter out chunks queued on streams that won't exist anymore */
> > > > > sched->unsched_all(stream);
> > > > > sctp_stream_outq_migrate(stream, NULL, outcnt); <--- [A]
> > > > > sched->sched_all(stream);
> > > > >
> > > > > ret = sctp_stream_alloc_out(stream, outcnt, gfp); <--- [B]
> > > > > if (ret)
> > > > > goto out;
> > > > >
> > > > > stream->outcnt = outcnt; <--- [C]
> > > > > ...
> > > > >
> > > > > We have a problem here because [A] is freeing ->ext, but [B] can fail (ENOMEM),
> > > > > which would lead it to not update outcnt in [C] even after the
> > > > > changes already performed in [A].
> > > > >
> > > > > It should handle the freeing of ->ext in sctp_stream_alloc_out()
> > > > > instead, or allocate the flexarray earlier (so it can bail out before
> > > > > freeing stuff).
> > > > Agree that it's better to do:
> > > > sched->unsched_all(stream);
> > > > sctp_stream_outq_migrate(stream, NULL, outcnt);
> > > > sched->sched_all(stream);
> > > > after the flexarray allocation.
> > > >
> > > > Just sctp_stream_alloc_out() can not be used here anymore, as
> > > > stream->out will be set in it.
> > >
> > > What about carving out a sctp_stream_init_out() ? Like
> > >
> > > struct flex_array *new_out;
> > >
> > > /* just allocate it, nothing more */
> > > new_out = sctp_stream_alloc_out(outcnt, gfp);
> > > if (!new_out)
> > > goto out;
> > >
> > > /* Filter out chunks queued on streams that won't exist anymore */
> > > sched->unsched_all(stream);
> > > sctp_stream_outq_migrate(stream, NULL, outcnt);
> > > sched->sched_all(stream);
> > >
> > > /* initialization stuff, and it can't fail anymore */
> > > sctp_stream_init_out(stream, outcnt, new_out);
> > >
> > > This may hurt a bit more with the genradix migration, but then we
> > > don't end up dropping data for nothing.
> >
> > Reviewing calls to this function, if this allocation fails it will
> > either cause the asoc to be terminated or sendmsg error. So data loss
> > is not really an issue here.
> >
> sctp_stream_init+0xbc/0x410 net/sctp/stream.c:139
> sctp_process_init+0x21c3/0x2b20 net/sctp/sm_make_chunk.c:2466
> sctp_cmd_process_init net/sctp/sm_sideeffect.c:682 [inline]
> sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1410 [inline]
>
> on this path, seems that it won't terminate the asoc nor report a sending error.
Seems that's only triggered from sctp_sf_do_5_1C_ack() and quite
interesting. If we can't perform the reduction of the number of output
streams, we will be violating the handshake. Considering the error
will cause it to stop processing the commands from the state machine,
it won't output cookie echo pkt and this asoc will be left in a
somewhat funny state. I think the fact that it won't stop the T1
then, allows it to get corrected later on.
But that err_chunk, if any, gets leaked.
^ permalink raw reply
* Re: [PATCH net] dsa: mv88e6xxx: Ensure all pending interrupts are handled prior to exit
From: David Miller @ 2019-02-14 15:27 UTC (permalink / raw)
To: andrew; +Cc: dave.anglin, linux, vivien.didelot, f.fainelli, netdev
In-Reply-To: <20190214045019.GC20024@lunn.ch>
From: Andrew Lunn <andrew@lunn.ch>
Date: Thu, 14 Feb 2019 05:50:19 +0100
>> Should I queue just this one for -stable? I didn't queue up Heiner's change for
>> -stable because it fixes a 5.0-rcX regression.
>
> Yes please.
Done.
^ permalink raw reply
* Re: TC stats / hw offload question
From: Andy Gospodarek @ 2019-02-14 15:17 UTC (permalink / raw)
To: Jamal Hadi Salim
Cc: Edward Cree, netdev, Jiri Pirko, Cong Wang, Or Gerlitz,
PJ Waskiewicz, Anjali Singhai Jain, Jakub Kicinski
In-Reply-To: <702dd5b7-c6ed-b669-8270-d44f5ff4fb30@mojatatu.com>
On Thu, Feb 14, 2019 at 07:39:28AM -0500, Jamal Hadi Salim wrote:
>
> On 2019-02-11 6:44 a.m., Edward Cree wrote:
>
> > But since the
> > other vendors don't seem to do that, I wondered if there was a
> > reason, or if perhaps the counter resources (and PCI bw to read
> > them) could be saved if all those separate counters aren't really
> > needed.
>
> Probably nobody has paid attention or asked as you did.
That's not totally true, but I'm glad to hear that others are
considering it.
> Will let the h/w folks speak for themselves. My understanding
> based on experience is counters are cheap. Most modern NICs
> and ASICs have a gazillion of them at their disposal.
Counters can be cheap to implement (though that does not always mean
that everyone chooses to add many of them to hardware), but the real
cost to them, as Edward points out, is the cost of accessing them an
keeping them up to date. If we are concerned about keeping track of
flow counters on thousands (or more) flows the cost on the PCI bus can
be quite high.
We have been looking at a few options to deal with tracking this massive
number of counts, but are open to hearing what others feel they would
like to see happen to this. Though stats sometimes seem boring to some
developers, they are critical and we have been considering whether or
not we need to think about different driver or global infra to handle
it.
^ permalink raw reply
* Re: [PATCH 2/8] net: ethernet: stmmac: update to support all PHY config for stm32mp157c.
From: Andrew Lunn @ 2019-02-14 15:15 UTC (permalink / raw)
To: Christophe ROULLIER
Cc: robh@kernel.org, davem@davemloft.net, joabreu@synopsys.com,
mark.rutland@arm.com, mcoquelin.stm32@gmail.com, Alexandre TORGUE,
Peppe CAVALLARO, linux-stm32@st-md-mailman.stormreply.com,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, netdev@vger.kernel.org
In-Reply-To: <02cd7773-899f-b28a-ff61-849b17cbe82d@st.com>
> Sorry, I've misunderstood your question ;-)
>
> And you spoke about :
>
> case PHY_INTERFACE_MODE_RGMII:
> case PHY_INTERFACE_MODE_RGMII_ID:
> case PHY_INTERFACE_MODE_RGMII_RXID:
> case PHY_INTERFACE_MODE_RGMII_TXID:
>
> So in my setup I've only RGMII interface, so I've never tested 3 others
> interfaces (_ID, _RXID, _TXID).
>
> So do I need to add cases in my driver ?
Yes. They all indicate the MAC should be using RGMII.
This appears to be an old issue, but now would be a good time to fix
it.
Andrew
^ permalink raw reply
* Re: [PATCH] mm: page_alloc: fix ref bias in page_frag_alloc() for 1-byte allocs
From: Jann Horn @ 2019-02-14 15:13 UTC (permalink / raw)
To: Alexander Duyck
Cc: linux-mm, Andrew Morton, LKML, Michal Hocko, Vlastimil Babka,
Pavel Tatashin, Oscar Salvador, Mel Gorman, Aaron Lu, Netdev,
Alexander Duyck
In-Reply-To: <CAKgT0Uc7wheUjStv5a4BSNv_=-iu1Ttdj9f_10CdR_oc2BhVig@mail.gmail.com>
On Wed, Feb 13, 2019 at 11:42 PM Alexander Duyck
<alexander.duyck@gmail.com> wrote:
> On Wed, Feb 13, 2019 at 12:42 PM Jann Horn <jannh@google.com> wrote:
> > The basic idea behind ->pagecnt_bias is: If we pre-allocate the maximum
> > number of references that we might need to create in the fastpath later,
> > the bump-allocation fastpath only has to modify the non-atomic bias value
> > that tracks the number of extra references we hold instead of the atomic
> > refcount. The maximum number of allocations we can serve (under the
> > assumption that no allocation is made with size 0) is nc->size, so that's
> > the bias used.
> >
> > However, even when all memory in the allocation has been given away, a
> > reference to the page is still held; and in the `offset < 0` slowpath, the
> > page may be reused if everyone else has dropped their references.
> > This means that the necessary number of references is actually
> > `nc->size+1`.
> >
> > Luckily, from a quick grep, it looks like the only path that can call
> > page_frag_alloc(fragsz=1) is TAP with the IFF_NAPI_FRAGS flag, which
> > requires CAP_NET_ADMIN in the init namespace and is only intended to be
> > used for kernel testing and fuzzing.
>
> Actually that has me somewhat concerned. I wouldn't be surprised if
> most drivers expect the netdev_alloc_frags call to at least output an
> SKB_DATA_ALIGN sized value.
>
> We probably should update __netdev_alloc_frag and __napi_alloc_frag so
> that they will pass fragsz through SKB_DATA_ALIGN.
Do you want to do a separate patch for that? I'd like to not mix
logically separate changes in a single patch, and I also don't have a
good understanding of the alignment concerns here.
> > To test for this issue, put a `WARN_ON(page_ref_count(page) == 0)` in the
> > `offset < 0` path, below the virt_to_page() call, and then repeatedly call
> > writev() on a TAP device with IFF_TAP|IFF_NO_PI|IFF_NAPI_FRAGS|IFF_NAPI,
> > with a vector consisting of 15 elements containing 1 byte each.
> >
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Jann Horn <jannh@google.com>
> > ---
> > mm/page_alloc.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> > index 35fdde041f5c..46285d28e43b 100644
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -4675,11 +4675,11 @@ void *page_frag_alloc(struct page_frag_cache *nc,
> > /* Even if we own the page, we do not use atomic_set().
> > * This would break get_page_unless_zero() users.
> > */
> > - page_ref_add(page, size - 1);
> > + page_ref_add(page, size);
> >
> > /* reset page count bias and offset to start of new frag */
> > nc->pfmemalloc = page_is_pfmemalloc(page);
> > - nc->pagecnt_bias = size;
> > + nc->pagecnt_bias = size + 1;
> > nc->offset = size;
> > }
> >
> > @@ -4695,10 +4695,10 @@ void *page_frag_alloc(struct page_frag_cache *nc,
> > size = nc->size;
> > #endif
> > /* OK, page count is 0, we can safely set it */
> > - set_page_count(page, size);
> > + set_page_count(page, size + 1);
> >
> > /* reset page count bias and offset to start of new frag */
> > - nc->pagecnt_bias = size;
> > + nc->pagecnt_bias = size + 1;
> > offset = size - fragsz;
> > }
>
> If we already have to add a constant it might be better to just use
> PAGE_FRAG_CACHE_MAX_SIZE + 1 in all these spots where you are having
> to use "size + 1" instead of "size". That way we can avoid having to
> add a constant to a register value and then program that value.
> instead we can just assign the constant value right from the start.
I doubt that these few instructions make a difference, but sure, I can
send a v2 with that changed.
^ permalink raw reply
* [PATCH net] net: adaptec: starfire: replace dev_kfree_skb_irq by dev_consume_skb_irq for drop profiles
From: Yang Wei @ 2019-02-14 15:06 UTC (permalink / raw)
To: netdev; +Cc: davem, ionut, yang.wei9, albin_yang
From: Yang Wei <yang.wei9@zte.com.cn>
dev_consume_skb_irq() should be called in intr_handler() when skb
xmit done. It makes drop profiles(dropwatch, perf) more friendly.
Signed-off-by: Yang Wei <yang.wei9@zte.com.cn>
---
drivers/net/ethernet/adaptec/starfire.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/adaptec/starfire.c b/drivers/net/ethernet/adaptec/starfire.c
index 097467f..816540e 100644
--- a/drivers/net/ethernet/adaptec/starfire.c
+++ b/drivers/net/ethernet/adaptec/starfire.c
@@ -1390,7 +1390,7 @@ static irqreturn_t intr_handler(int irq, void *dev_instance)
}
}
- dev_kfree_skb_irq(skb);
+ dev_consume_skb_irq(skb);
}
np->tx_done_q[np->tx_done].status = 0;
np->tx_done = (np->tx_done + 1) % DONE_Q_SIZE;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] net: phy: at803x: disable delay only for RGMII mode
From: Niklas Cassel @ 2019-02-14 15:06 UTC (permalink / raw)
To: Peter Ujfalusi
Cc: Marc Gonzalez, Andrew Lunn, Florian Fainelli, Vinod Koul,
David S Miller, linux-arm-msm, Bjorn Andersson, netdev,
Nori, Sekhar
In-Reply-To: <96271de7-bda1-a86a-a78e-e132bc097efb@ti.com>
On Thu, Feb 14, 2019 at 03:22:28PM +0200, Peter Ujfalusi wrote:
> Hi Niklas,
>
> On 14/02/2019 14.39, Niklas Cassel wrote:
> >>> So, I've rebased your old patch, see attachment.
> >>> I suggest that Peter test it on am335x-evm.
> >>
> >> with the patch + s/rgmii-txid/rgmii-id in the am335x-evmsk.dts ethernet
> >> is working.
> >> I don't have am335x-evm to test, but it has the same PHY as evmsk.
> >>
> >
> > Florian's concern was that this PHY driver looked at "phy-mode" from the
> > perspective of the MAC rather than the PHY.
> > However, if s/rgmii-txid/rgmii-id is the correct fix for am335x-evm,
> > then this means that this PHY driver was just broken.
> >
> > If the driver had misinterpreted the perspective, then the correct
> > fix for am335x-evm would have been s/rgmii-txid/rgmii-rxid.
>
> Not sure if I got this right, but:
> rgmii-id/txid/rxid is the delay mode between PHY and MAC, right?
> on the PHY node it is from the PHY perspective, right?
Yes, from the PHY perspective.
(According to Florian, IIUC, some old PHY drivers were implemented before
it was decided that it is from PHY perspective, rather than from MAC
perspective.)
>
> The errata I have mentioned for am335x say:
> "The reset state of RGMII1_IDMODE (bit 4) and RGMII2_IDMODE (bit 5) in
> the GMII_SEL register enables internal delay mode on the transmit clock
> of the respective RGMII port. The AM335x device does not support
> internal delay mode, so RGMII1_IDMODE and RGMII2_IDMODE must be set to 1b."
>
> If the delay mode on the transmit clock is not working on the am335x,
> then this translate that the rxid needs to be enabled on the PHY side?
IIUC what Florian explained, then either MAC or PHY needs to add delays,
so if the PHY only adds delay on e.g. TX, then the MAC needs to add delay
on RX.
However, in your case, the errata says that your MAC is not capable of
adding a delay on TX, therefore the PHY needs to add a delay on TX.
>
> But then why it worked when only the txid was enabled and rxid was not
> on the PHY side, and why it works if both txid and rxid is enabled?
Because the PHY driver was broken, so the PHY driver always enabled
delays on both TX and RX.
This is how the driver looked before Vinod's change:
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_RXID ||
phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
ret = at803x_enable_rx_delay(phydev);
if (ret < 0)
return ret;
}
if (phydev->interface == PHY_INTERFACE_MODE_RGMII_TXID ||
phydev->interface == PHY_INTERFACE_MODE_RGMII_ID) {
ret = at803x_enable_tx_delay(phydev);
if (ret < 0)
return ret;
}
Yet, the initial value for this PHY is that both TX and RX delay is
enabled, and since this driver never disabled TX/RX delays,
the TX and RX delays were always enabled, no matter what phy-mode
you specified.
>
> Just tried w/ your patch and setting rgmii-rxid for am335x-evmsk and
> ethernet is not working, it only works w/ rgmii-id (so both tx and rx
> delay is enabled on the PHY side?)
>
> > So considering that this driver seems to be really broken
> > (rather then just inverted perspective),
> > perhaps we can merge the patch I attached in my previous email after all?
> > (Together with a s/rgmii-txid/rgmii-id in the am335x-evmsk.dts.)
>
> at the same time am335x-evm.dts needs to have the same change and most
> likely other boards which uses the same PHY needs to be checked?
>
> PS: sorry for my lack of knowledge on the networking stuff...
>
> - Péter
>
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
^ permalink raw reply
* Re: [net-next, PATCH] net: stmmac: use correct define to get rx timestamp on GMAC4
From: Alexandre Torgue @ 2019-02-14 15:00 UTC (permalink / raw)
To: Jose Abreu, Giuseppe Cavallaro, davem
Cc: netdev, linux-stm32, linux-arm-kernel, linux-kernel
In-Reply-To: <7c03dff4-5185-dbd2-eeb4-867972512f2b@synopsys.com>
Hi Jose
On 2/14/19 3:18 PM, Jose Abreu wrote:
> Hi Alexandre,
>
> On 2/14/2019 2:12 PM, Alexandre Torgue wrote:
>> In dwmac4_wrback_get_rx_timestamp_status we looking for a RX timestamp.
>> For that receive descriptors are handled and so we should use defines
>> related to receive descriptors. It'll no change the functional behavior
>> as RDES3_RDES1_VALID=TDES3_RS1V=BIT(26) but it makes code easier to read.
>>
>> Signed-off-by: Alexandre Torgue <alexandre.torgue@st.com>
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
>> index 20299f6..9f062b3 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c
>> @@ -268,7 +268,7 @@ static int dwmac4_wrback_get_rx_timestamp_status(void *desc, void *next_desc,
>> int ret = -EINVAL;
>>
>> /* Get the status from normal w/b descriptor */
>> - if (likely(p->des3 & TDES3_RS1V)) {
>> + if (likely(p->des3 & RDES3_RDES1_VALID)) {
>
> Shouldn't this also use le32_to_cpu() like bellow ?
I agree. I focused on cosmetic but yes you are right, we have to take
car about endianness as this IP is used by different processors (using
different endianness). I gonna send a v2.
I think dwmac4_rx_check_timestamp have the same kind of issue. Another
patch should be sent for it. no ?
regards
Alex
>
> Thanks,
> Jose Miguel Abreu
>
>> if (likely(le32_to_cpu(p->des1) & RDES1_TIMESTAMP_AVAILABLE)) {
>> int i = 0;
>>
>>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox