Netdev List
 help / color / mirror / Atom feed
* [PATCH net] cxgb4: Fix FW flash errors
From: Ganesh Goudar @ 2017-12-29 18:36 UTC (permalink / raw)
  To: netdev, davem
  Cc: nirranjan, indranil, venkatesh, Arjun Vynipadath, Casey Leedom,
	Ganesh Goudar

From: Arjun Vynipadath <arjun@chelsio.com>

Initialize adapter->params.sf_fw_start to fix firmware flash
issues. Use existing macros defined for FW flash addresses.

Fixes: 96ac18f14a5a ("cxgb4: Add support for new flash parts")
Signed-off-by: Arjun Vynipadath <arjun@chelsio.com>
Signed-off-by: Casey Leedom <leedom@chelsio.com>
Signed-off-by: Ganesh Goudar <ganeshgr@chelsio.com>
---
 drivers/net/ethernet/chelsio/cxgb4/cxgb4.h |  1 -
 drivers/net/ethernet/chelsio/cxgb4/t4_hw.c | 17 ++++++++---------
 2 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
index 6f9fa6e..d8424ed 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
+++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4.h
@@ -344,7 +344,6 @@ struct adapter_params {
 
 	unsigned int sf_size;             /* serial flash size in bytes */
 	unsigned int sf_nsec;             /* # of flash sectors */
-	unsigned int sf_fw_start;         /* start of FW image in flash */
 
 	unsigned int fw_vers;		  /* firmware version */
 	unsigned int bs_vers;		  /* bootstrap version */
diff --git a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
index f63210f..375ef86 100644
--- a/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
+++ b/drivers/net/ethernet/chelsio/cxgb4/t4_hw.c
@@ -2844,8 +2844,6 @@ enum {
 	SF_RD_DATA_FAST = 0xb,        /* read flash */
 	SF_RD_ID        = 0x9f,       /* read ID */
 	SF_ERASE_SECTOR = 0xd8,       /* erase sector */
-
-	FW_MAX_SIZE = 16 * SF_SEC_SIZE,
 };
 
 /**
@@ -3558,8 +3556,9 @@ int t4_load_fw(struct adapter *adap, const u8 *fw_data, unsigned int size)
 	const __be32 *p = (const __be32 *)fw_data;
 	const struct fw_hdr *hdr = (const struct fw_hdr *)fw_data;
 	unsigned int sf_sec_size = adap->params.sf_size / adap->params.sf_nsec;
-	unsigned int fw_img_start = adap->params.sf_fw_start;
-	unsigned int fw_start_sec = fw_img_start / sf_sec_size;
+	unsigned int fw_start_sec = FLASH_FW_START_SEC;
+	unsigned int fw_size = FLASH_FW_MAX_SIZE;
+	unsigned int fw_start = FLASH_FW_START;
 
 	if (!size) {
 		dev_err(adap->pdev_dev, "FW image has no data\n");
@@ -3575,9 +3574,9 @@ int t4_load_fw(struct adapter *adap, const u8 *fw_data, unsigned int size)
 			"FW image size differs from size in FW header\n");
 		return -EINVAL;
 	}
-	if (size > FW_MAX_SIZE) {
+	if (size > fw_size) {
 		dev_err(adap->pdev_dev, "FW image too large, max is %u bytes\n",
-			FW_MAX_SIZE);
+			fw_size);
 		return -EFBIG;
 	}
 	if (!t4_fw_matches_chip(adap, hdr))
@@ -3604,11 +3603,11 @@ int t4_load_fw(struct adapter *adap, const u8 *fw_data, unsigned int size)
 	 */
 	memcpy(first_page, fw_data, SF_PAGE_SIZE);
 	((struct fw_hdr *)first_page)->fw_ver = cpu_to_be32(0xffffffff);
-	ret = t4_write_flash(adap, fw_img_start, SF_PAGE_SIZE, first_page);
+	ret = t4_write_flash(adap, fw_start, SF_PAGE_SIZE, first_page);
 	if (ret)
 		goto out;
 
-	addr = fw_img_start;
+	addr = fw_start;
 	for (size -= SF_PAGE_SIZE; size; size -= SF_PAGE_SIZE) {
 		addr += SF_PAGE_SIZE;
 		fw_data += SF_PAGE_SIZE;
@@ -3618,7 +3617,7 @@ int t4_load_fw(struct adapter *adap, const u8 *fw_data, unsigned int size)
 	}
 
 	ret = t4_write_flash(adap,
-			     fw_img_start + offsetof(struct fw_hdr, fw_ver),
+			     fw_start + offsetof(struct fw_hdr, fw_ver),
 			     sizeof(hdr->fw_ver), (const u8 *)&hdr->fw_ver);
 out:
 	if (ret)
-- 
2.1.0

^ permalink raw reply related

* [net 1/1] tipc: fix problems with multipoint-to-point flow control
From: Jon Maloy @ 2017-12-29 18:48 UTC (permalink / raw)
  To: davem, netdev
  Cc: tipc-discussion, hoang.h.le, mohan.krishna.ghanta.krishnamurthy

In commit 04d7b574b245 ("tipc: add multipoint-to-point flow control") we
introduced a protocol for preventing buffer overflow when many group
members try to simultaneously send messages to the same receiving member.

Stress test of this mechanism has revealed a couple of related bugs:

- When the receiving member receives an advertisement REMIT message from
  one of the senders, it will sometimes prematurely activate a pending
  member and send it the remitted advertisement, although the upper
  limit for active senders has been reached. This leads to accumulation
  of illegal advertisements, and eventually to messages being dropped
  because of receive buffer overflow.

- When the receiving member leaves REMITTED state while a received
  message is being read, we miss to look at the pending queue, to
  activate the oldest pending peer. This leads to some pending senders
  being starved out, and never getting the opportunity to profit from
  the remitted advertisement.

We fix the former in the function tipc_group_proto_rcv() by returning
directly from the function once it becomes clear that the remitting
peer cannot leave REMITTED state at that point.

We fix the latter in the function tipc_group_update_rcv_win() by looking
up and activate the longest pending peer when it becomes clear that the
remitting peer now can leave REMITTED state.

Signed-off-by: Jon Maloy <jon.maloy@ericsson.com>
---
 net/tipc/group.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/net/tipc/group.c b/net/tipc/group.c
index 8e12ab5..5f4ffae 100644
--- a/net/tipc/group.c
+++ b/net/tipc/group.c
@@ -109,7 +109,8 @@ static void tipc_group_proto_xmit(struct tipc_group *grp, struct tipc_member *m,
 static void tipc_group_decr_active(struct tipc_group *grp,
 				   struct tipc_member *m)
 {
-	if (m->state == MBR_ACTIVE || m->state == MBR_RECLAIMING)
+	if (m->state == MBR_ACTIVE || m->state == MBR_RECLAIMING ||
+	    m->state == MBR_REMITTED)
 		grp->active_cnt--;
 }
 
@@ -562,7 +563,7 @@ void tipc_group_update_rcv_win(struct tipc_group *grp, int blks, u32 node,
 	int max_active = grp->max_active;
 	int reclaim_limit = max_active * 3 / 4;
 	int active_cnt = grp->active_cnt;
-	struct tipc_member *m, *rm;
+	struct tipc_member *m, *rm, *pm;
 
 	m = tipc_group_find_member(grp, node, port);
 	if (!m)
@@ -605,6 +606,17 @@ void tipc_group_update_rcv_win(struct tipc_group *grp, int blks, u32 node,
 			pr_warn_ratelimited("Rcv unexpected msg after REMIT\n");
 			tipc_group_proto_xmit(grp, m, GRP_ADV_MSG, xmitq);
 		}
+		grp->active_cnt--;
+		list_del_init(&m->list);
+		if (list_empty(&grp->pending))
+			return;
+
+		/* Set oldest pending member to active and advertise */
+		pm = list_first_entry(&grp->pending, struct tipc_member, list);
+		pm->state = MBR_ACTIVE;
+		list_move_tail(&pm->list, &grp->active);
+		grp->active_cnt++;
+		tipc_group_proto_xmit(grp, pm, GRP_ADV_MSG, xmitq);
 		break;
 	case MBR_RECLAIMING:
 	case MBR_DISCOVERED:
@@ -742,14 +754,14 @@ void tipc_group_proto_rcv(struct tipc_group *grp, bool *usr_wakeup,
 		if (!m || m->state != MBR_RECLAIMING)
 			return;
 
-		list_del_init(&m->list);
-		grp->active_cnt--;
 		remitted = msg_grp_remitted(hdr);
 
 		/* Messages preceding the REMIT still in receive queue */
 		if (m->advertised > remitted) {
 			m->state = MBR_REMITTED;
 			in_flight = m->advertised - remitted;
+			m->advertised = ADV_IDLE + in_flight;
+			return;
 		}
 		/* All messages preceding the REMIT have been read */
 		if (m->advertised <= remitted) {
@@ -761,6 +773,8 @@ void tipc_group_proto_rcv(struct tipc_group *grp, bool *usr_wakeup,
 			tipc_group_proto_xmit(grp, m, GRP_ADV_MSG, xmitq);
 
 		m->advertised = ADV_IDLE + in_flight;
+		grp->active_cnt--;
+		list_del_init(&m->list);
 
 		/* Set oldest pending member to active and advertise */
 		if (list_empty(&grp->pending))
-- 
2.1.4


------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

^ permalink raw reply related

* Re: [PATCH net-next 2/2] l2tp: add peer_offset parameter
From: James Chapman @ 2017-12-29 18:53 UTC (permalink / raw)
  To: Guillaume Nault, Lorenzo Bianconi; +Cc: davem, netdev, liuhangbin
In-Reply-To: <20171228194556.GA1256@alphalink.fr>

Sorry for only just seeing this (vacation).

On 28/12/17 19:45, Guillaume Nault wrote:
> On Thu, Dec 28, 2017 at 07:23:48PM +0100, Lorenzo Bianconi wrote:
>> On Dec 28, Guillaume Nault wrote:
>>> After a quick review of L2TPv3 and pseudowires RFCs, I still don't see
>>> how adding some padding between the L2TPv3 header and the payload could
>>> constitute a valid frame. Of course, the base feature is already there,
>>> but after a quick test, it looks like the padding bits aren't
>>> initialised and leak memory.
>> Do you mean for L2TPv2 or L2TPv3? For L2TPv3 offset/peer_offset are initialized
>> in l2tp_nl_cmd_session_create()
>>
> That's the offsets stored in the l2tp_session_cfg structure. But I was
> talking about the xmit path: l2tp_build_l2tpv3_header() doesn't
> initialise the padding between the header and the payload. So when
> someone activates this option, then every transmitted packet leaks
> memory on the wire.
>
>> Setting session data offset is already supported in L2TP kernel module
>> (and could be already used by userspace applications);
>> for L2TPv2 there is an optional 16-bit value in the header while for L2TPv3
>> the offset is configured by userspace.
>> At the moment the kernel (for L2TPv3) uses offset for both tx and rx side.
>> Userspace (iproute2) allows to distinguish tx offset (offset) from rx one
>> (peer_offset) but since the rx part is not handled at the moment
>> (I fixed peer_offset support in iproute2, I have not sent the patch upstream yet, attached below)
>> this leads to a misalignment between tunnel endpoints.
>> You can easily reproduce the issue using this setup (and the below patch for iproute2):
>>
>> ip l2tp add tunnel local <ip0> remote <ip1> tunnel_id <id0> peer_tunnel_id <id1> udp_sport <p0> udp_dport <p1>
>> ip l2tp add tunnel local <ip1> remote <ip0> tunnel_id <id1> peer_tunnel_id <id0> udp_sport <p1> udp_dport <p0>
>>
>> ip l2tp add session name l2tp0 tunnel_id <id0> session_id <s0> peer_session_id <s1> offset 8 peer_offset 16
>> ip l2tp add session name l2tp0 tunnel_id <id1> session_id <s1> peer_session_id <s0> offset 16 peer_offset 8
>>
> Yes, I'm well aware of that. And thanks for having worked on a full
> solution including iproute2. But does one really need to set
> asymetrical offset values? It doesn't look wrong to require setting the
> same value on both sides. Other options need this, like "l2spec_type".
>
> Here we have an option that:
>    * creates invalid packets (AFAIK),
>    * is buggy and leaks memory on the network,
>    * doesn't seem to have any use case (even the manpage
>      says "This is hardly ever used").
>
> So I'm sorry, but I don't see the point in expanding this option to
> allow even stranger setups. If there's a use case, then fine.
> Otherwise, let's just acknowledge that the "peer_offset" option of
> iproute2 is a noop (and maybe remove it from the manpage).
>
> And the kernel "offset" option needs to be fixed. Actually, I wouldn't
> mind if it was converted to be a noop, or even rejected. L2TP already
> has its share of unused features that complicate the code and hamper
> evolution and bug fixing. As I said earlier, if it's buggy, unused and
> can't even produce valid packets, then why bothering with it?
>
> But that's just my point of view. James, do you have an opinion on
> this?

I agree, Guillaume.

The L2TPv3 protocol RFC dropped the configurable offset of L2TPv2 - 
instead, the Layer-2-Specific-Sublayer is supposed to handle any 
transport-specific data alignment requirements. I think a configurable 
offset has found its way into iproute2 l2tp commands by mistake, perhaps 
because the netlink API defines an attribute for it, but which was only 
intended for use with L2TPv2. For L2TPv2, we only configure the offset 
for transmitted packets. In received packets, the offset (if present) is 
obtained from the L2TPv2 header in each received packet. There is no 
need to add a peer-offset netlink attribute to set the offset expected 
in received packets.

Lorenzo, is this being added to fix interoperability with another L2TPv3 
implementation? If so, can you share more details?

^ permalink raw reply

* [PATCH net-next] net: dsa: Fix dsa_legacy_register() return value
From: Florian Fainelli @ 2017-12-29 19:05 UTC (permalink / raw)
  To: netdev
  Cc: davem, Egil Hjelmeland, Florian Fainelli, Andrew Lunn,
	Vivien Didelot, open list

We need to make the dsa_legacy_register() stub return 0 in order for
dsa_init_module() to successfully register and continue registering the
ETH_P_XDSA packet handler.

Fixes: 2a93c1a3651f ("net: dsa: Allow compiling out legacy support")
Reported-by: Egil Hjelmeland <privat@egil-hjelmeland.no>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 net/dsa/dsa_priv.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/dsa/dsa_priv.h b/net/dsa/dsa_priv.h
index b03665e8fb4e..cefb0c3c6d51 100644
--- a/net/dsa/dsa_priv.h
+++ b/net/dsa/dsa_priv.h
@@ -103,7 +103,7 @@ void dsa_legacy_unregister(void);
 #else
 static inline int dsa_legacy_register(void)
 {
-	return -ENODEV;
+	return 0;
 }
 
 static inline void dsa_legacy_unregister(void) { }
-- 
2.14.1

^ permalink raw reply related

* [PATCH] wlcore: Delete an error message for a failed memory allocation in three functions
From: SF Markus Elfring @ 2017-12-29 19:50 UTC (permalink / raw)
  To: linux-wireless, netdev, Arend Van Spriel, Arnd Bergmann,
	Eyal Reizer, Franky Lin, Gustavo A. R. Silva, Iain Hunter,
	Johannes Berg, Kalle Valo, Sebastian Reichel, Tony Lindgren
  Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 29 Dec 2017 20:40:49 +0100

Omit an extra message for a memory allocation failure in these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/wireless/ti/wlcore/main.c | 8 ++------
 drivers/net/wireless/ti/wlcore/spi.c  | 4 +---
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ti/wlcore/main.c b/drivers/net/wireless/ti/wlcore/main.c
index c346c021b999..970e59871547 100644
--- a/drivers/net/wireless/ti/wlcore/main.c
+++ b/drivers/net/wireless/ti/wlcore/main.c
@@ -1301,10 +1301,8 @@ static struct sk_buff *wl12xx_alloc_dummy_packet(struct wl1271 *wl)
 			    sizeof(struct wl1271_tx_hw_descr) - sizeof(*hdr);
 
 	skb = dev_alloc_skb(TOTAL_TX_DUMMY_PACKET_SIZE);
-	if (!skb) {
-		wl1271_warning("Failed to allocate a dummy packet skb");
+	if (!skb)
 		return NULL;
-	}
 
 	skb_reserve(skb, sizeof(struct wl1271_tx_hw_descr));
 
@@ -1421,10 +1419,8 @@ int wl1271_rx_filter_alloc_field(struct wl12xx_rx_filter *filter,
 	field = &filter->fields[filter->num_fields];
 
 	field->pattern = kzalloc(len, GFP_KERNEL);
-	if (!field->pattern) {
-		wl1271_warning("Failed to allocate RX filter pattern");
+	if (!field->pattern)
 		return -ENOMEM;
-	}
 
 	filter->num_fields++;
 
diff --git a/drivers/net/wireless/ti/wlcore/spi.c b/drivers/net/wireless/ti/wlcore/spi.c
index 62ce54a949e9..fc94a0d1a01d 100644
--- a/drivers/net/wireless/ti/wlcore/spi.c
+++ b/drivers/net/wireless/ti/wlcore/spi.c
@@ -489,10 +489,8 @@ static int wl1271_probe(struct spi_device *spi)
 	pdev_data->if_ops = &spi_ops;
 
 	glue = devm_kzalloc(&spi->dev, sizeof(*glue), GFP_KERNEL);
-	if (!glue) {
-		dev_err(&spi->dev, "can't allocate glue\n");
+	if (!glue)
 		return -ENOMEM;
-	}
 
 	glue->dev = &spi->dev;
 
-- 
2.15.1

^ permalink raw reply related

* Re: b53 tags on bpi-r1 (bcm53125)
From: Florian Fainelli @ 2017-12-29 20:21 UTC (permalink / raw)
  To: Jochen Friedrich; +Cc: netdev
In-Reply-To: <514a187b-cd3d-a15c-cd28-60b0b2d49dcd@scram.de>

Hi Jochen,

Le 12/18/17 à 02:44, Jochen Friedrich a écrit :
> Hi Florian,
> 
> unfortunately, this doesn't make any difference.
> 
> Just out of curiosity, BPI-R1 has pull-down resistors on LED6 and 7
> (MII_MODE0/1). However, the public available 53125U sheet doesn't
> document these pins:
> 
> LED[6] IMP_MODE[0] Pull-up Active low (since IMP Mode is not used)
> LED[7] IMP_MODE[1] Pull-up Active low (since IMP Mode is not used)
> 
> Is this MII mode maybe incompatible with Broadcom tags?

AFAICT, it should not, this is largely independent from enabling
Broadcom tags.

I have now reproduced this on my BPI-R1 as well and am looking at what
might be going wrong.

Thanks!
-- 
Florian

^ permalink raw reply

* [PATCH] cw1200: Delete an error message for a failed memory allocation in three functions
From: SF Markus Elfring @ 2017-12-29 20:52 UTC (permalink / raw)
  To: linux-wireless, netdev, Kalle Valo, Solomon Peachy; +Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 29 Dec 2017 21:48:05 +0100

Omit an extra message for a memory allocation failure in these functions.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/wireless/st/cw1200/cw1200_sdio.c | 4 +---
 drivers/net/wireless/st/cw1200/cw1200_spi.c  | 4 +---
 drivers/net/wireless/st/cw1200/fwio.c        | 1 -
 3 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/st/cw1200/cw1200_sdio.c b/drivers/net/wireless/st/cw1200/cw1200_sdio.c
index 1037ec62659d..82c6827579a9 100644
--- a/drivers/net/wireless/st/cw1200/cw1200_sdio.c
+++ b/drivers/net/wireless/st/cw1200/cw1200_sdio.c
@@ -289,10 +289,8 @@ static int cw1200_sdio_probe(struct sdio_func *func,
 		return -ENODEV;
 
 	self = kzalloc(sizeof(*self), GFP_KERNEL);
-	if (!self) {
-		pr_err("Can't allocate SDIO hwbus_priv.\n");
+	if (!self)
 		return -ENOMEM;
-	}
 
 	func->card->quirks |= MMC_QUIRK_LENIENT_FN0;
 
diff --git a/drivers/net/wireless/st/cw1200/cw1200_spi.c b/drivers/net/wireless/st/cw1200/cw1200_spi.c
index 412fb6e49aed..7b6d5e5c5a62 100644
--- a/drivers/net/wireless/st/cw1200/cw1200_spi.c
+++ b/drivers/net/wireless/st/cw1200/cw1200_spi.c
@@ -399,10 +399,8 @@ static int cw1200_spi_probe(struct spi_device *func)
 	}
 
 	self = devm_kzalloc(&func->dev, sizeof(*self), GFP_KERNEL);
-	if (!self) {
-		pr_err("Can't allocate SPI hwbus_priv.");
+	if (!self)
 		return -ENOMEM;
-	}
 
 	self->pdata = plat_data;
 	self->func = func;
diff --git a/drivers/net/wireless/st/cw1200/fwio.c b/drivers/net/wireless/st/cw1200/fwio.c
index 30e7646d04af..79dd7a8ffb05 100644
--- a/drivers/net/wireless/st/cw1200/fwio.c
+++ b/drivers/net/wireless/st/cw1200/fwio.c
@@ -153,7 +153,6 @@ static int cw1200_load_firmware_cw1200(struct cw1200_common *priv)
 
 	buf = kmalloc(DOWNLOAD_BLOCK_SIZE, GFP_KERNEL | GFP_DMA);
 	if (!buf) {
-		pr_err("Can't allocate firmware load buffer.\n");
 		ret = -ENOMEM;
 		goto firmware_release;
 	}
-- 
2.15.1

^ permalink raw reply related

* [PATCH] rt2x00: Delete an error message for a failed memory allocation in rt2x00queue_allocate()
From: SF Markus Elfring @ 2017-12-29 21:18 UTC (permalink / raw)
  To: linux-wireless, netdev, Helmut Schaa, Kalle Valo,
	Stanislaw Gruszka
  Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 29 Dec 2017 22:11:42 +0100

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/wireless/ralink/rt2x00/rt2x00queue.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
index a2c1ca5c76d1..6598cefdbe71 100644
--- a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c
@@ -1244,10 +1244,8 @@ int rt2x00queue_allocate(struct rt2x00_dev *rt2x00dev)
 	rt2x00dev->data_queues = 2 + rt2x00dev->ops->tx_queues + req_atim;
 
 	queue = kcalloc(rt2x00dev->data_queues, sizeof(*queue), GFP_KERNEL);
-	if (!queue) {
-		rt2x00_err(rt2x00dev, "Queue allocation failed\n");
+	if (!queue)
 		return -ENOMEM;
-	}
 
 	/*
 	 * Initialize pointers
-- 
2.15.1

^ permalink raw reply related

* [PATCH] p54spi: Delete an error message for a failed memory allocation in p54spi_rx()
From: SF Markus Elfring @ 2017-12-29 21:40 UTC (permalink / raw)
  To: linux-wireless, netdev, Christian Lamparter, Kalle Valo
  Cc: LKML, kernel-janitors

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Fri, 29 Dec 2017 22:33:10 +0100

Omit an extra message for a memory allocation failure in this function.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/net/wireless/intersil/p54/p54spi.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/net/wireless/intersil/p54/p54spi.c b/drivers/net/wireless/intersil/p54/p54spi.c
index e41bf042352e..900cfaacf25e 100644
--- a/drivers/net/wireless/intersil/p54/p54spi.c
+++ b/drivers/net/wireless/intersil/p54/p54spi.c
@@ -367,7 +367,6 @@ static int p54spi_rx(struct p54s_priv *priv)
 	skb = dev_alloc_skb(len + 4);
 	if (!skb) {
 		p54spi_sleep(priv);
-		dev_err(&priv->spi->dev, "could not alloc skb");
 		return -ENOMEM;
 	}
 
-- 
2.15.1

^ permalink raw reply related

* Re: [PATCH net-next 5/6] arm64: dts: marvell: mcbin: enable the fourth network interface
From: Antoine Tenart @ 2017-12-29 21:41 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Antoine Tenart, davem, kishon, andrew, jason,
	sebastian.hesselbarth, gregory.clement, mw, stefanc, ymarkman,
	thomas.petazzoni, miquel.raynal, nadavh, netdev, linux-arm-kernel,
	linux-kernel, Jon Nettleton
In-Reply-To: <20171228185920.GW10595@n2100.armlinux.org.uk>

Hi Russell,

On Thu, Dec 28, 2017 at 06:59:21PM +0000, Russell King - ARM Linux wrote:
> On Thu, Dec 28, 2017 at 11:04:16AM +0100, Antoine Tenart wrote:
> > 
> > That's not what I remembered. You had some valid points, and others
> > related to PHY modes the driver wasn't supporting before the phylink
> > transition. My understanding of this was that you wanted a full
> > featured support while I only wanted to convert the already supported
> > modes.
> 
> You are mistaken - you can get a full refresher on where things were
> at via https://patchwork.kernel.org/patch/9963971/

I read it again and still have the same feeling. There's been a
misunderstanding at some point. Anyway, let's move forward :)

> 1. I asked for details about what mvpp2.c supports that phylink does
>    not (as you indicated that there were certain things that mvpp2
>    supports that phylink does not.)  I'm still awaiting a response.

I don't remember PHY modes supported in the PPv2 driver that aren't
supported in PHYLINK. I think this point is the main misunderstanding. I
thought you wanted me to support modes unsupported in the PPv2 driver
before. But you explained quite well what these comments were about
below.

So I guess this point is resolved (aka I'll have to take your comments
into account for the v2).

> 2. 25th Sept, you indicated that you would get someone to test
>    an issue related to in-band AN. No results of that testing have
>    been forthcoming.

That's right. I asked someone to make a test, but did not get an answer.
And because the PHYLINK patch stalled on my side I kinda forget about
it. I'll try again to have this test made.

> I am not after a full featured support, what I'm after is ensuring
> that phylink is (a) used correctly and (b) implementations using it
> are correct.  Part of that is ensuring that users don't introduce
> unexpected failure conditions.
> 
> So, when you do this in the validate() callback:
> 
>  +       phylink_set(mask, 1000baseX_Full);
> 
> and then do this in the mac_config() callback:
> 
>  +       if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
>  +           port->phy_interface != PHY_INTERFACE_MODE_SGMII)
>  +               return;
> 
> and this in the link_state() callback:
> 
>  +       if (!phy_interface_mode_is_rgmii(port->phy_interface) &&
>  +           port->phy_interface != PHY_INTERFACE_MODE_SGMII)
>  +               return 0;
> 
> the result is that phylink thinks that you support 1000base-X modes,
> and it will call mac_config() asking for 1000base-X, but you silently
> ignore that, leaving the hardware configured in whatever state it was.
> That leads to a silent failure as far as the user is concerned.
> 
> So, if you do not intend to support 1000base-X initially, don't
> allow it in the validate callback until you do.
> 
> It gets worse, because the return in link_state() means that phylink
> thinks that the link is up if it has requested 1000base-X, which it
> won't be unless you've properly configured it.
> 
> It's this kind of unreliability that I was concerned about in your
> patch.  I'm not demanding "full featured implementation" but I do
> want you to use it correctly.

Thanks for the detailed explanations!

> > > What I'm most concerned about, given the bindings for comphy that
> > > have been merged, is that Free Electrons is pushing forward seemingly
> > > with no regard to the requirement that the serdes lanes are dynamically
> > > reconfigurable, and that's a basic requirement for SFP, and for the
> > > 88x3310 PHYs on the Macchiatobin platform.
> > 
> > The main idea behind the comphy driver is to provide a way to
> > reconfigure the serdes lanes at runtime. Could you develop what are
> > blocking points to properly support SFP, regarding the current comphy
> > support?
> 
> If it supports serdes lane mode reconfiguration (iow, switching between
> 1000base-X, 2500base-X, SGMII, 10G-KR), then that's all that's required.

It does, and the PPv2 driver already ask the COMPHY driver to perform
these reconfigurations (when using the 10G/1G interface on the mcbin for
example).

Thanks!
Antoine

-- 
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

^ permalink raw reply

* Re: b53 tags on bpi-r1 (bcm53125)
From: Florian Fainelli @ 2017-12-29 21:56 UTC (permalink / raw)
  To: Jochen Friedrich; +Cc: netdev
In-Reply-To: <41ee1d36-a939-b1f0-4274-f799a96c8a2c@gmail.com>

Le 12/29/17 à 12:21, Florian Fainelli a écrit :
> Hi Jochen,
> 
> Le 12/18/17 à 02:44, Jochen Friedrich a écrit :
>> Hi Florian,
>>
>> unfortunately, this doesn't make any difference.
>>
>> Just out of curiosity, BPI-R1 has pull-down resistors on LED6 and 7
>> (MII_MODE0/1). However, the public available 53125U sheet doesn't
>> document these pins:
>>
>> LED[6] IMP_MODE[0] Pull-up Active low (since IMP Mode is not used)
>> LED[7] IMP_MODE[1] Pull-up Active low (since IMP Mode is not used)
>>
>> Is this MII mode maybe incompatible with Broadcom tags?
> 
> AFAICT, it should not, this is largely independent from enabling
> Broadcom tags.
> 
> I have now reproduced this on my BPI-R1 as well and am looking at what
> might be going wrong.

OK, so I have been able to find out what was going on. On BCM53125 and
earlier switches, we need to do a couple of things for Broadcom tags to
be usable:

- turn on managed mode (SM_SW_FWD_MODE)
- configure Port 8 for IMP mode (B53_GLOBAL_CONFIG, setting bit
GC_FRM_MGMT_PORT_MII)

After doing that, I can now see the correct outgoing packets on my host,
however, the replies don't seem to be delivered to the per-port DSA
network devices, and I suspect it's because of stmmac, so I am
investigating this now.
-- 
Florian

^ permalink raw reply

* [PATCH iproute2 1/4] man: drop references to Debian-specific paths
From: Luca Boccassi @ 2017-12-29 22:01 UTC (permalink / raw)
  To: netdev

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 1699 bytes --]

Documentation should be distribution-agnostic - any specific quirks
should be handled by downstream maintainers, if necessary.
Remove mentions of Debian paths and package names.

Signed-off-by: Luca Boccassi <bluca@debian.org>
---
 man/man8/lnstat.8 | 3 +--
 man/man8/ss.8     | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/man/man8/lnstat.8 b/man/man8/lnstat.8
index acd5f4a2..b98241bf 100644
--- a/man/man8/lnstat.8
+++ b/man/man8/lnstat.8
@@ -254,8 +254,7 @@ Number of hash table list traversals for output traffic. Deprecated since IP
 route cache removal, therefore always zero.
 
 .SH SEE ALSO
-.BR ip (8),
-and /usr/share/doc/iproute-doc/README.lnstat (package iproute-doc on Debian)
+.BR ip (8)
 .br
 .SH AUTHOR
 lnstat was written by Harald Welte <laforge@gnumonks.org>.
diff --git a/man/man8/ss.8 b/man/man8/ss.8
index 0d526734..973afbe0 100644
--- a/man/man8/ss.8
+++ b/man/man8/ss.8
@@ -327,7 +327,7 @@ Read filter information from FILE.
 Each line of FILE is interpreted like single command line option. If FILE is - stdin is used.
 .TP
 .B FILTER := [ state STATE-FILTER ] [ EXPRESSION ]
-Please take a look at the official documentation (Debian package iproute-doc) for details regarding filters.
+Please take a look at the official documentation for details regarding filters.
 
 .SH STATE-FILTER
 
@@ -382,7 +382,6 @@ Find all local processes connected to X server.
 List all the tcp sockets in state FIN-WAIT-1 for our apache to network 193.233.7/24 and look at their timers.
 .SH SEE ALSO
 .BR ip (8),
-.BR /usr/share/doc/iproute-doc/ss.html " (package iproute­doc)",
 .br
 .BR RFC " 793 "
 - https://tools.ietf.org/rfc/rfc793.txt (TCP states)
-- 
2.14.2

^ permalink raw reply related

* [PATCH iproute2 2/4] man: add more keywords to ip.8 short description
From: Luca Boccassi @ 2017-12-29 22:01 UTC (permalink / raw)
  To: netdev
In-Reply-To: <20171229220125.13579-1-bluca@debian.org>

A Debian user suggested adding more network-related keywords to the
ip manpage, so that manpage-scraping and indexing software like
apropos can do a better job of categorizing the programs.

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=877983

Suggested-by: Lynoure Braakman <lynoure@gmail.com>
Signed-off-by: Luca Boccassi <bluca@debian.org>
---
 man/man8/ip.8 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man/man8/ip.8 b/man/man8/ip.8
index ae018fdf..94e64319 100644
--- a/man/man8/ip.8
+++ b/man/man8/ip.8
@@ -1,6 +1,6 @@
 .TH IP 8 "20 Dec 2011" "iproute2" "Linux"
 .SH NAME
-ip \- show / manipulate routing, devices, policy routing and tunnels
+ip \- show / manipulate routing, network devices, policy routing, interfaces (ethernet and more) and tunnels
 .SH SYNOPSIS
 
 .ad l
-- 
2.14.2

^ permalink raw reply related

* [PATCH iproute2 3/4] man: ip-address: document 15-char limit for LABEL
From: Luca Boccassi @ 2017-12-29 22:01 UTC (permalink / raw)
  To: netdev
In-Reply-To: <20171229220125.13579-1-bluca@debian.org>

Trying to set a label longer than 15 characters returns an error:
 RTNETLINK answers: Numerical result out of range

Document the limit in the manpage.

Originally reported as a Debian bug:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=661886

Reported-by: Gabor Kiss <kissg@ssg.ki.iif.hu>
Signed-off-by: Luca Boccassi <bluca@debian.org>
---
 man/man8/ip-address.8.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/man/man8/ip-address.8.in b/man/man8/ip-address.8.in
index eaa179c6..e7f14533 100644
--- a/man/man8/ip-address.8.in
+++ b/man/man8/ip-address.8.in
@@ -190,6 +190,7 @@ Each address may be tagged with a label string.
 In order to preserve compatibility with Linux-2.0 net aliases,
 this string must coincide with the name of the device or must be prefixed
 with the device name followed by colon.
+The maximum allowed length is 15 characters.
 
 .TP
 .BI scope " SCOPE_VALUE"
-- 
2.14.2

^ permalink raw reply related

* [PATCH iproute2 4/4] man: routel/routef: don't mention filesystem paths
From: Luca Boccassi @ 2017-12-29 22:01 UTC (permalink / raw)
  To: netdev
In-Reply-To: <20171229220125.13579-1-bluca@debian.org>

The filesytem paths to these scripts might be different on various
distros, so don't mention it in the manpages. It is not really useful
information anyway.

Originally submitted as Debian bug:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=561424

Reported-by: jidanni@jidanni.org
Signed-off-by: Luca Boccassi <bluca@debian.org>
---
 man/man8/routel.8 | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/man/man8/routel.8 b/man/man8/routel.8
index 82d580fb..2270eacb 100644
--- a/man/man8/routel.8
+++ b/man/man8/routel.8
@@ -17,11 +17,6 @@ The routel script will list routes in a format that some might consider easier t
 .br
 The routef script does not take any arguments and will simply flush the routing table down the drain. Beware! This means deleting all routes which will make your network unusable!
 
-.SH "FILES"
-.LP
-\fI/usr/bin/routef\fP
-.br
-\fI/usr/bin/routel\fP
 .SH "AUTHORS"
 .LP
 The routel script was written by Stephen R. van den Berg <srb@cuci.nl>, 1999/04/18 and donated to the public domain.
-- 
2.14.2

^ permalink raw reply related

* Re: [PATCH net-next 2/2] l2tp: add peer_offset parameter
From: Lorenzo Bianconi @ 2017-12-29 22:21 UTC (permalink / raw)
  To: James Chapman; +Cc: Guillaume Nault, David S. Miller, netdev, Hangbin Liu
In-Reply-To: <27e0a7c8-b95b-04eb-d808-bcae67e417b1@katalix.com>

> Sorry for only just seeing this (vacation).
>
>
> On 28/12/17 19:45, Guillaume Nault wrote:
>>
>> On Thu, Dec 28, 2017 at 07:23:48PM +0100, Lorenzo Bianconi wrote:
>>>
>>> On Dec 28, Guillaume Nault wrote:
>>>>
>>>> After a quick review of L2TPv3 and pseudowires RFCs, I still don't see
>>>> how adding some padding between the L2TPv3 header and the payload could
>>>> constitute a valid frame. Of course, the base feature is already there,
>>>> but after a quick test, it looks like the padding bits aren't
>>>> initialised and leak memory.
>>>
>>> Do you mean for L2TPv2 or L2TPv3? For L2TPv3 offset/peer_offset are
>>> initialized
>>> in l2tp_nl_cmd_session_create()
>>>
>> That's the offsets stored in the l2tp_session_cfg structure. But I was
>> talking about the xmit path: l2tp_build_l2tpv3_header() doesn't
>> initialise the padding between the header and the payload. So when
>> someone activates this option, then every transmitted packet leaks
>> memory on the wire.
>>
>>> Setting session data offset is already supported in L2TP kernel module
>>> (and could be already used by userspace applications);
>>> for L2TPv2 there is an optional 16-bit value in the header while for
>>> L2TPv3
>>> the offset is configured by userspace.
>>> At the moment the kernel (for L2TPv3) uses offset for both tx and rx
>>> side.
>>> Userspace (iproute2) allows to distinguish tx offset (offset) from rx one
>>> (peer_offset) but since the rx part is not handled at the moment
>>> (I fixed peer_offset support in iproute2, I have not sent the patch
>>> upstream yet, attached below)
>>> this leads to a misalignment between tunnel endpoints.
>>> You can easily reproduce the issue using this setup (and the below patch
>>> for iproute2):
>>>
>>> ip l2tp add tunnel local <ip0> remote <ip1> tunnel_id <id0>
>>> peer_tunnel_id <id1> udp_sport <p0> udp_dport <p1>
>>> ip l2tp add tunnel local <ip1> remote <ip0> tunnel_id <id1>
>>> peer_tunnel_id <id0> udp_sport <p1> udp_dport <p0>
>>>
>>> ip l2tp add session name l2tp0 tunnel_id <id0> session_id <s0>
>>> peer_session_id <s1> offset 8 peer_offset 16
>>> ip l2tp add session name l2tp0 tunnel_id <id1> session_id <s1>
>>> peer_session_id <s0> offset 16 peer_offset 8
>>>
>> Yes, I'm well aware of that. And thanks for having worked on a full
>> solution including iproute2. But does one really need to set
>> asymetrical offset values? It doesn't look wrong to require setting the
>> same value on both sides. Other options need this, like "l2spec_type".
>>
>> Here we have an option that:
>>    * creates invalid packets (AFAIK),
>>    * is buggy and leaks memory on the network,
>>    * doesn't seem to have any use case (even the manpage
>>      says "This is hardly ever used").
>>
>> So I'm sorry, but I don't see the point in expanding this option to
>> allow even stranger setups. If there's a use case, then fine.
>> Otherwise, let's just acknowledge that the "peer_offset" option of
>> iproute2 is a noop (and maybe remove it from the manpage).
>>
>> And the kernel "offset" option needs to be fixed. Actually, I wouldn't
>> mind if it was converted to be a noop, or even rejected. L2TP already
>> has its share of unused features that complicate the code and hamper
>> evolution and bug fixing. As I said earlier, if it's buggy, unused and
>> can't even produce valid packets, then why bothering with it?
>>
>> But that's just my point of view. James, do you have an opinion on
>> this?
>
>
> I agree, Guillaume.
>
> The L2TPv3 protocol RFC dropped the configurable offset of L2TPv2 - instead,
> the Layer-2-Specific-Sublayer is supposed to handle any transport-specific
> data alignment requirements. I think a configurable offset has found its way
> into iproute2 l2tp commands by mistake, perhaps because the netlink API
> defines an attribute for it, but which was only intended for use with
> L2TPv2. For L2TPv2, we only configure the offset for transmitted packets. In
> received packets, the offset (if present) is obtained from the L2TPv2 header
> in each received packet. There is no need to add a peer-offset netlink
> attribute to set the offset expected in received packets.
>
> Lorenzo, is this being added to fix interoperability with another L2TPv3
> implementation? If so, can you share more details?
>

Hi James,

I introduced peer_offset parameter to fix a specific setup where
tunnel endpoints
running L2TPv3 would use different values for tx offset (since in
iproute2 there is no
restriction on it), not to fix a given an interoperability issue.
I introduced this feature since:
 - offset has been added for long time to L2TPv3 implementation
   (commit f7faffa3ff8ef6ae712ef16312b8a2aa7a1c95fe and
   commit 309795f4bec2d69cd507a631f82065c2198a0825) and I wanted to
preserve UABI
 - have the same degree of freedom for offset parameter we have in
L2TPv2 and fix the issue
   described above

Now what we can do I guess is:
- as suggested by Guillaume drop completely the offset support without removing
  netlink attribute in order to not break UABI
- fix offset support initializing properly padding bits

I think at the moment we can skip the option to impose to have the
same offset value
for both tx and rx side.

Regards,
Lorenzo

^ permalink raw reply

* Re: [RFT net-next v3 0/5] dwmac-meson8b: RGMII clock fixes for Meson8b
From: Emiliano Ingrassia @ 2017-12-29 23:00 UTC (permalink / raw)
  To: Jerome Brunet, Martin Blumenstingl
  Cc: netdev, linus.luessing, khilman, linux-amlogic, Neil Armstrong,
	peppe.cavallaro, alexandre.torgue
In-Reply-To: <1514570663.7439.19.camel@baylibre.com>

Hi Jerome, Hi Martin,

On Fri, Dec 29, 2017 at 07:04:23PM +0100, Jerome Brunet wrote:
> On Fri, 2017-12-29 at 02:31 +0100, Emiliano Ingrassia wrote:
> > Hi Martin, Hi Dave,
> > 
> > On Thu, Dec 28, 2017 at 11:21:23PM +0100, Martin Blumenstingl wrote:
> > > Hi Dave,
> > > 
> > > please do not apply this series until it got a Tested-by from Emiliano.
> > > 
> > > 
> > > Hi Emiliano,
> > > 
> > > you reported [0] that you couldn't get dwmac-meson8b to work on your
> > > Odroid-C1. With your findings (register dumps, clk_summary output, etc.)
> > > I think I was able to find a fix: it consists of two patches (which you
> > > find in this series)
> > > 
> > > Unfortunately I don't have any Meson8b boards with RGMII PHY so I could
> > > only partially test this (I could only check if the clocks were
> > > calculated correctly when using a dummy 500002394Hz input clock instead
> > > of MPLL2).
> > > 
> > > Could you please give this series a try and let me know about the
> > > results?
> > > You obviously still need your two "ARM: dts: meson8b" patches which
> > > - add the amlogic,meson8b-dwmac" compatible to meson8b.dtsi
> > > - enable Ethernet on the Odroid-C1
> > > 
> > > When testing on Meson8b this also needs a fix for the MPLL clock driver:
> > > "clk: meson: mpll: use 64-bit maths in params_from_rate", see:
> > > https://patchwork.kernel.org/patch/10131677/
> > > 
> > > 
> > > I have tested this myself on a Khadas VIM (GXL SoC, internal RMII PHY)
> > > and a Khadas VIM2 (GXM SoC, external RGMII PHY). Both are still working
> > > fine (so let's hope that this also fixes your Meson8b issue :)).
> > > 
> > > 
> > > changes since v1 at [1]:
> > > - changed the subject of the cover-letter to indicate that this is all
> > >   about the RGMII clock
> > > - added PATCH #1 which ensures that we don't unnecessarily change the
> > >   parent clocks in RMII mode (and also makes the code easier to
> > >   understand)
> > > - changed subject of PATCH #2 (formerly PATCH #1) to state that this
> > >   is about the RGMII clock
> > > - added Jerome's Reviewed-by to PATCH #2 (formerly PATCH #1)
> > > - replaced PATCH #3 (formerly PATCH #2) with one that sets
> > >   CLK_SET_RATE_PARENT on the mux and thus re-configures the MPLL2 clock
> > >   on Meson8b correctly
> > > 
> > > changes since v2 at [2]:
> > > - added PATCH #2 to make the following patch easier
> > > - Emiliano reported that there's currently another bug in the
> > >   dwmac-meson8b driver which prevents it from working with RGMII PHYs on
> > >   Meson8b: bit 10 of the PRG_ETH0 register is configures a clock gate
> > >   (instead of a divide by 5 or divide by 10 clock divider). This has not
> > >   been visible on GXBB and later due to the input clock which always led
> > >   to a selection of "divide by 10" (which is done internally in the IP
> > >   block, but the bit actually means "enable RGMII clock output").
> > >   PATCH #3 was added to address this issue.
> > > - the commit message of PATCH #4 and #5 (formerly PATCH #2 and #3) were
> > >   updated and the patch itself rebased because the m25_div clock was
> > >   removed with the new PATCH #3 (so some of the statements were not
> > >   valid anymore)
> > > 
> > 
> > Here is the clk_summary relative to ethernet on Odroid-C1+
> > with this new series applied:
> > 
> > xtal				    1            1    24000000          0 0
> >  sys_pll			    0            0  1200000000          0 0
> >   cpu_clk			    0            0  1200000000          0 0
> >  vid_pll			    0            0   732000000          0 0
> >  fixed_pll			    2            2  2550000000          0 0
> >   mpll2				    1            1   249999701          0 0
> >    c9410000.ethernet#m250_sel       1            1   249999701          0 0
> >     c9410000.ethernet#m250_div	    1            1   249999701          0 0
> >      c9410000.ethernet#fixed_div10  1            1    24999970          0 0
> >       c9410000.ethernet#m25_en	    1            1    24999970          0 0
> > 
> > The ethernet prg0 register is set to 0x74A1 which should be correct with
> > respect to the information contained in the S805 SoC manual.
> > Actually, the ethernet is not yet fully functional.
> > Trying to ping the board, I can see ARP request from host to board using
> > tcpdump. However, the host can't see any response.
> 
> If the rx path is ok-ish, I suppose the clock setting applied is good.
> Maybe you could try to play with the tx delay (BIT 5-6 of the register) ?
>

Thanks for the suggestion. Finally the ethernet works correctly using 4
ns as tx-delay.
The clock summary is the same reported above. The prg0 ethernet register
value is 0x74c1 as expected.

I would like to thanks Martin for the support!
As soon as this patch series [v3] will be submitted, I'll submit my patch for
the device tree.
Let me know if you have any question.

Thanks again,

Emiliano

> > 
> > Following the U-Boot value for prg0 register, which is 0x7d21, I also
> > tried to set bit 11. As expected, this did not have any influence.
> > Another thing that we should check is the "Ethernet Memory PD" (see S805
> > manual - sec. 5.4) register which bits 3-2 enable/disable ethernet
> > normal operation. However, those bits are already cleared by U-Boot.
> > 
> > Thank you for the support.
> > 
> > Best regards,
> > 
> > Emiliano
> > 
> > > 
> > > [0] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005596.html
> > > [1] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005848.html
> > > [2] http://lists.infradead.org/pipermail/linux-amlogic/2017-December/005861.html
> > > 
> > > 
> > > Martin Blumenstingl (5):
> > >   net: stmmac: dwmac-meson8b: only configure the clocks in RGMII mode
> > >   net: stmmac: dwmac-meson8b: simplify generating the clock names
> > >   net: stmmac: dwmac-meson8b: fix internal RGMII clock configuration
> > >   net: stmmac: dwmac-meson8b: fix setting the RGMII clock on Meson8b
> > >   net: stmmac: dwmac-meson8b: propagate rate changes to the parent clock
> > > 
> > >  .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 119 +++++++++++----------
> > >  1 file changed, 63 insertions(+), 56 deletions(-)
> > > 
> > > -- 
> > > 2.15.1
> > > 
> 

^ permalink raw reply

* [net-next v2] ipv6: sr: export some functions of seg6local
From: Ahmed Abdelsalam @ 2017-12-29 23:08 UTC (permalink / raw)
  To: davem; +Cc: david.lebrun, netdev, linux-kernel, amsalam20

Some functions of seg6local are very useful to process SRv6
encapsulated packets

This patch exports some functions of seg6local that are useful and
can be re-used at different parts of the kernel.

The set of exported functions are:
(1) seg6_get_srh()
(2) seg6_advance_nextseg()
(3) seg6_lookup_nexthop

Signed-off-by: Ahmed Abdelsalam <amsalam20@gmail.com>
---
 Functions names are prefixed with seg6_
 include/net/seg6.h    |  5 +++++
 net/ipv6/seg6_local.c | 37 ++++++++++++++++++++-----------------
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/include/net/seg6.h b/include/net/seg6.h
index 099bad5..5d308e9b 100644
--- a/include/net/seg6.h
+++ b/include/net/seg6.h
@@ -60,6 +60,11 @@ extern int seg6_local_init(void);
 extern void seg6_local_exit(void);
 
 extern bool seg6_validate_srh(struct ipv6_sr_hdr *srh, int len);
+extern struct ipv6_sr_hdr *seg6_get_srh(struct sk_buff *skb);
+extern void seg6_advance_nextseg(struct ipv6_sr_hdr *srh,
+				struct in6_addr *daddr);
+extern void seg6_lookup_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr,
+				u32 tbl_id);
 extern int seg6_do_srh_encap(struct sk_buff *skb, struct ipv6_sr_hdr *osrh,
 			     int proto);
 extern int seg6_do_srh_inline(struct sk_buff *skb, struct ipv6_sr_hdr *osrh);
diff --git a/net/ipv6/seg6_local.c b/net/ipv6/seg6_local.c
index 825b8e0..ea86ba8 100644
--- a/net/ipv6/seg6_local.c
+++ b/net/ipv6/seg6_local.c
@@ -59,7 +59,7 @@ static struct seg6_local_lwt *seg6_local_lwtunnel(struct lwtunnel_state *lwt)
 	return (struct seg6_local_lwt *)lwt->data;
 }
 
-static struct ipv6_sr_hdr *get_srh(struct sk_buff *skb)
+struct ipv6_sr_hdr *seg6_get_srh(struct sk_buff *skb)
 {
 	struct ipv6_sr_hdr *srh;
 	int len, srhoff = 0;
@@ -82,12 +82,13 @@ static struct ipv6_sr_hdr *get_srh(struct sk_buff *skb)
 
 	return srh;
 }
+EXPORT_SYMBOL_GPL(seg6_get_srh);
 
 static struct ipv6_sr_hdr *get_and_validate_srh(struct sk_buff *skb)
 {
 	struct ipv6_sr_hdr *srh;
 
-	srh = get_srh(skb);
+	srh = seg6_get_srh(skb);
 	if (!srh)
 		return NULL;
 
@@ -107,7 +108,7 @@ static bool decap_and_validate(struct sk_buff *skb, int proto)
 	struct ipv6_sr_hdr *srh;
 	unsigned int off = 0;
 
-	srh = get_srh(skb);
+	srh = seg6_get_srh(skb);
 	if (srh && srh->segments_left > 0)
 		return false;
 
@@ -131,7 +132,7 @@ static bool decap_and_validate(struct sk_buff *skb, int proto)
 	return true;
 }
 
-static void advance_nextseg(struct ipv6_sr_hdr *srh, struct in6_addr *daddr)
+void seg6_advance_nextseg(struct ipv6_sr_hdr *srh, struct in6_addr *daddr)
 {
 	struct in6_addr *addr;
 
@@ -139,9 +140,10 @@ static void advance_nextseg(struct ipv6_sr_hdr *srh, struct in6_addr *daddr)
 	addr = srh->segments + srh->segments_left;
 	*daddr = *addr;
 }
+EXPORT_SYMBOL_GPL(seg6_advance_nextseg);
 
-static void lookup_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr,
-			   u32 tbl_id)
+void seg6_lookup_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr,
+			 u32 tbl_id)
 {
 	struct net *net = dev_net(skb->dev);
 	struct ipv6hdr *hdr = ipv6_hdr(skb);
@@ -188,6 +190,7 @@ static void lookup_nexthop(struct sk_buff *skb, struct in6_addr *nhaddr,
 	skb_dst_drop(skb);
 	skb_dst_set(skb, dst);
 }
+EXPORT_SYMBOL_GPL(seg6_lookup_nexthop);
 
 /* regular endpoint function */
 static int input_action_end(struct sk_buff *skb, struct seg6_local_lwt *slwt)
@@ -198,9 +201,9 @@ static int input_action_end(struct sk_buff *skb, struct seg6_local_lwt *slwt)
 	if (!srh)
 		goto drop;
 
-	advance_nextseg(srh, &ipv6_hdr(skb)->daddr);
+	seg6_advance_nextseg(srh, &ipv6_hdr(skb)->daddr);
 
-	lookup_nexthop(skb, NULL, 0);
+	seg6_lookup_nexthop(skb, NULL, 0);
 
 	return dst_input(skb);
 
@@ -218,9 +221,9 @@ static int input_action_end_x(struct sk_buff *skb, struct seg6_local_lwt *slwt)
 	if (!srh)
 		goto drop;
 
-	advance_nextseg(srh, &ipv6_hdr(skb)->daddr);
+	seg6_advance_nextseg(srh, &ipv6_hdr(skb)->daddr);
 
-	lookup_nexthop(skb, &slwt->nh6, 0);
+	seg6_lookup_nexthop(skb, &slwt->nh6, 0);
 
 	return dst_input(skb);
 
@@ -237,9 +240,9 @@ static int input_action_end_t(struct sk_buff *skb, struct seg6_local_lwt *slwt)
 	if (!srh)
 		goto drop;
 
-	advance_nextseg(srh, &ipv6_hdr(skb)->daddr);
+	seg6_advance_nextseg(srh, &ipv6_hdr(skb)->daddr);
 
-	lookup_nexthop(skb, NULL, slwt->table);
+	seg6_lookup_nexthop(skb, NULL, slwt->table);
 
 	return dst_input(skb);
 
@@ -331,7 +334,7 @@ static int input_action_end_dx6(struct sk_buff *skb,
 	if (!ipv6_addr_any(&slwt->nh6))
 		nhaddr = &slwt->nh6;
 
-	lookup_nexthop(skb, nhaddr, 0);
+	seg6_lookup_nexthop(skb, nhaddr, 0);
 
 	return dst_input(skb);
 drop:
@@ -380,7 +383,7 @@ static int input_action_end_dt6(struct sk_buff *skb,
 	if (!pskb_may_pull(skb, sizeof(struct ipv6hdr)))
 		goto drop;
 
-	lookup_nexthop(skb, NULL, slwt->table);
+	seg6_lookup_nexthop(skb, NULL, slwt->table);
 
 	return dst_input(skb);
 
@@ -406,7 +409,7 @@ static int input_action_end_b6(struct sk_buff *skb, struct seg6_local_lwt *slwt)
 	ipv6_hdr(skb)->payload_len = htons(skb->len - sizeof(struct ipv6hdr));
 	skb_set_transport_header(skb, sizeof(struct ipv6hdr));
 
-	lookup_nexthop(skb, NULL, 0);
+	seg6_lookup_nexthop(skb, NULL, 0);
 
 	return dst_input(skb);
 
@@ -426,7 +429,7 @@ static int input_action_end_b6_encap(struct sk_buff *skb,
 	if (!srh)
 		goto drop;
 
-	advance_nextseg(srh, &ipv6_hdr(skb)->daddr);
+	seg6_advance_nextseg(srh, &ipv6_hdr(skb)->daddr);
 
 	skb_reset_inner_headers(skb);
 	skb->encapsulation = 1;
@@ -438,7 +441,7 @@ static int input_action_end_b6_encap(struct sk_buff *skb,
 	ipv6_hdr(skb)->payload_len = htons(skb->len - sizeof(struct ipv6hdr));
 	skb_set_transport_header(skb, sizeof(struct ipv6hdr));
 
-	lookup_nexthop(skb, NULL, 0);
+	seg6_lookup_nexthop(skb, NULL, 0);
 
 	return dst_input(skb);
 
-- 
2.1.4

^ permalink raw reply related

* Re: [RFT net-next v3 3/5] net: stmmac: dwmac-meson8b: fix internal RGMII clock configuration
From: Martin Blumenstingl @ 2017-12-29 23:40 UTC (permalink / raw)
  To: Jerome Brunet
  Cc: netdev, ingrassia, linus.luessing, khilman, linux-amlogic,
	Neil Armstrong, peppe.cavallaro, alexandre.torgue
In-Reply-To: <1514570234.7439.15.camel@baylibre.com>

Hi Jerome,

On Fri, Dec 29, 2017 at 6:57 PM, Jerome Brunet <jbrunet@baylibre.com> wrote:
> On Thu, 2017-12-28 at 23:21 +0100, Martin Blumenstingl wrote:
>> While testing the dwmac-meson8b with an RGMII PHY on Meson8b we
>> discovered that the m25_div is not actually a divider but rather a gate.
>> This matches with the datasheet which describes bit 10 as "Generate
>> 25MHz clock for PHY". Back when the driver was written it was assumed
>> that this was a divider (which could divide by 5 or 10) because other
>> clock bits in the datasheet were documented incorrectly.
>
> Maybe this bit 10 is indeed a 5/10 divider, as amlogic claims it is. Maybe, as
> Emiliano suggested, the output rate of div250 actually needs to be 250Mhz in
> RGMII, before being divided by 10 to produce the 25MHz of div25
>
> IOW, maybe we need this intermediate rate.
I am starting to believe that you (Emiliano and Jerome) are both right
does anyone of you have access to a scope so we can measure the actual
clock output?

> It would not be surprising, 1GBps usually requires a 125MHz clock somewhere.
this could mean that two clocks are derived from m250_div (names are
not final obviously):
- phy_ref_clk (25MHz or 50MHz)
- rgmii_tx_clk (fixed divide by 2, 125MHz)

> This is still doable:
> * Keep the divider
> * drop CLK_SET_RATE_PARENT on div25
> * call set_rate on div250 with 250MHz then on div25 with 25Mhz
yep, I will try this next
this would also be work with the assumption that the rgmii_tx_clk is
derived from m250_div

>
>>
>> Tests have shown that without bit 10 set the RTL8211F RGMII PHY on
>> Odroid-C1 (using a Meson8b SoC) does not work.
>> On GXBB and newer SoCs (where the driver was initially tested with RGMII
>> PHYs) this is not a problem because the input clock is running at 1GHz.
>> The m250_div clock's biggest possible divider is 7 (3-bit divider, with
>> value 0 being reserved). Thus we end up with a m250_div of 4 and a
>> "m25_div" of 10 (= register value 1).
>>
>> Instead it turns out that the Ethernet IP block seems to have a fixed
>> "divide by 10" clock internally. This means that bit 10 is a gate clock
>> which enables the RGMII clock output.
>>
>> This replaces the "m25_div" clock with a clock gate called "m25_en"
>> which ensures that we can set this bit to 1 whenever we enable RGMII
>> mode. This however means that we are now missing a "divide by 10" after
>> the m250_div (and before our new m25_en), otherwise the common clock
>> framework thinks that the rate of the m25_en clock is 10-times higher
>> than it is in the actual hardware. That is solved by adding a
>> fixed-factor clock which divides the m250_div output by 10.
>>
>> Fixes: 566e8251625304 ("net: stmmac: add a glue driver for the Amlogic Meson 8b / GXBB DWMAC")
>> Reported-by: Emiliano Ingrassia <ingrassia@epigenesys.com>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
>> ---
>>  .../net/ethernet/stmicro/stmmac/dwmac-meson8b.c    | 66 +++++++++++++---------
>>  1 file changed, 38 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
>> index 1c14210df465..7199e8c08536 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-meson8b.c
>> @@ -40,9 +40,7 @@
>>  #define PRG_ETH0_CLK_M250_DIV_SHIFT    7
>>  #define PRG_ETH0_CLK_M250_DIV_WIDTH    3
>>
>> -/* divides the result of m25_sel by either 5 (bit unset) or 10 (bit set) */
>> -#define PRG_ETH0_CLK_M25_DIV_SHIFT     10
>> -#define PRG_ETH0_CLK_M25_DIV_WIDTH     1
>> +#define PRG_ETH0_GENERATE_25M_PHY_CLOCK        10
>>
>>  #define PRG_ETH0_INVERTED_RMII_CLK     BIT(11)
>>  #define PRG_ETH0_TX_AND_PHY_REF_CLK    BIT(12)
>> @@ -63,8 +61,11 @@ struct meson8b_dwmac {
>>         struct clk_divider      m250_div;
>>         struct clk              *m250_div_clk;
>>
>> -       struct clk_divider      m25_div;
>> -       struct clk              *m25_div_clk;
>> +       struct clk_fixed_factor fixed_div10;
>> +       struct clk              *fixed_div10_clk;
>> +
>> +       struct clk_gate         m25_en;
>> +       struct clk              *m25_en_clk;
>
> Maybe it could be the topic of another series but we don't need to keep all
> those structures around, thanks to devm
>
> all clk_divider, fixed_factor, gate and mux can go away
> You only need to keep  the'struct clk*' you are going to use later on
>
> at the moment it would be m25_en_clk only.
let's get the whole thing to work first, then I will have another look
at the struct members (it already annoyed me too)


PS: on another note: the title of this series and most patches is
wrong as I just discovered:
the 25MHz clock is *NOT* the RGMII clock, but it's the "PHY reference
clock". Hardkernel calls it "ETH_PHY_REF_CLK_25MOUT" in their
Odroid-C1 schematics [4] on page 23, which is the only actual
description I could find for this pin (on the Khadas VIM2 schematics
for example there's a 25MHz clock seemingly coming out of nowhere)

I used three publicly available datasheets for reference:
1) TI DP83822 (MII/RMII/RGMII): [0]
page: 5
pin: XI
description for MII and RGMII: Reference clock 25-MHz ±50
ppm-tolerance crystal or oscillator input. The device supports either
an external crystal resonator connected across pins XI and XO, or an
external CMOS-level oscillator connected to pin XI only
description for RMII: RMII reference clock: Reference clock 50-MHz ±50
ppm-tolerance CMOS-level oscillator in RMII Slave mode. Reference
clock 25-MHz ±50 ppm-tolerance crystal or oscillator in RMII Master
mode.

2) micrel DP83822 (RGMII): [1]
page: 13
pin: XI
description: Crystal / Oscillator / External Clock Input
25MHz ±50ppm tolerance

3) Realtek RTL8211E (RGMII, should be close to the RTL8211F PHY on our
Amlogic boards): [2]
page: 17
pin: CKXTAL1
description: 25/50MHz Crystal Input

this shows that Ethernet PHYs "typically" support 25MHz and 50MHz as
"reference clock input"
it also supports Emiliano's and Jerome's suggestion that m250_div
should run at 250MHz and m25_div might act as a divide-by-5 or
divide-by-10 bit.


Regards
Martin


[0] http://www.ti.com/lit/ds/symlink/dp83822h.pdf
[1] https://datasheet.lcsc.com/szlcsc/KSZ9031RNXCA_C58758.pdf
[2] https://www.pine64.pro/download/documents/realtek-10-100-1000-ethernet.pdf
[3] https://dn.odroid.com/S805/Schematics/odroid-c1+_rev0.4_20160226.pdf

^ permalink raw reply

* [PATCH] af_key: fix buffer overread in verify_address_len()
From: Eric Biggers @ 2017-12-30  0:13 UTC (permalink / raw)
  To: netdev, Steffen Klassert, Herbert Xu, David S . Miller
  Cc: Alexander Potapenko, Dmitry Vyukov, Kostya Serebryany, syzkaller,
	Eric Biggers, stable
In-Reply-To: <CACT4Y+bVLWvMTqf4PoOJtu4_r5GseVAjDqE53kvm87ocxQz-ww@mail.gmail.com>

From: Eric Biggers <ebiggers@google.com>

If a message sent to a PF_KEY socket ended with one of the extensions
that takes a 'struct sadb_address' but there were not enough bytes
remaining in the message for the ->sa_family member of the 'struct
sockaddr' which is supposed to follow, then verify_address_len() read
past the end of the message, into uninitialized memory.  Fix it by
returning -EINVAL in this case.

This bug was found using syzkaller with KMSAN.

Reproducer:

	#include <linux/pfkeyv2.h>
	#include <sys/socket.h>
	#include <unistd.h>

	int main()
	{
		int sock = socket(PF_KEY, SOCK_RAW, PF_KEY_V2);
		char buf[24] = { 0 };
		struct sadb_msg *msg = (void *)buf;
		struct sadb_address *addr = (void *)(msg + 1);

		msg->sadb_msg_version = PF_KEY_V2;
		msg->sadb_msg_type = SADB_DELETE;
		msg->sadb_msg_len = 3;
		addr->sadb_address_len = 1;
		addr->sadb_address_exttype = SADB_EXT_ADDRESS_SRC;

		write(sock, buf, 24);
	}

Reported-by: Alexander Potapenko <glider@google.com>
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 net/key/af_key.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/key/af_key.c b/net/key/af_key.c
index 3dffb892d52c..596499cc8b2f 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -401,6 +401,11 @@ static int verify_address_len(const void *p)
 #endif
 	int len;
 
+	if (sp->sadb_address_len <
+	    DIV_ROUND_UP(sizeof(*sp) + offsetofend(typeof(*addr), sa_family),
+			 sizeof(uint64_t)))
+		return -EINVAL;
+
 	switch (addr->sa_family) {
 	case AF_INET:
 		len = DIV_ROUND_UP(sizeof(*sp) + sizeof(*sin), sizeof(uint64_t));
-- 
2.15.1

^ permalink raw reply related

* Re: [PATCH v3 bpf-next 1/2] tools/bpftool: use version from the kernel source tree
From: Daniel Borkmann @ 2017-12-30  0:13 UTC (permalink / raw)
  To: Roman Gushchin, netdev; +Cc: linux-kernel, kernel-team, Alexei Starovoitov
In-Reply-To: <20171227191629.4920-1-guro@fb.com>

On 12/27/2017 08:16 PM, Roman Gushchin wrote:
> Bpftool determines it's own version based on the kernel
> version, which is picked from the linux/version.h header.
> 
> It's strange to use the version of the installed kernel
> headers, and makes much more sense to use the version
> of the actual source tree, where bpftool sources are.
> 
> Fix this by building kernelversion target and use
> the resulting string as bpftool version.

Series applied to bpf-next, thanks everyone!

^ permalink raw reply

* [PATCH] af_key: fix buffer overread in parse_exthdrs()
From: Eric Biggers @ 2017-12-30  0:15 UTC (permalink / raw)
  To: netdev, Steffen Klassert, Herbert Xu, David S . Miller
  Cc: Alexander Potapenko, Dmitry Vyukov, Kostya Serebryany, syzkaller,
	Eric Biggers, stable

From: Eric Biggers <ebiggers@google.com>

If a message sent to a PF_KEY socket ended with an incomplete extension
header (fewer than 4 bytes remaining), then parse_exthdrs() read past
the end of the message, into uninitialized memory.  Fix it by returning
-EINVAL in this case.

Reproducer:

	#include <linux/pfkeyv2.h>
	#include <sys/socket.h>
	#include <unistd.h>

	int main()
	{
		int sock = socket(PF_KEY, SOCK_RAW, PF_KEY_V2);
		char buf[17] = { 0 };
		struct sadb_msg *msg = (void *)buf;

		msg->sadb_msg_version = PF_KEY_V2;
		msg->sadb_msg_type = SADB_DELETE;
		msg->sadb_msg_len = 2;

		write(sock, buf, 17);
	}

Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 net/key/af_key.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/key/af_key.c b/net/key/af_key.c
index 596499cc8b2f..d40861a048fe 100644
--- a/net/key/af_key.c
+++ b/net/key/af_key.c
@@ -516,6 +516,9 @@ static int parse_exthdrs(struct sk_buff *skb, const struct sadb_msg *hdr, void *
 		uint16_t ext_type;
 		int ext_len;
 
+		if (len < sizeof(*ehdr))
+			return -EINVAL;
+
 		ext_len  = ehdr->sadb_ext_len;
 		ext_len *= sizeof(uint64_t);
 		ext_type = ehdr->sadb_ext_type;
-- 
2.15.1

^ permalink raw reply related

* Re: KMSAN reports use of uninitialized memory in pfkey_sendmsg()
From: Eric Biggers @ 2017-12-30  0:19 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Alexander Potapenko, David Miller, Steffen Klassert, Herbert Xu,
	Networking, syzkaller, Kostya Serebryany
In-Reply-To: <CACT4Y+bVLWvMTqf4PoOJtu4_r5GseVAjDqE53kvm87ocxQz-ww@mail.gmail.com>

On Fri, Dec 29, 2017 at 05:49:34PM +0100, Dmitry Vyukov wrote:
> On Fri, Dec 29, 2017 at 5:48 PM, Alexander Potapenko <glider@google.com> wrote:
> > Hi all,
> >
> > KMSAN reports a use of uninitialized value on the following program:
> >
> > ==========================
> > // autogenerated by syzkaller (http://github.com/google/syzkaller)
> >
> > #include <string.h>
> > #include <sys/types.h>
> > #include <sys/socket.h>
> >
> > int main()
> > {
> >   int sock = socket(PF_KEY, SOCK_RAW, 2);
> >   char buf[24];
> >   memset(buf, 0, 24);
> >   buf[0] = 2;
> >   buf[1] = 4;
> >   buf[4] = 3;
> >   buf[16] = 1;
> >   buf[18] = 0x17;  // SADB_X_EXT_NAT_T_OA
> >   struct msghdr hdr;
> >   memset(&hdr, 0, sizeof(struct msghdr));
> >   struct iovec iov;
> >   hdr.msg_iov = &iov;
> >   hdr.msg_iovlen = 1;
> >   iov.iov_base = buf;
> >   iov.iov_len = 0x18;
> >   sendmsg(sock, &hdr, 0);
> > }
> > ==========================
> >
> > the report is below:
> >
> > ==================================================================
> > BUG: KMSAN: use of uninitialized memory in pfkey_sendmsg+0x11ad/0x1900
> > CPU: 0 PID: 2946 Comm: probe Not tainted 4.13.0+ #3626
> > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> > Call Trace:
> >  show_stack+0x12f/0x180 arch/x86/kernel/dumpstack.c:177
> >  __dump_stack lib/dump_stack.c:16
> >  dump_stack+0x185/0x1d0 lib/dump_stack.c:52
> >  kmsan_report+0x137/0x1c0 mm/kmsan/kmsan.c:1066
> >  __msan_warning_32+0x69/0xb0 mm/kmsan/kmsan_instr.c:676
> >  verify_address_len net/key/af_key.c:404
> >  parse_exthdrs net/key/af_key.c:532
> >  pfkey_process net/key/af_key.c:2811
> >  pfkey_sendmsg+0x11ad/0x1900 net/key/af_key.c:3654
> >  sock_sendmsg_nosec net/socket.c:633
> >  sock_sendmsg net/socket.c:643
> >  ___sys_sendmsg+0xed5/0x1330 net/socket.c:2035
> >  __sys_sendmsg net/socket.c:2069
> >  SYSC_sendmsg+0x2a6/0x3d0 net/socket.c:2080
> >  SyS_sendmsg+0x54/0x80 net/socket.c:2076
> >  do_syscall_64+0x2f4/0x420 arch/x86/entry/common.c:284
> >  entry_SYSCALL64_slow_path+0x25/0x25 arch/x86/entry/entry_64.S:245
> > RIP: 0033:0x401140
> > RSP: 002b:00007ffe52abc9e8 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
> > RAX: ffffffffffffffda RBX: 00000000004002b0 RCX: 0000000000401140
> > RDX: 0000000000000000 RSI: 00007ffe52abca10 RDI: 0000000000000003
> > RBP: 00007ffe52abca80 R08: 0000000000000002 R09: 0000000000000004
> > R10: 0000000000000004 R11: 0000000000000246 R12: 0000000000000000
> > R13: 00000000004063e0 R14: 0000000000406470 R15: 0000000000000000
> > origin:
> >  save_stack_trace+0x16/0x20 arch/x86/kernel/stacktrace.c:63
> >  kmsan_save_stack_with_flags mm/kmsan/kmsan.c:303
> >  kmsan_internal_poison_shadow+0xb8/0x1b0 mm/kmsan/kmsan.c:213
> >  kmsan_kmalloc+0x94/0x100 mm/kmsan/kmsan.c:339
> >  kmsan_slab_alloc+0x11/0x20 mm/kmsan/kmsan.c:346
> >  slab_post_alloc_hook mm/slab.h:442
> >  slab_alloc_node mm/slub.c:2724
> >  __kmalloc_node_track_caller+0xa46/0x1230 mm/slub.c:4335
> >  __kmalloc_reserve net/core/skbuff.c:139
> >  __alloc_skb+0x2c6/0x9f0 net/core/skbuff.c:232
> >  alloc_skb ./include/linux/skbuff.h:904
> >  pfkey_sendmsg+0x201/0x1900 net/key/af_key.c:3641
> >  sock_sendmsg_nosec net/socket.c:633
> >  sock_sendmsg net/socket.c:643
> >  ___sys_sendmsg+0xed5/0x1330 net/socket.c:2035
> >  __sys_sendmsg net/socket.c:2069
> >  SYSC_sendmsg+0x2a6/0x3d0 net/socket.c:2080
> >  SyS_sendmsg+0x54/0x80 net/socket.c:2076
> >  do_syscall_64+0x2f4/0x420 arch/x86/entry/common.c:284
> >  return_from_SYSCALL_64+0x0/0x6a arch/x86/entry/entry_64.S:245
> > ==================================================================
> >
> > Apparently pfkey_sendmsg reads skb past the end of the buffer copied
> > from the userspace.
> > Could someone please take a look?
> 

Thanks for reporting this!  The problem is that verify_address_len() doesn't
verify that the buffer has space for the ->sa_family field before reading it.
I've sent out a patch.

I also noticed a similar bug in parse_exthdrs(); it doesn't verify that the
buffer has space for the 'struct sadb_ext' before reading it.  I've sent out a
patch for that as well.

Eric

^ permalink raw reply

* RE: ATTENTION!!!
From: Loretta Robles @ 2017-12-30  0:28 UTC (permalink / raw)
  To: Loretta Robles
In-Reply-To: <35D4F79A8B8138489F38DE5CF6200454E570015C@HCI-EX-MB2.hci.utah.edu>


________________________________
From: Loretta Robles
Sent: Friday, December 29, 2017 1:01 PM
To: Loretta Robles
Subject: ATTENTION!!!

You have been randomly selected for a donation. Contact soriz4040@gmail.com for claims.

^ permalink raw reply

* Re: b53 tags on bpi-r1 (bcm53125)
From: Florian Fainelli @ 2017-12-30  3:22 UTC (permalink / raw)
  To: Jochen Friedrich; +Cc: netdev
In-Reply-To: <1552ddff-4c48-689f-d941-15d5b0a52f47@gmail.com>

Le 12/29/17 à 13:56, Florian Fainelli a écrit :
> Le 12/29/17 à 12:21, Florian Fainelli a écrit :
>> Hi Jochen,
>>
>> Le 12/18/17 à 02:44, Jochen Friedrich a écrit :
>>> Hi Florian,
>>>
>>> unfortunately, this doesn't make any difference.
>>>
>>> Just out of curiosity, BPI-R1 has pull-down resistors on LED6 and 7
>>> (MII_MODE0/1). However, the public available 53125U sheet doesn't
>>> document these pins:
>>>
>>> LED[6] IMP_MODE[0] Pull-up Active low (since IMP Mode is not used)
>>> LED[7] IMP_MODE[1] Pull-up Active low (since IMP Mode is not used)
>>>
>>> Is this MII mode maybe incompatible with Broadcom tags?
>>
>> AFAICT, it should not, this is largely independent from enabling
>> Broadcom tags.
>>
>> I have now reproduced this on my BPI-R1 as well and am looking at what
>> might be going wrong.
> 
> OK, so I have been able to find out what was going on. On BCM53125 and
> earlier switches, we need to do a couple of things for Broadcom tags to
> be usable:
> 
> - turn on managed mode (SM_SW_FWD_MODE)
> - configure Port 8 for IMP mode (B53_GLOBAL_CONFIG, setting bit
> GC_FRM_MGMT_PORT_MII)
> 
> After doing that, I can now see the correct outgoing packets on my host,
> however, the replies don't seem to be delivered to the per-port DSA
> network devices, and I suspect it's because of stmmac, so I am
> investigating this now.
> 

So stmmac was indeed part of the problem. I had to clear the
GMAC_CONTROL_ACS bit in GMAC_CORE_INIT in order to allow stmmac to
properly receive packets, otherwise, packets were truncated to 8 bytes
on reception which I assume is because the Broadcom tags may look like
some sort of weird LLC/SNAP packet which was confusing the hardware.
This is all working correctly now after a bunch of changes that I will
submit in the next few days.

Preliminary patches available here:

https://github.com/ffainelli/linux/commits/stmmac-fixes

Thank you very much for your patience!
-- 
Florian

^ permalink raw reply


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