Netdev List
 help / color / mirror / Atom feed
* [PATCH net-next] fm10k: use struct_size() in kzalloc()
From: Gustavo A. R. Silva @ 2019-02-08  3:55 UTC (permalink / raw)
  To: Jeff Kirsher, David S. Miller
  Cc: intel-wired-lan, netdev, linux-kernel, Gustavo A. R. Silva

One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:

struct foo {
    int stuff;
    struct boo entry[];
};

size = sizeof(struct foo) + count * sizeof(struct boo);
instance = kzalloc(size, GFP_KERNEL);

Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:

instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);

Notice that, in this case, variable size is not necessary, hence
it is removed.

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/net/ethernet/intel/fm10k/fm10k_main.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/fm10k/fm10k_main.c b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
index 6fd15a734324..5a0419421511 100644
--- a/drivers/net/ethernet/intel/fm10k/fm10k_main.c
+++ b/drivers/net/ethernet/intel/fm10k/fm10k_main.c
@@ -1603,14 +1603,12 @@ static int fm10k_alloc_q_vector(struct fm10k_intfc *interface,
 {
 	struct fm10k_q_vector *q_vector;
 	struct fm10k_ring *ring;
-	int ring_count, size;
+	int ring_count;
 
 	ring_count = txr_count + rxr_count;
-	size = sizeof(struct fm10k_q_vector) +
-	       (sizeof(struct fm10k_ring) * ring_count);
 
 	/* allocate q_vector and rings */
-	q_vector = kzalloc(size, GFP_KERNEL);
+	q_vector = kzalloc(struct_size(q_vector, ring, ring_count), GFP_KERNEL);
 	if (!q_vector)
 		return -ENOMEM;
 
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH][next] Bluetooth: a2mp: Use struct_size() helper
From: Joe Perches @ 2019-02-08  4:00 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Marcel Holtmann, Johan Hedberg,
	David S. Miller
  Cc: linux-bluetooth, netdev, linux-kernel
In-Reply-To: <20190208002817.GA15338@embeddedor>

On Thu, 2019-02-07 at 18:28 -0600, Gustavo A. R. Silva wrote:
> One of the more common cases of allocation size calculations is finding
> the size of a structure that has a zero-sized array at the end, along
> with memory for some number of elements for that array. For example:
> 
> struct foo {
>     int stuff;
>     struct boo entry[];
> };
> 
> size = sizeof(struct foo) + count * sizeof(struct boo);
> instance = alloc(size, GFP_KERNEL)
> 
> Instead of leaving these open-coded and prone to type mistakes, we can
> now use the new struct_size() helper:
> 
> size = struct_size(instance, entry, count);
> instance = alloc(size, GFP_KERNEL)
> 
> This code was detected with the help of Coccinelle.
[]
> diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
[]
> @@ -174,7 +174,7 @@ static int a2mp_discover_req(struct amp_mgr *mgr, struct sk_buff *skb,
>  			num_ctrl++;
>  	}
>  
> -	len = num_ctrl * sizeof(struct a2mp_cl) + sizeof(*rsp);
> +	len = struct_size(rsp, cl, num_ctrl);
>  	rsp = kmalloc(len, GFP_ATOMIC);
>  	if (!rsp) {
>  		read_unlock(&hci_dev_list_lock);

At least a weakness in this code is len is u16
and struct_size is size_t so there's a size
truncation possible.



^ permalink raw reply

* Re: [PATCH][next] Bluetooth: hci_event: Use struct_size() helper
From: Joe Perches @ 2019-02-08  4:02 UTC (permalink / raw)
  To: Gustavo A. R. Silva, Marcel Holtmann, Johan Hedberg,
	David S. Miller
  Cc: linux-bluetooth, netdev, linux-kernel
In-Reply-To: <20190208004033.GA15609@embeddedor>

On Thu, 2019-02-07 at 18:40 -0600, Gustavo A. R. Silva wrote:
> Make use of the struct_size() helper instead of an open-coded version
> in order to avoid any potential type mistakes, in particular in the
> context in which this code is being used.
> 
> So, change the following form:
> 
> sizeof(*ev) + ev->num_hndl * sizeof(struct hci_comp_pkts_info)
> 
>  to :
> 
> struct_size(ev, handles, ev->num_hndl)
> 
> This code was detected with the help of Coccinelle.
[]
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
[]
> @@ -3556,8 +3556,8 @@ static void hci_num_comp_pkts_evt(struct hci_dev *hdev, struct sk_buff *skb)
>  		return;
>  	}
>  
> -	if (skb->len < sizeof(*ev) || skb->len < sizeof(*ev) +
> -	    ev->num_hndl * sizeof(struct hci_comp_pkts_info)) {
> +	if (skb->len < sizeof(*ev) ||
> +	    skb->len < struct_size(ev, handles, ev->num_hndl)) {
>  		BT_DBG("%s bad parameters", hdev->name);
>  		return;
>  	}
> @@ -3644,8 +3644,8 @@ static void hci_num_comp_blocks_evt(struct hci_dev *hdev, struct sk_buff *skb)
>  		return;
>  	}
>  
> -	if (skb->len < sizeof(*ev) || skb->len < sizeof(*ev) +
> -	    ev->num_hndl * sizeof(struct hci_comp_blocks_info)) {
> +	if (skb->len < sizeof(*ev) ||
> +	    skb->len < struct_size(ev, handles, ev->num_hndl)) {

Unless these are in a real fast path where skb->len < sizeof(*ev)
is pretty likely, it seems a bit redundant as the multiplication
is pretty cheap and num_hndl is unsigned (actually __u8)

This could/should be simply

	if (skb->len < struct_size(...))



^ permalink raw reply

* [PATCH net] net: dsa: microchip: add switch offload forwarding support
From: Tristram.Ha @ 2019-02-08  4:05 UTC (permalink / raw)
  To: David S. Miller, Sergio Paracuellos
  Cc: Tristram Ha, Andrew Lunn, Florian Fainelli, Pavel Machek,
	UNGLinuxDriver, netdev

From: Tristram Ha <Tristram.Ha@microchip.com>

The flag offload_fwd_mark is set as the switch can forward frames by
itself.

This can be considered a fix to a problem introduced in commit
c2e866911e254067 where the port membership are not set in sync.  The flag
offload_fwd_mark just needs to be set in tag_ksz.c to prevent the software
bridge from forwarding duplicate multicast frames.

Fixes: c2e866911e254067 ("microchip: break KSZ9477 DSA driver into two files")
Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477.c | 7 ++++---
 net/dsa/tag_ksz.c                   | 2 ++
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 89ed059..674d77e 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -397,6 +397,7 @@ static void ksz9477_port_stp_state_set(struct dsa_switch *ds, int port,
 	struct ksz_port *p = &dev->ports[port];
 	u8 data;
 	int member = -1;
+	int forward = dev->member;
 
 	ksz_pread8(dev, port, P_STP_CTRL, &data);
 	data &= ~(PORT_TX_ENABLE | PORT_RX_ENABLE | PORT_LEARN_DISABLE);
@@ -464,10 +465,10 @@ static void ksz9477_port_stp_state_set(struct dsa_switch *ds, int port,
 	}
 
 	/* When topology has changed the function ksz_update_port_member
-	 * should be called to modify port forwarding behavior.  However
-	 * as the offload_fwd_mark indication cannot be reported here
-	 * the switch forwarding function is not enabled.
+	 * should be called to modify port forwarding behavior.
 	 */
+	if (forward != dev->member)
+		ksz_update_port_member(dev, port);
 }
 
 static void ksz9477_flush_dyn_mac_table(struct ksz_device *dev, int port)
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index da71b9e..927e9c8 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -67,6 +67,8 @@ static struct sk_buff *ksz_common_rcv(struct sk_buff *skb,
 
 	pskb_trim_rcsum(skb, skb->len - len);
 
+	skb->offload_fwd_mark = true;
+
 	return skb;
 }
 
-- 
1.9.1


^ permalink raw reply related

* [PATCH v1 net-next 0/4] net: dsa: microchip: add MIB counters support
From: Tristram.Ha @ 2019-02-08  4:07 UTC (permalink / raw)
  To: Sergio Paracuellos, Andrew Lunn, Florian Fainelli, Pavel Machek
  Cc: Tristram Ha, UNGLinuxDriver, netdev

From: Tristram Ha <Tristram.Ha@microchip.com>

This series of patches is to modify the KSZ9477 DSA driver to read MIB
counters periodically to avoid overflow.

v1
- Use readx_poll_time
- Do not clear MIB counters when port is enabled
- Do not advertise 1000 half-duplex mode when port is enabled
- Do not use freeze function as MIB counters may miss counts

Tristram Ha (4):
  net: dsa: microchip: prepare PHY for proper advertisement
  net: dsa: microchip: add MIB counter reading support
  net: dsa: microchip: use readx_poll_time for polling
  net: dsa: microchip: remove unnecessary include headers

 drivers/net/dsa/microchip/ksz9477.c    | 249 ++++++++++++++++++---------------
 drivers/net/dsa/microchip/ksz_common.c | 115 ++++++++++++++-
 drivers/net/dsa/microchip/ksz_common.h |   6 +-
 drivers/net/dsa/microchip/ksz_priv.h   |  11 +-
 4 files changed, 264 insertions(+), 117 deletions(-)

-- 
1.9.1


^ permalink raw reply

* [PATCH v1 net-next 1/4] net: dsa: microchip: prepare PHY for proper advertisement
From: Tristram.Ha @ 2019-02-08  4:07 UTC (permalink / raw)
  To: Sergio Paracuellos, Andrew Lunn, Florian Fainelli, Pavel Machek
  Cc: Tristram Ha, UNGLinuxDriver, netdev
In-Reply-To: <1549598829-25970-1-git-send-email-Tristram.Ha@microchip.com>

From: Tristram Ha <Tristram.Ha@microchip.com>

Prepare PHY for proper advertisement and get link status for the port.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477.c    | 17 ++++++++++++++++-
 drivers/net/dsa/microchip/ksz_common.c | 19 ++++++++++++++++++-
 drivers/net/dsa/microchip/ksz_common.h |  4 +++-
 drivers/net/dsa/microchip/ksz_priv.h   |  4 +++-
 4 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 89ed059..0fdb22d 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -2,7 +2,7 @@
 /*
  * Microchip KSZ9477 switch driver main logic
  *
- * Copyright (C) 2017-2018 Microchip Technology Inc.
+ * Copyright (C) 2017-2019 Microchip Technology Inc.
  */
 
 #include <linux/delay.h>
@@ -965,6 +965,19 @@ static void ksz9477_port_mirror_del(struct dsa_switch *ds, int port,
 			     PORT_MIRROR_SNIFFER, false);
 }
 
+static void ksz9477_phy_setup(struct ksz_device *dev, int port,
+			      struct phy_device *phy)
+{
+	/* ETHTOOL_LINK_MODE_Pause_BIT and ETHTOOL_LINK_MODE_Asym_Pause_BIT
+	 * can be removed to disable flow control when rate limiting is used.
+	 */
+	if (port < dev->phy_port_cnt) {
+		/* The MAC actually cannot run in 1000 half-duplex mode. */
+		phy_remove_link_mode(phy,
+				     ETHTOOL_LINK_MODE_1000baseT_Half_BIT);
+	}
+}
+
 static void ksz9477_port_setup(struct ksz_device *dev, int port, bool cpu_port)
 {
 	u8 data8;
@@ -1151,6 +1164,7 @@ static int ksz9477_setup(struct dsa_switch *ds)
 	.setup			= ksz9477_setup,
 	.phy_read		= ksz9477_phy_read16,
 	.phy_write		= ksz9477_phy_write16,
+	.adjust_link		= ksz_adjust_link,
 	.port_enable		= ksz_enable_port,
 	.port_disable		= ksz_disable_port,
 	.get_strings		= ksz9477_get_strings,
@@ -1298,6 +1312,7 @@ static void ksz9477_switch_exit(struct ksz_device *dev)
 	.get_port_addr = ksz9477_get_port_addr,
 	.cfg_port_member = ksz9477_cfg_port_member,
 	.flush_dyn_mac_table = ksz9477_flush_dyn_mac_table,
+	.phy_setup = ksz9477_phy_setup,
 	.port_setup = ksz9477_port_setup,
 	.shutdown = ksz9477_reset_switch,
 	.detect = ksz9477_switch_detect,
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index 8a5111f..a57bda7 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -2,7 +2,7 @@
 /*
  * Microchip switch driver main logic
  *
- * Copyright (C) 2017-2018 Microchip Technology Inc.
+ * Copyright (C) 2017-2019 Microchip Technology Inc.
  */
 
 #include <linux/delay.h>
@@ -61,6 +61,22 @@ int ksz_phy_write16(struct dsa_switch *ds, int addr, int reg, u16 val)
 }
 EXPORT_SYMBOL_GPL(ksz_phy_write16);
 
+void ksz_adjust_link(struct dsa_switch *ds, int port,
+		     struct phy_device *phydev)
+{
+	struct ksz_device *dev = ds->priv;
+	struct ksz_port *p = &dev->ports[port];
+
+	if (phydev->link) {
+		dev->live_ports |= (1 << port) & dev->on_ports;
+	} else if (p->phydev.link) {
+		p->link_just_down = 1;
+		dev->live_ports &= ~(1 << port);
+	}
+	p->phydev = *phydev;
+}
+EXPORT_SYMBOL_GPL(ksz_adjust_link);
+
 int ksz_sset_count(struct dsa_switch *ds, int port, int sset)
 {
 	struct ksz_device *dev = ds->priv;
@@ -238,6 +254,7 @@ int ksz_enable_port(struct dsa_switch *ds, int port, struct phy_device *phy)
 
 	/* setup slave port */
 	dev->dev_ops->port_setup(dev, port, false);
+	dev->dev_ops->phy_setup(dev, port, phy);
 
 	/* port_stp_state_set() will be called after to enable the port so
 	 * there is no need to do anything.
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 2dd832d..6d49319 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -1,7 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0
  * Microchip switch driver common header
  *
- * Copyright (C) 2017-2018 Microchip Technology Inc.
+ * Copyright (C) 2017-2019 Microchip Technology Inc.
  */
 
 #ifndef __KSZ_COMMON_H
@@ -13,6 +13,8 @@
 
 int ksz_phy_read16(struct dsa_switch *ds, int addr, int reg);
 int ksz_phy_write16(struct dsa_switch *ds, int addr, int reg, u16 val);
+void ksz_adjust_link(struct dsa_switch *ds, int port,
+		     struct phy_device *phydev);
 int ksz_sset_count(struct dsa_switch *ds, int port, int sset);
 int ksz_port_bridge_join(struct dsa_switch *ds, int port,
 			 struct net_device *br);
diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h
index 60b4901..0fdc58b 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -2,7 +2,7 @@
  *
  * Microchip KSZ series switch common definitions
  *
- * Copyright (C) 2017-2018 Microchip Technology Inc.
+ * Copyright (C) 2017-2019 Microchip Technology Inc.
  */
 
 #ifndef __KSZ_PRIV_H
@@ -137,6 +137,8 @@ struct ksz_dev_ops {
 	u32 (*get_port_addr)(int port, int offset);
 	void (*cfg_port_member)(struct ksz_device *dev, int port, u8 member);
 	void (*flush_dyn_mac_table)(struct ksz_device *dev, int port);
+	void (*phy_setup)(struct ksz_device *dev, int port,
+			  struct phy_device *phy);
 	void (*port_setup)(struct ksz_device *dev, int port, bool cpu_port);
 	void (*r_phy)(struct ksz_device *dev, u16 phy, u16 reg, u16 *val);
 	void (*w_phy)(struct ksz_device *dev, u16 phy, u16 reg, u16 val);
-- 
1.9.1


^ permalink raw reply related

* Re: [PATCH][next] Bluetooth: a2mp: Use struct_size() helper
From: Gustavo A. R. Silva @ 2019-02-08  4:07 UTC (permalink / raw)
  To: Joe Perches, Marcel Holtmann, Johan Hedberg, David S. Miller
  Cc: linux-bluetooth, netdev, linux-kernel
In-Reply-To: <4faff31066c10285bab0afdb0c1f88f6d3d1a21b.camel@perches.com>



On 2/7/19 10:00 PM, Joe Perches wrote:
> On Thu, 2019-02-07 at 18:28 -0600, Gustavo A. R. Silva wrote:
>> One of the more common cases of allocation size calculations is finding
>> the size of a structure that has a zero-sized array at the end, along
>> with memory for some number of elements for that array. For example:
>>
>> struct foo {
>>     int stuff;
>>     struct boo entry[];
>> };
>>
>> size = sizeof(struct foo) + count * sizeof(struct boo);
>> instance = alloc(size, GFP_KERNEL)
>>
>> Instead of leaving these open-coded and prone to type mistakes, we can
>> now use the new struct_size() helper:
>>
>> size = struct_size(instance, entry, count);
>> instance = alloc(size, GFP_KERNEL)
>>
>> This code was detected with the help of Coccinelle.
> []
>> diff --git a/net/bluetooth/a2mp.c b/net/bluetooth/a2mp.c
> []
>> @@ -174,7 +174,7 @@ static int a2mp_discover_req(struct amp_mgr *mgr, struct sk_buff *skb,
>>  			num_ctrl++;
>>  	}
>>  
>> -	len = num_ctrl * sizeof(struct a2mp_cl) + sizeof(*rsp);
>> +	len = struct_size(rsp, cl, num_ctrl);
>>  	rsp = kmalloc(len, GFP_ATOMIC);
>>  	if (!rsp) {
>>  		read_unlock(&hci_dev_list_lock);
> 
> At least a weakness in this code is len is u16
> and struct_size is size_t so there's a size
> truncation possible.
> 
> 

That's true.  I didn't change the type to size_t because of the call
to le16_to_cpu():

u16 len = le16_to_cpu(hdr->len);

I've been changing the type of the variable in other cases.

--
Gustavo

^ permalink raw reply

* [PATCH v1 net-next 3/4] net: dsa: microchip: use readx_poll_time for polling
From: Tristram.Ha @ 2019-02-08  4:07 UTC (permalink / raw)
  To: Sergio Paracuellos, Andrew Lunn, Florian Fainelli, Pavel Machek
  Cc: Tristram Ha, UNGLinuxDriver, netdev
In-Reply-To: <1549598829-25970-1-git-send-email-Tristram.Ha@microchip.com>

From: Tristram Ha <Tristram.Ha@microchip.com>

Replace register polling functions using timeout with readx_poll_time call.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477.c | 91 +++++++++++--------------------------
 1 file changed, 27 insertions(+), 64 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 4502e13..8391b9e 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -114,28 +114,11 @@ static void ksz9477_port_cfg32(struct ksz_device *dev, int port, int offset,
 	data; \
 })
 
-static int ksz9477_wait_vlan_ctrl_ready(struct ksz_device *dev, u32 waiton,
-					int timeout)
-{
-	u8 data;
-
-	do {
-		ksz_read8(dev, REG_SW_VLAN_CTRL, &data);
-		if (!(data & waiton))
-			break;
-		usleep_range(1, 10);
-	} while (timeout-- > 0);
-
-	if (timeout <= 0)
-		return -ETIMEDOUT;
-
-	return 0;
-}
-
 static int ksz9477_get_vlan_table(struct ksz_device *dev, u16 vid,
 				  u32 *vlan_table)
 {
 	int ret;
+	u8 data;
 
 	mutex_lock(&dev->vlan_mutex);
 
@@ -143,7 +126,8 @@ static int ksz9477_get_vlan_table(struct ksz_device *dev, u16 vid,
 	ksz_write8(dev, REG_SW_VLAN_CTRL, VLAN_READ | VLAN_START);
 
 	/* wait to be cleared */
-	ret = ksz9477_wait_vlan_ctrl_ready(dev, VLAN_START, 1000);
+	ret = readx_poll_timeout(read8_op, REG_SW_VLAN_CTRL, data,
+				 !(data & VLAN_START), 10, 1000);
 	if (ret < 0) {
 		dev_dbg(dev->dev, "Failed to read vlan table\n");
 		goto exit;
@@ -165,6 +149,7 @@ static int ksz9477_set_vlan_table(struct ksz_device *dev, u16 vid,
 				  u32 *vlan_table)
 {
 	int ret;
+	u8 data;
 
 	mutex_lock(&dev->vlan_mutex);
 
@@ -176,7 +161,8 @@ static int ksz9477_set_vlan_table(struct ksz_device *dev, u16 vid,
 	ksz_write8(dev, REG_SW_VLAN_CTRL, VLAN_START | VLAN_WRITE);
 
 	/* wait to be cleared */
-	ret = ksz9477_wait_vlan_ctrl_ready(dev, VLAN_START, 1000);
+	ret = readx_poll_timeout(read8_op, REG_SW_VLAN_CTRL, data,
+				 !(data & VLAN_START), 10, 1000);
 	if (ret < 0) {
 		dev_dbg(dev->dev, "Failed to write vlan table\n");
 		goto exit;
@@ -211,42 +197,6 @@ static void ksz9477_write_table(struct ksz_device *dev, u32 *table)
 	ksz_write32(dev, REG_SW_ALU_VAL_D, table[3]);
 }
 
-static int ksz9477_wait_alu_ready(struct ksz_device *dev, u32 waiton,
-				  int timeout)
-{
-	u32 data;
-
-	do {
-		ksz_read32(dev, REG_SW_ALU_CTRL__4, &data);
-		if (!(data & waiton))
-			break;
-		usleep_range(1, 10);
-	} while (timeout-- > 0);
-
-	if (timeout <= 0)
-		return -ETIMEDOUT;
-
-	return 0;
-}
-
-static int ksz9477_wait_alu_sta_ready(struct ksz_device *dev, u32 waiton,
-				      int timeout)
-{
-	u32 data;
-
-	do {
-		ksz_read32(dev, REG_SW_ALU_STAT_CTRL__4, &data);
-		if (!(data & waiton))
-			break;
-		usleep_range(1, 10);
-	} while (timeout-- > 0);
-
-	if (timeout <= 0)
-		return -ETIMEDOUT;
-
-	return 0;
-}
-
 static int ksz9477_reset_switch(struct ksz_device *dev)
 {
 	u8 data8;
@@ -649,7 +599,8 @@ static int ksz9477_port_fdb_add(struct dsa_switch *ds, int port,
 	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_READ | ALU_START);
 
 	/* wait to be finished */
-	ret = ksz9477_wait_alu_ready(dev, ALU_START, 1000);
+	ret = readx_poll_timeout(read32_op, REG_SW_ALU_CTRL__4, data,
+				 !(data & ALU_START), 10, 1000);
 	if (ret < 0) {
 		dev_dbg(dev->dev, "Failed to read ALU\n");
 		goto exit;
@@ -673,7 +624,8 @@ static int ksz9477_port_fdb_add(struct dsa_switch *ds, int port,
 	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_WRITE | ALU_START);
 
 	/* wait to be finished */
-	ret = ksz9477_wait_alu_ready(dev, ALU_START, 1000);
+	ret = readx_poll_timeout(read32_op, REG_SW_ALU_CTRL__4, data,
+				 !(data & ALU_START), 10, 1000);
 	if (ret < 0)
 		dev_dbg(dev->dev, "Failed to write ALU\n");
 
@@ -706,7 +658,8 @@ static int ksz9477_port_fdb_del(struct dsa_switch *ds, int port,
 	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_READ | ALU_START);
 
 	/* wait to be finished */
-	ret = ksz9477_wait_alu_ready(dev, ALU_START, 1000);
+	ret = readx_poll_timeout(read32_op, REG_SW_ALU_CTRL__4, data,
+				 !(data & ALU_START), 10, 1000);
 	if (ret < 0) {
 		dev_dbg(dev->dev, "Failed to read ALU\n");
 		goto exit;
@@ -740,7 +693,8 @@ static int ksz9477_port_fdb_del(struct dsa_switch *ds, int port,
 	ksz_write32(dev, REG_SW_ALU_CTRL__4, ALU_WRITE | ALU_START);
 
 	/* wait to be finished */
-	ret = ksz9477_wait_alu_ready(dev, ALU_START, 1000);
+	ret = readx_poll_timeout(read32_op, REG_SW_ALU_CTRL__4, data,
+				 !(data & ALU_START), 10, 1000);
 	if (ret < 0)
 		dev_dbg(dev->dev, "Failed to write ALU\n");
 
@@ -832,6 +786,7 @@ static void ksz9477_port_mdb_add(struct dsa_switch *ds, int port,
 	u32 static_table[4];
 	u32 data;
 	int index;
+	int ret;
 	u32 mac_hi, mac_lo;
 
 	mac_hi = ((mdb->addr[0] << 8) | mdb->addr[1]);
@@ -847,7 +802,10 @@ static void ksz9477_port_mdb_add(struct dsa_switch *ds, int port,
 		ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
 
 		/* wait to be finished */
-		if (ksz9477_wait_alu_sta_ready(dev, ALU_STAT_START, 1000) < 0) {
+		ret = readx_poll_timeout(read32_op, REG_SW_ALU_STAT_CTRL__4,
+					 data, !(data & ALU_STAT_START),
+					 10, 1000);
+		if (ret < 0) {
 			dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
 			goto exit;
 		}
@@ -888,7 +846,9 @@ static void ksz9477_port_mdb_add(struct dsa_switch *ds, int port,
 	ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
 
 	/* wait to be finished */
-	if (ksz9477_wait_alu_sta_ready(dev, ALU_STAT_START, 1000) < 0)
+	ret = readx_poll_timeout(read32_op, REG_SW_ALU_STAT_CTRL__4, data,
+				 !(data & ALU_STAT_START), 10, 1000);
+	if (ret < 0)
 		dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
 
 exit:
@@ -918,7 +878,9 @@ static int ksz9477_port_mdb_del(struct dsa_switch *ds, int port,
 		ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
 
 		/* wait to be finished */
-		ret = ksz9477_wait_alu_sta_ready(dev, ALU_STAT_START, 1000);
+		ret = readx_poll_timeout(read32_op, REG_SW_ALU_STAT_CTRL__4,
+					 data, !(data & ALU_STAT_START),
+					 10, 1000);
 		if (ret < 0) {
 			dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
 			goto exit;
@@ -960,7 +922,8 @@ static int ksz9477_port_mdb_del(struct dsa_switch *ds, int port,
 	ksz_write32(dev, REG_SW_ALU_STAT_CTRL__4, data);
 
 	/* wait to be finished */
-	ret = ksz9477_wait_alu_sta_ready(dev, ALU_STAT_START, 1000);
+	ret = readx_poll_timeout(read32_op, REG_SW_ALU_STAT_CTRL__4, data,
+				 !(data & ALU_STAT_START), 10, 1000);
 	if (ret < 0)
 		dev_dbg(dev->dev, "Failed to read ALU STATIC\n");
 
-- 
1.9.1


^ permalink raw reply related

* [PATCH v1 net-next 2/4] net: dsa: microchip: add MIB counter reading support
From: Tristram.Ha @ 2019-02-08  4:07 UTC (permalink / raw)
  To: Sergio Paracuellos, Andrew Lunn, Florian Fainelli, Pavel Machek
  Cc: Tristram Ha, UNGLinuxDriver, netdev
In-Reply-To: <1549598829-25970-1-git-send-email-Tristram.Ha@microchip.com>

From: Tristram Ha <Tristram.Ha@microchip.com>

Add MIB counter reading support.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477.c    | 139 +++++++++++++++++++++++----------
 drivers/net/dsa/microchip/ksz_common.c |  96 +++++++++++++++++++++++
 drivers/net/dsa/microchip/ksz_common.h |   2 +
 drivers/net/dsa/microchip/ksz_priv.h   |   7 +-
 4 files changed, 198 insertions(+), 46 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 0fdb22d..4502e13 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -10,6 +10,7 @@
 #include <linux/gpio.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
+#include <linux/iopoll.h>
 #include <linux/platform_data/microchip-ksz.h>
 #include <linux/phy.h>
 #include <linux/etherdevice.h>
@@ -18,8 +19,8 @@
 #include <net/switchdev.h>
 
 #include "ksz_priv.h"
-#include "ksz_common.h"
 #include "ksz9477_reg.h"
+#include "ksz_common.h"
 
 static const struct {
 	int index;
@@ -92,6 +93,27 @@ static void ksz9477_port_cfg32(struct ksz_device *dev, int port, int offset,
 	ksz_write32(dev, addr, data);
 }
 
+#define read8_op(addr)	\
+({ \
+	u8 data; \
+	ksz_read8(dev, addr, &data); \
+	data; \
+})
+
+#define read32_op(addr)	\
+({ \
+	u32 data; \
+	ksz_read32(dev, addr, &data); \
+	data; \
+})
+
+#define pread32_op(addr)	\
+({ \
+	u32 data; \
+	ksz_pread32(dev, port, addr, &data); \
+	data; \
+})
+
 static int ksz9477_wait_vlan_ctrl_ready(struct ksz_device *dev, u32 waiton,
 					int timeout)
 {
@@ -259,6 +281,70 @@ static int ksz9477_reset_switch(struct ksz_device *dev)
 	return 0;
 }
 
+static void ksz9477_r_mib_cnt(struct ksz_device *dev, int port, u16 addr,
+			      u64 *cnt)
+{
+	u32 data;
+	int ret;
+	struct ksz_port *p = &dev->ports[port];
+
+	/* retain the flush/freeze bit */
+	data = p->freeze ? MIB_COUNTER_FLUSH_FREEZE : 0;
+	data |= MIB_COUNTER_READ;
+	data |= (addr << MIB_COUNTER_INDEX_S);
+	ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, data);
+
+	ret = readx_poll_timeout(pread32_op, REG_PORT_MIB_CTRL_STAT__4, data,
+				 !(data & MIB_COUNTER_READ), 10, 1000);
+
+	/* failed to read MIB. get out of loop */
+	if (ret < 0) {
+		dev_dbg(dev->dev, "Failed to get MIB\n");
+		return;
+	}
+
+	/* count resets upon read */
+	ksz_pread32(dev, port, REG_PORT_MIB_DATA, &data);
+	*cnt += data;
+}
+
+static void ksz9477_r_mib_pkt(struct ksz_device *dev, int port, u16 addr,
+			      u64 *dropped, u64 *cnt)
+{
+	addr = ksz9477_mib_names[addr].index;
+	ksz9477_r_mib_cnt(dev, port, addr, cnt);
+}
+
+static void ksz9477_freeze_mib(struct ksz_device *dev, int port, bool freeze)
+{
+	struct ksz_port *p = &dev->ports[port];
+	u32 val = freeze ? MIB_COUNTER_FLUSH_FREEZE : 0;
+
+	/* enable/disable the port for flush/freeze function */
+	mutex_lock(&p->mib.cnt_mutex);
+	ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, val);
+
+	/* used by MIB counter reading code to know freeze is enabled */
+	p->freeze = freeze;
+	mutex_unlock(&p->mib.cnt_mutex);
+}
+
+static void ksz9477_port_init_cnt(struct ksz_device *dev, int port)
+{
+	struct ksz_port_mib *mib = &dev->ports[port].mib;
+
+	/* flush all enabled port MIB counters */
+	mutex_lock(&mib->cnt_mutex);
+	ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4,
+		     MIB_COUNTER_FLUSH_FREEZE);
+	ksz_write8(dev, REG_SW_MAC_CTRL_6, SW_MIB_COUNTER_FLUSH);
+	ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, 0);
+	mutex_unlock(&mib->cnt_mutex);
+
+	mib->cnt_ptr = 0;
+	memset(mib->counters, 0, dev->mib_cnt * sizeof(u64));
+}
+
 static enum dsa_tag_protocol ksz9477_get_tag_protocol(struct dsa_switch *ds,
 						      int port)
 {
@@ -342,47 +428,6 @@ static void ksz9477_get_strings(struct dsa_switch *ds, int port,
 	}
 }
 
-static void ksz_get_ethtool_stats(struct dsa_switch *ds, int port,
-				  uint64_t *buf)
-{
-	struct ksz_device *dev = ds->priv;
-	int i;
-	u32 data;
-	int timeout;
-
-	mutex_lock(&dev->stats_mutex);
-
-	for (i = 0; i < TOTAL_SWITCH_COUNTER_NUM; i++) {
-		data = MIB_COUNTER_READ;
-		data |= ((ksz9477_mib_names[i].index & 0xFF) <<
-			MIB_COUNTER_INDEX_S);
-		ksz_pwrite32(dev, port, REG_PORT_MIB_CTRL_STAT__4, data);
-
-		timeout = 1000;
-		do {
-			ksz_pread32(dev, port, REG_PORT_MIB_CTRL_STAT__4,
-				    &data);
-			usleep_range(1, 10);
-			if (!(data & MIB_COUNTER_READ))
-				break;
-		} while (timeout-- > 0);
-
-		/* failed to read MIB. get out of loop */
-		if (!timeout) {
-			dev_dbg(dev->dev, "Failed to get MIB\n");
-			break;
-		}
-
-		/* count resets upon read */
-		ksz_pread32(dev, port, REG_PORT_MIB_DATA, &data);
-
-		dev->mib_value[i] += (uint64_t)data;
-		buf[i] = dev->mib_value[i];
-	}
-
-	mutex_unlock(&dev->stats_mutex);
-}
-
 static void ksz9477_cfg_port_member(struct ksz_device *dev, int port,
 				    u8 member)
 {
@@ -1153,9 +1198,14 @@ static int ksz9477_setup(struct dsa_switch *ds)
 	/* queue based egress rate limit */
 	ksz_cfg(dev, REG_SW_MAC_CTRL_5, SW_OUT_RATE_LIMIT_QUEUE_BASED, true);
 
+	/* enable global MIB counter freeze function */
+	ksz_cfg(dev, REG_SW_MAC_CTRL_6, SW_MIB_COUNTER_FREEZE, true);
+
 	/* start switch */
 	ksz_cfg(dev, REG_SW_OPERATION, SW_START, true);
 
+	ksz_init_mib_timer(dev);
+
 	return 0;
 }
 
@@ -1290,6 +1340,7 @@ static int ksz9477_switch_init(struct ksz_device *dev)
 	if (!dev->ports)
 		return -ENOMEM;
 	for (i = 0; i < dev->mib_port_cnt; i++) {
+		mutex_init(&dev->ports[i].mib.cnt_mutex);
 		dev->ports[i].mib.counters =
 			devm_kzalloc(dev->dev,
 				     sizeof(u64) *
@@ -1314,6 +1365,10 @@ static void ksz9477_switch_exit(struct ksz_device *dev)
 	.flush_dyn_mac_table = ksz9477_flush_dyn_mac_table,
 	.phy_setup = ksz9477_phy_setup,
 	.port_setup = ksz9477_port_setup,
+	.r_mib_cnt = ksz9477_r_mib_cnt,
+	.r_mib_pkt = ksz9477_r_mib_pkt,
+	.freeze_mib = ksz9477_freeze_mib,
+	.port_init_cnt = ksz9477_port_init_cnt,
 	.shutdown = ksz9477_reset_switch,
 	.detect = ksz9477_switch_detect,
 	.init = ksz9477_switch_init,
diff --git a/drivers/net/dsa/microchip/ksz_common.c b/drivers/net/dsa/microchip/ksz_common.c
index a57bda7..62d4344 100644
--- a/drivers/net/dsa/microchip/ksz_common.c
+++ b/drivers/net/dsa/microchip/ksz_common.c
@@ -40,6 +40,82 @@ void ksz_update_port_member(struct ksz_device *dev, int port)
 }
 EXPORT_SYMBOL_GPL(ksz_update_port_member);
 
+static void port_r_cnt(struct ksz_device *dev, int port)
+{
+	struct ksz_port_mib *mib = &dev->ports[port].mib;
+	u64 *dropped;
+
+	/* Some ports may not have MIB counters before SWITCH_COUNTER_NUM. */
+	while (mib->cnt_ptr < dev->reg_mib_cnt) {
+		dev->dev_ops->r_mib_cnt(dev, port, mib->cnt_ptr,
+					&mib->counters[mib->cnt_ptr]);
+		++mib->cnt_ptr;
+	}
+
+	/* last one in storage */
+	dropped = &mib->counters[dev->mib_cnt];
+
+	/* Some ports may not have MIB counters after SWITCH_COUNTER_NUM. */
+	while (mib->cnt_ptr < dev->mib_cnt) {
+		dev->dev_ops->r_mib_pkt(dev, port, mib->cnt_ptr,
+					dropped, &mib->counters[mib->cnt_ptr]);
+		++mib->cnt_ptr;
+	}
+	mib->cnt_ptr = 0;
+}
+
+static void ksz_mib_read_work(struct work_struct *work)
+{
+	struct ksz_device *dev =
+		container_of(work, struct ksz_device, mib_read);
+	struct ksz_port *p;
+	struct ksz_port_mib *mib;
+	int i;
+
+	for (i = 0; i < dev->mib_port_cnt; i++) {
+		p = &dev->ports[i];
+		if (!p->on)
+			continue;
+		mib = &p->mib;
+		mutex_lock(&mib->cnt_mutex);
+
+		/* read only dropped counters when link is not up */
+		if (p->link_just_down)
+			p->link_just_down = 0;
+		else if (!p->phydev.link)
+			mib->cnt_ptr = dev->reg_mib_cnt;
+		port_r_cnt(dev, i);
+		mutex_unlock(&mib->cnt_mutex);
+	}
+}
+
+static void mib_monitor(struct timer_list *t)
+{
+	struct ksz_device *dev = from_timer(dev, t, mib_read_timer);
+
+	mod_timer(&dev->mib_read_timer, jiffies + dev->mib_read_interval);
+	schedule_work(&dev->mib_read);
+}
+
+void ksz_init_mib_timer(struct ksz_device *dev)
+{
+	int i;
+
+	/* Read MIB counters every 30 seconds to avoid overflow. */
+	dev->mib_read_interval = msecs_to_jiffies(30000);
+
+	INIT_WORK(&dev->mib_read, ksz_mib_read_work);
+	timer_setup(&dev->mib_read_timer, mib_monitor, 0);
+
+	for (i = 0; i < dev->mib_port_cnt; i++)
+		dev->dev_ops->port_init_cnt(dev, i);
+
+	/* Start the timer 2 seconds later. */
+	dev->mib_read_timer.expires = jiffies + msecs_to_jiffies(2000);
+	add_timer(&dev->mib_read_timer);
+}
+EXPORT_SYMBOL_GPL(ksz_init_mib_timer);
+
 int ksz_phy_read16(struct dsa_switch *ds, int addr, int reg)
 {
 	struct ksz_device *dev = ds->priv;
@@ -88,6 +164,20 @@ int ksz_sset_count(struct dsa_switch *ds, int port, int sset)
 }
 EXPORT_SYMBOL_GPL(ksz_sset_count);
 
+void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf)
+{
+	struct ksz_device *dev = ds->priv;
+	struct ksz_port_mib *mib;
+
+	mib = &dev->ports[port].mib;
+
+	mutex_lock(&mib->cnt_mutex);
+	port_r_cnt(dev, port);
+	memcpy(buf, mib->counters, dev->mib_cnt * sizeof(u64));
+	mutex_unlock(&mib->cnt_mutex);
+}
+EXPORT_SYMBOL_GPL(ksz_get_ethtool_stats);
+
 int ksz_port_bridge_join(struct dsa_switch *ds, int port,
 			 struct net_device *br)
 {
@@ -355,6 +445,12 @@ int ksz_switch_register(struct ksz_device *dev,
 
 void ksz_switch_remove(struct ksz_device *dev)
 {
+	/* timer started */
+	if (dev->mib_read_timer.expires) {
+		del_timer_sync(&dev->mib_read_timer);
+		flush_work(&dev->mib_read);
+	}
+
 	dev->dev_ops->exit(dev);
 	dsa_unregister_switch(dev->ds);
 
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 6d49319..f5f5fd8 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -8,6 +8,7 @@
 #define __KSZ_COMMON_H
 
 void ksz_update_port_member(struct ksz_device *dev, int port);
+void ksz_init_mib_timer(struct ksz_device *dev);
 
 /* Common DSA access functions */
 
@@ -16,6 +17,7 @@
 void ksz_adjust_link(struct dsa_switch *ds, int port,
 		     struct phy_device *phydev);
 int ksz_sset_count(struct dsa_switch *ds, int port, int sset);
+void ksz_get_ethtool_stats(struct dsa_switch *ds, int port, uint64_t *buf);
 int ksz_port_bridge_join(struct dsa_switch *ds, int port,
 			 struct net_device *br);
 void ksz_port_bridge_leave(struct dsa_switch *ds, int port,
diff --git a/drivers/net/dsa/microchip/ksz_priv.h b/drivers/net/dsa/microchip/ksz_priv.h
index 0fdc58b..a35bb34 100644
--- a/drivers/net/dsa/microchip/ksz_priv.h
+++ b/drivers/net/dsa/microchip/ksz_priv.h
@@ -14,8 +14,6 @@
 #include <linux/etherdevice.h>
 #include <net/dsa.h>
 
-#include "ksz9477_reg.h"
-
 struct ksz_io_ops;
 
 struct vlan_table {
@@ -23,6 +21,7 @@ struct vlan_table {
 };
 
 struct ksz_port_mib {
+	struct mutex cnt_mutex;		/* structure access */
 	u8 cnt_ptr;
 	u64 *counters;
 };
@@ -39,6 +38,7 @@ struct ksz_port {
 	u32 sgmii:1;			/* port is SGMII */
 	u32 force:1;
 	u32 link_just_down:1;		/* link just goes down */
+	u32 freeze:1;			/* MIB counter freeze is enabled */
 
 	struct ksz_port_mib mib;
 };
@@ -79,8 +79,6 @@ struct ksz_device {
 
 	struct vlan_table *vlan_cache;
 
-	u64 mib_value[TOTAL_SWITCH_COUNTER_NUM];
-
 	u8 *txbuf;
 
 	struct ksz_port *ports;
@@ -153,6 +151,7 @@ struct ksz_dev_ops {
 			  u64 *cnt);
 	void (*r_mib_pkt)(struct ksz_device *dev, int port, u16 addr,
 			  u64 *dropped, u64 *cnt);
+	void (*freeze_mib)(struct ksz_device *dev, int port, bool freeze);
 	void (*port_init_cnt)(struct ksz_device *dev, int port);
 	int (*shutdown)(struct ksz_device *dev);
 	int (*detect)(struct ksz_device *dev);
-- 
1.9.1


^ permalink raw reply related

* [PATCH v1 net-next 4/4] net: dsa: microchip: remove unnecessary include headers
From: Tristram.Ha @ 2019-02-08  4:07 UTC (permalink / raw)
  To: Sergio Paracuellos, Andrew Lunn, Florian Fainelli, Pavel Machek
  Cc: Tristram Ha, UNGLinuxDriver, netdev
In-Reply-To: <1549598829-25970-1-git-send-email-Tristram.Ha@microchip.com>

From: Tristram Ha <Tristram.Ha@microchip.com>

Remove unnecessary header include lines.

Signed-off-by: Tristram Ha <Tristram.Ha@microchip.com>
---
 drivers/net/dsa/microchip/ksz9477.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/net/dsa/microchip/ksz9477.c b/drivers/net/dsa/microchip/ksz9477.c
index 8391b9e..7c51edd 100644
--- a/drivers/net/dsa/microchip/ksz9477.c
+++ b/drivers/net/dsa/microchip/ksz9477.c
@@ -5,15 +5,11 @@
  * Copyright (C) 2017-2019 Microchip Technology Inc.
  */
 
-#include <linux/delay.h>
-#include <linux/export.h>
-#include <linux/gpio.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/iopoll.h>
 #include <linux/platform_data/microchip-ksz.h>
 #include <linux/phy.h>
-#include <linux/etherdevice.h>
 #include <linux/if_bridge.h>
 #include <net/dsa.h>
 #include <net/switchdev.h>
-- 
1.9.1


^ permalink raw reply related

* [PATCH net-next] ipmr: ip6mr: Create new sockopt to clear mfc cache or vifs
From: Callum Sinclair @ 2019-02-08  4:11 UTC (permalink / raw)
  To: davem, kuznet, yoshfuji, nikolay, netdev, linux-kernel; +Cc: Callum Sinclair
In-Reply-To: <20190208041103.31299-1-callum.sinclair@alliedtelesis.co.nz>

Currently the only way to clear the mfc cache was to delete the entries
one by one using the MRT_DEL_MFC socket option or to destroy and
recreate the socket.

Create a new socket option which will clear the multicast forwarding
cache on the socket without destroying the socket. The new socket option
MRT_FLUSH_ENTRIES will clear all multicast entries on the sockets table
and the MRT_FLUSH_VIFS will delete all multicast vifs on the socket
table.

Signed-off-by: Callum Sinclair <callum.sinclair@alliedtelesis.co.nz>
---
 include/uapi/linux/mroute.h  |  7 +++-
 include/uapi/linux/mroute6.h |  7 +++-
 net/ipv4/ipmr.c              | 69 ++++++++++++++++++++-------------
 net/ipv6/ip6mr.c             | 74 ++++++++++++++++++++++--------------
 4 files changed, 100 insertions(+), 57 deletions(-)

diff --git a/include/uapi/linux/mroute.h b/include/uapi/linux/mroute.h
index 5d37a9ccce63..673495ca3495 100644
--- a/include/uapi/linux/mroute.h
+++ b/include/uapi/linux/mroute.h
@@ -28,12 +28,17 @@
 #define MRT_TABLE	(MRT_BASE+9)	/* Specify mroute table ID		*/
 #define MRT_ADD_MFC_PROXY	(MRT_BASE+10)	/* Add a (*,*|G) mfc entry	*/
 #define MRT_DEL_MFC_PROXY	(MRT_BASE+11)	/* Del a (*,*|G) mfc entry	*/
-#define MRT_MAX		(MRT_BASE+11)
+#define MRT_FLUSH	(MRT_BASE+12)	/* Flush all multicast entries and vifs	*/
+#define MRT_MAX		(MRT_BASE+12)
 
 #define SIOCGETVIFCNT	SIOCPROTOPRIVATE	/* IP protocol privates */
 #define SIOCGETSGCNT	(SIOCPROTOPRIVATE+1)
 #define SIOCGETRPF	(SIOCPROTOPRIVATE+2)
 
+/* MRT_FLUSH optional flags */
+#define MRT_FLUSH_ENTRIES	1	/* For flushing all multicast entries */
+#define MRT_FLUSH_VIFS		2	/* For flushing all multicast vifs */
+
 #define MAXVIFS		32
 typedef unsigned long vifbitmap_t;	/* User mode code depends on this lot */
 typedef unsigned short vifi_t;
diff --git a/include/uapi/linux/mroute6.h b/include/uapi/linux/mroute6.h
index 9999cc006390..1f7ac3f0bc20 100644
--- a/include/uapi/linux/mroute6.h
+++ b/include/uapi/linux/mroute6.h
@@ -31,12 +31,17 @@
 #define MRT6_TABLE	(MRT6_BASE+9)	/* Specify mroute table ID		*/
 #define MRT6_ADD_MFC_PROXY	(MRT6_BASE+10)	/* Add a (*,*|G) mfc entry	*/
 #define MRT6_DEL_MFC_PROXY	(MRT6_BASE+11)	/* Del a (*,*|G) mfc entry	*/
-#define MRT6_MAX	(MRT6_BASE+11)
+#define MRT6_FLUSH	(MRT6_BASE+12)	/* Flush all multicast entries and vifs	*/
+#define MRT6_MAX	(MRT6_BASE+12)
 
 #define SIOCGETMIFCNT_IN6	SIOCPROTOPRIVATE	/* IP protocol privates */
 #define SIOCGETSGCNT_IN6	(SIOCPROTOPRIVATE+1)
 #define SIOCGETRPF	(SIOCPROTOPRIVATE+2)
 
+/* MRT6_FLUSH optional flags */
+#define MRT6_FLUSH_ENTRIES	1	/* For flushing all multicast entries */
+#define MRT6_FLUSH_VIFS		2	/* For flushing all multicast vifs */
+
 #define MAXMIFS		32
 typedef unsigned long mifbitmap_t;	/* User mode code depends on this lot */
 typedef unsigned short mifi_t;
diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index e536970557dd..dc7a4ee1d7ca 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -110,7 +110,7 @@ static int ipmr_cache_report(struct mr_table *mrt,
 static void mroute_netlink_event(struct mr_table *mrt, struct mfc_cache *mfc,
 				 int cmd);
 static void igmpmsg_netlink_event(struct mr_table *mrt, struct sk_buff *pkt);
-static void mroute_clean_tables(struct mr_table *mrt, bool all);
+static void mroute_clean_tables(struct mr_table *mrt, bool all, int flags);
 static void ipmr_expire_process(struct timer_list *t);
 
 #ifdef CONFIG_IP_MROUTE_MULTIPLE_TABLES
@@ -415,7 +415,7 @@ static struct mr_table *ipmr_new_table(struct net *net, u32 id)
 static void ipmr_free_table(struct mr_table *mrt)
 {
 	del_timer_sync(&mrt->ipmr_expire_timer);
-	mroute_clean_tables(mrt, true);
+	mroute_clean_tables(mrt, true, MRT_FLUSH_VIFS | MRT_FLUSH_ENTRIES);
 	rhltable_destroy(&mrt->mfc_hash);
 	kfree(mrt);
 }
@@ -1296,7 +1296,7 @@ static int ipmr_mfc_add(struct net *net, struct mr_table *mrt,
 }
 
 /* Close the multicast socket, and clear the vif tables etc */
-static void mroute_clean_tables(struct mr_table *mrt, bool all)
+static void mroute_clean_tables(struct mr_table *mrt, bool all, int flags)
 {
 	struct net *net = read_pnet(&mrt->net);
 	struct mr_mfc *c, *tmp;
@@ -1305,35 +1305,39 @@ static void mroute_clean_tables(struct mr_table *mrt, bool all)
 	int i;
 
 	/* Shut down all active vif entries */
-	for (i = 0; i < mrt->maxvif; i++) {
-		if (!all && (mrt->vif_table[i].flags & VIFF_STATIC))
-			continue;
-		vif_delete(mrt, i, 0, &list);
+	if (flags & MRT_FLUSH_VIFS) {
+		for (i = 0; i < mrt->maxvif; i++) {
+			if (!all && (mrt->vif_table[i].flags & VIFF_STATIC))
+				continue;
+			vif_delete(mrt, i, 0, &list);
+		}
+		unregister_netdevice_many(&list);
 	}
-	unregister_netdevice_many(&list);
 
 	/* Wipe the cache */
-	list_for_each_entry_safe(c, tmp, &mrt->mfc_cache_list, list) {
-		if (!all && (c->mfc_flags & MFC_STATIC))
-			continue;
-		rhltable_remove(&mrt->mfc_hash, &c->mnode, ipmr_rht_params);
-		list_del_rcu(&c->list);
-		cache = (struct mfc_cache *)c;
-		call_ipmr_mfc_entry_notifiers(net, FIB_EVENT_ENTRY_DEL, cache,
-					      mrt->id);
-		mroute_netlink_event(mrt, cache, RTM_DELROUTE);
-		mr_cache_put(c);
-	}
-
-	if (atomic_read(&mrt->cache_resolve_queue_len) != 0) {
-		spin_lock_bh(&mfc_unres_lock);
-		list_for_each_entry_safe(c, tmp, &mrt->mfc_unres_queue, list) {
-			list_del(&c->list);
+	if (flags & MRT_FLUSH_ENTRIES) {
+		list_for_each_entry_safe(c, tmp, &mrt->mfc_cache_list, list) {
+			if (!all && (c->mfc_flags & MFC_STATIC))
+				continue;
+			rhltable_remove(&mrt->mfc_hash, &c->mnode, ipmr_rht_params);
+			list_del_rcu(&c->list);
 			cache = (struct mfc_cache *)c;
+			call_ipmr_mfc_entry_notifiers(net, FIB_EVENT_ENTRY_DEL, cache,
+										  mrt->id);
 			mroute_netlink_event(mrt, cache, RTM_DELROUTE);
-			ipmr_destroy_unres(mrt, cache);
+			mr_cache_put(c);
+		}
+
+		if (atomic_read(&mrt->cache_resolve_queue_len) != 0) {
+			spin_lock_bh(&mfc_unres_lock);
+			list_for_each_entry_safe(c, tmp, &mrt->mfc_unres_queue, list) {
+				list_del(&c->list);
+				cache = (struct mfc_cache *)c;
+				mroute_netlink_event(mrt, cache, RTM_DELROUTE);
+				ipmr_destroy_unres(mrt, cache);
+			}
+			spin_unlock_bh(&mfc_unres_lock);
 		}
-		spin_unlock_bh(&mfc_unres_lock);
 	}
 }
 
@@ -1354,7 +1358,7 @@ static void mrtsock_destruct(struct sock *sk)
 						    NETCONFA_IFINDEX_ALL,
 						    net->ipv4.devconf_all);
 			RCU_INIT_POINTER(mrt->mroute_sk, NULL);
-			mroute_clean_tables(mrt, false);
+			mroute_clean_tables(mrt, false, MRT_FLUSH_VIFS | MRT_FLUSH_ENTRIES);
 		}
 	}
 	rtnl_unlock();
@@ -1479,6 +1483,17 @@ int ip_mroute_setsockopt(struct sock *sk, int optname, char __user *optval,
 					   sk == rtnl_dereference(mrt->mroute_sk),
 					   parent);
 		break;
+	case MRT_FLUSH:
+		if (optlen != sizeof(val)) {
+			ret = -EINVAL;
+			break;
+		}
+		if (get_user(val, (int __user *)optval)) {
+			ret = -EFAULT;
+			break;
+		}
+		mroute_clean_tables(mrt, true, val);
+		break;
 	/* Control PIM assert. */
 	case MRT_ASSERT:
 		if (optlen != sizeof(val)) {
diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index cc01aa3f2b5e..01f015fb33f0 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -97,7 +97,7 @@ static void mr6_netlink_event(struct mr_table *mrt, struct mfc6_cache *mfc,
 static void mrt6msg_netlink_event(struct mr_table *mrt, struct sk_buff *pkt);
 static int ip6mr_rtm_dumproute(struct sk_buff *skb,
 			       struct netlink_callback *cb);
-static void mroute_clean_tables(struct mr_table *mrt, bool all);
+static void mroute_clean_tables(struct mr_table *mrt, bool all, int flags);
 static void ipmr_expire_process(struct timer_list *t);
 
 #ifdef CONFIG_IPV6_MROUTE_MULTIPLE_TABLES
@@ -393,7 +393,7 @@ static struct mr_table *ip6mr_new_table(struct net *net, u32 id)
 static void ip6mr_free_table(struct mr_table *mrt)
 {
 	del_timer_sync(&mrt->ipmr_expire_timer);
-	mroute_clean_tables(mrt, true);
+	mroute_clean_tables(mrt, true, MRT6_FLUSH_VIFS | MRT6_FLUSH_ENTRIES);
 	rhltable_destroy(&mrt->mfc_hash);
 	kfree(mrt);
 }
@@ -1496,42 +1496,46 @@ static int ip6mr_mfc_add(struct net *net, struct mr_table *mrt,
  *	Close the multicast socket, and clear the vif tables etc
  */
 
-static void mroute_clean_tables(struct mr_table *mrt, bool all)
+static void mroute_clean_tables(struct mr_table *mrt, bool all, int flags)
 {
 	struct mr_mfc *c, *tmp;
 	LIST_HEAD(list);
 	int i;
 
 	/* Shut down all active vif entries */
-	for (i = 0; i < mrt->maxvif; i++) {
-		if (!all && (mrt->vif_table[i].flags & VIFF_STATIC))
-			continue;
-		mif6_delete(mrt, i, 0, &list);
+	if (flags & MRT6_FLUSH_VIFS) {
+		for (i = 0; i < mrt->maxvif; i++) {
+			if (!all && (mrt->vif_table[i].flags & VIFF_STATIC))
+				continue;
+			mif6_delete(mrt, i, 0, &list);
+		}
+		unregister_netdevice_many(&list);
 	}
-	unregister_netdevice_many(&list);
 
 	/* Wipe the cache */
-	list_for_each_entry_safe(c, tmp, &mrt->mfc_cache_list, list) {
-		if (!all && (c->mfc_flags & MFC_STATIC))
-			continue;
-		rhltable_remove(&mrt->mfc_hash, &c->mnode, ip6mr_rht_params);
-		list_del_rcu(&c->list);
-		call_ip6mr_mfc_entry_notifiers(read_pnet(&mrt->net),
-					       FIB_EVENT_ENTRY_DEL,
-					       (struct mfc6_cache *)c, mrt->id);
-		mr6_netlink_event(mrt, (struct mfc6_cache *)c, RTM_DELROUTE);
-		mr_cache_put(c);
-	}
+	if (flags & MRT6_FLUSH_ENTRIES) {
+		list_for_each_entry_safe(c, tmp, &mrt->mfc_cache_list, list) {
+			if (!all && (c->mfc_flags & MFC_STATIC))
+				continue;
+			rhltable_remove(&mrt->mfc_hash, &c->mnode, ip6mr_rht_params);
+			list_del_rcu(&c->list);
+			call_ip6mr_mfc_entry_notifiers(read_pnet(&mrt->net),
+										   FIB_EVENT_ENTRY_DEL,
+										   (struct mfc6_cache *)c, mrt->id);
+			mr6_netlink_event(mrt, (struct mfc6_cache *)c, RTM_DELROUTE);
+			mr_cache_put(c);
+		}
 
-	if (atomic_read(&mrt->cache_resolve_queue_len) != 0) {
-		spin_lock_bh(&mfc_unres_lock);
-		list_for_each_entry_safe(c, tmp, &mrt->mfc_unres_queue, list) {
-			list_del(&c->list);
-			mr6_netlink_event(mrt, (struct mfc6_cache *)c,
-					  RTM_DELROUTE);
-			ip6mr_destroy_unres(mrt, (struct mfc6_cache *)c);
+		if (atomic_read(&mrt->cache_resolve_queue_len) != 0) {
+			spin_lock_bh(&mfc_unres_lock);
+			list_for_each_entry_safe(c, tmp, &mrt->mfc_unres_queue, list) {
+				list_del(&c->list);
+				mr6_netlink_event(mrt, (struct mfc6_cache *)c,
+								  RTM_DELROUTE);
+				ip6mr_destroy_unres(mrt, (struct mfc6_cache *)c);
+			}
+			spin_unlock_bh(&mfc_unres_lock);
 		}
-		spin_unlock_bh(&mfc_unres_lock);
 	}
 }
 
@@ -1587,7 +1591,7 @@ int ip6mr_sk_done(struct sock *sk)
 						     NETCONFA_IFINDEX_ALL,
 						     net->ipv6.devconf_all);
 
-			mroute_clean_tables(mrt, false);
+			mroute_clean_tables(mrt, false, MRT6_FLUSH_VIFS | MRT6_FLUSH_ENTRIES);
 			err = 0;
 			break;
 		}
@@ -1703,6 +1707,20 @@ int ip6_mroute_setsockopt(struct sock *sk, int optname, char __user *optval, uns
 		rtnl_unlock();
 		return ret;
 
+	case MRT6_FLUSH:
+	{
+		int flags;
+
+		if (optlen != sizeof(flags))
+			return -EINVAL;
+		if (get_user(flags, (int __user *)optval))
+			return -EFAULT;
+		rtnl_lock();
+		mroute_clean_tables(mrt, true, flags);
+		rtnl_unlock();
+		return 0;
+	}
+
 	/*
 	 *	Control PIM assert (to activate pim will activate assert)
 	 */
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next] ipmr: ip6mr: Create new sockopt to clear mfc cache or vifs
From: Callum Sinclair @ 2019-02-08  4:11 UTC (permalink / raw)
  To: davem, kuznet, yoshfuji, nikolay, netdev, linux-kernel; +Cc: Callum Sinclair

Created a way to clear the multicast forwarding cache on a socket
without having to either remove the entries manually using the delete
entry socket option or destroy and recreate the multicast socket.

Using the flags MRT_FLUSH_ENTRIES and MRT_FLUSH_VIFS, all multicast
entries can be cleared, all multicast interfaces can be closed or both
can be cleared using one sockopt call.

Callum Sinclair (1):
  ipmr: ip6mr: Create new sockopt to clear mfc cache or vifs

 include/uapi/linux/mroute.h  |  7 +++-
 include/uapi/linux/mroute6.h |  7 +++-
 net/ipv4/ipmr.c              | 69 ++++++++++++++++++++-------------
 net/ipv6/ip6mr.c             | 74 ++++++++++++++++++++++--------------
 4 files changed, 100 insertions(+), 57 deletions(-)

-- 
2.20.1


^ permalink raw reply

* [PATCH net-next] igb: use struct_size() helper
From: Gustavo A. R. Silva @ 2019-02-08  4:15 UTC (permalink / raw)
  To: Jeff Kirsher, David S. Miller
  Cc: intel-wired-lan, netdev, linux-kernel, Gustavo A. R. Silva

One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:

struct foo {
    int stuff;
    struct boo entry[];
};

size = sizeof(struct foo) + count * sizeof(struct boo);
instance = alloc(size, GFP_KERNEL);

Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:

size = struct_size(instance, entry, count);

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 6d812e96572d..69b230c53fed 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -1189,15 +1189,15 @@ static int igb_alloc_q_vector(struct igb_adapter *adapter,
 {
 	struct igb_q_vector *q_vector;
 	struct igb_ring *ring;
-	int ring_count, size;
+	int ring_count;
+	size_t size;
 
 	/* igb only supports 1 Tx and/or 1 Rx queue per vector */
 	if (txr_count > 1 || rxr_count > 1)
 		return -ENOMEM;
 
 	ring_count = txr_count + rxr_count;
-	size = sizeof(struct igb_q_vector) +
-	       (sizeof(struct igb_ring) * ring_count);
+	size = struct_size(q_vector, ring, ring_count);
 
 	/* allocate q_vector and rings */
 	q_vector = adapter->q_vector[v_idx];
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next] igc: Use struct_size() helper
From: Gustavo A. R. Silva @ 2019-02-08  4:19 UTC (permalink / raw)
  To: Jeff Kirsher, David S. Miller
  Cc: intel-wired-lan, netdev, linux-kernel, Gustavo A. R. Silva

One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:

struct foo {
    int stuff;
    struct boo entry[];
};

size = sizeof(struct foo) + count * sizeof(struct boo);
instance = kzalloc(size, GFP_KERNEL)

Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:

instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL)

Notice that, in this case, variable size is not necessary, hence
it is removed.

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/net/ethernet/intel/igc/igc_main.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 11c75433fe22..87a11879bf2d 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -2958,22 +2958,21 @@ static int igc_alloc_q_vector(struct igc_adapter *adapter,
 {
 	struct igc_q_vector *q_vector;
 	struct igc_ring *ring;
-	int ring_count, size;
+	int ring_count;
 
 	/* igc only supports 1 Tx and/or 1 Rx queue per vector */
 	if (txr_count > 1 || rxr_count > 1)
 		return -ENOMEM;
 
 	ring_count = txr_count + rxr_count;
-	size = sizeof(struct igc_q_vector) +
-		(sizeof(struct igc_ring) * ring_count);
 
 	/* allocate q_vector and rings */
 	q_vector = adapter->q_vector[v_idx];
 	if (!q_vector)
-		q_vector = kzalloc(size, GFP_KERNEL);
+		q_vector = kzalloc(struct_size(q_vector, ring, ring_count),
+				   GFP_KERNEL);
 	else
-		memset(q_vector, 0, size);
+		memset(q_vector, 0, struct_size(q_vector, ring, ring_count));
 	if (!q_vector)
 		return -ENOMEM;
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next] ixgbe: Use struct_size() helper
From: Gustavo A. R. Silva @ 2019-02-08  4:22 UTC (permalink / raw)
  To: Jeff Kirsher, David S. Miller
  Cc: intel-wired-lan, netdev, linux-kernel, Gustavo A. R. Silva

One of the more common cases of allocation size calculations is finding
the size of a structure that has a zero-sized array at the end, along
with memory for some number of elements for that array. For example:

struct foo {
    int stuff;
    struct boo entry[];
};

size = sizeof(struct foo) + count * sizeof(struct boo);
instance = kzalloc(size, GFP_KERNEL);

Instead of leaving these open-coded and prone to type mistakes, we can
now use the new struct_size() helper:

instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL);

Notice that, in this case, variable size is not necessary, hence
it is removed.

This code was detected with the help of Coccinelle.

Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
index 62e6499e4146..cc3196ae5aea 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_lib.c
@@ -836,12 +836,10 @@ static int ixgbe_alloc_q_vector(struct ixgbe_adapter *adapter,
 	struct ixgbe_ring *ring;
 	int node = NUMA_NO_NODE;
 	int cpu = -1;
-	int ring_count, size;
+	int ring_count;
 	u8 tcs = adapter->hw_tcs;
 
 	ring_count = txr_count + rxr_count + xdp_count;
-	size = sizeof(struct ixgbe_q_vector) +
-	       (sizeof(struct ixgbe_ring) * ring_count);
 
 	/* customize cpu for Flow Director mapping */
 	if ((tcs <= 1) && !(adapter->flags & IXGBE_FLAG_SRIOV_ENABLED)) {
@@ -855,9 +853,11 @@ static int ixgbe_alloc_q_vector(struct ixgbe_adapter *adapter,
 	}
 
 	/* allocate q_vector and rings */
-	q_vector = kzalloc_node(size, GFP_KERNEL, node);
+	q_vector = kzalloc_node(struct_size(q_vector, ring, ring_count),
+				GFP_KERNEL, node);
 	if (!q_vector)
-		q_vector = kzalloc(size, GFP_KERNEL);
+		q_vector = kzalloc(struct_size(q_vector, ring, ring_count),
+				   GFP_KERNEL);
 	if (!q_vector)
 		return -ENOMEM;
 
-- 
2.20.1


^ permalink raw reply related

* [PATCH net-next] ethtool: Remove unnecessary null check in ethtool_rx_flow_rule_create
From: Nathan Chancellor @ 2019-02-08  4:46 UTC (permalink / raw)
  To: David S. Miller
  Cc: netdev, linux-kernel, Pablo Neira Ayuso, Jiri Pirko,
	Nick Desaulniers, Nathan Chancellor

net/core/ethtool.c:3023:19: warning: address of array
'ext_m_spec->h_dest' will always evaluate to 'true'
[-Wpointer-bool-conversion]
                if (ext_m_spec->h_dest) {
                ~~  ~~~~~~~~~~~~^~~~~~

h_dest is an array, it can't be null so remove this check.

Fixes: eca4205f9ec3 ("ethtool: add ethtool_rx_flow_spec to flow_rule structure translator")
Link: https://github.com/ClangBuiltLinux/linux/issues/353
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
 net/core/ethtool.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 0fbf39239b29..d2c47cdf25da 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -3020,17 +3020,15 @@ ethtool_rx_flow_rule_create(const struct ethtool_rx_flow_spec_input *input)
 		const struct ethtool_flow_ext *ext_h_spec = &fs->h_ext;
 		const struct ethtool_flow_ext *ext_m_spec = &fs->m_ext;
 
-		if (ext_m_spec->h_dest) {
-			memcpy(match->key.eth_addrs.dst, ext_h_spec->h_dest,
-			       ETH_ALEN);
-			memcpy(match->mask.eth_addrs.dst, ext_m_spec->h_dest,
-			       ETH_ALEN);
-
-			match->dissector.used_keys |=
-				BIT(FLOW_DISSECTOR_KEY_ETH_ADDRS);
-			match->dissector.offset[FLOW_DISSECTOR_KEY_ETH_ADDRS] =
-				offsetof(struct ethtool_rx_flow_key, eth_addrs);
-		}
+		memcpy(match->key.eth_addrs.dst, ext_h_spec->h_dest,
+		       ETH_ALEN);
+		memcpy(match->mask.eth_addrs.dst, ext_m_spec->h_dest,
+		       ETH_ALEN);
+
+		match->dissector.used_keys |=
+			BIT(FLOW_DISSECTOR_KEY_ETH_ADDRS);
+		match->dissector.offset[FLOW_DISSECTOR_KEY_ETH_ADDRS] =
+			offsetof(struct ethtool_rx_flow_key, eth_addrs);
 	}
 
 	act = &flow->rule->action.entries[0];
-- 
2.20.1


^ permalink raw reply related

* Clang warning in drivers/net/ethernet/intel/igc/igc_ethtool.c
From: Nathan Chancellor @ 2019-02-08  5:09 UTC (permalink / raw)
  To: Sasha Neftin, Jeff Kirsher
  Cc: Aaron Brown, intel-wired-lan, netdev, linux-kernel,
	Nick Desaulniers

Hi all,

After commit 8c5ad0dae93c ("igc: Add ethtool support"), Clang warns:

drivers/net/ethernet/intel/igc/igc_ethtool.c:9:19: warning: variable 'igc_priv_flags_strings' is not needed and will not be emitted [-Wunneeded-internal-declaration]
static const char igc_priv_flags_strings[][ETH_GSTRING_LEN] = {
                  ^
1 warning generated.

igc_priv_flags_strings is only used in an ARRAY_SIZE macro, which is a
compile time evaluation, so no reference to it is being emitted in the
final assembly. Is it actually needed and was forgotten to be used
somewhere or could it be eliminated so that Clang no longer warns?

Thanks,
Nathan

^ permalink raw reply

* Re: [PATCH bpf-next 1/6] bpf: Add a bpf_sock pointer to __sk_buff and a bpf_sk_fullsock helper
From: Martin Lau @ 2019-02-08  5:56 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: netdev@vger.kernel.org, Alexei Starovoitov, Kernel Team,
	Lawrence Brakmo
In-Reply-To: <58d44fb9-a10b-d6dd-6313-60f7802e4519@iogearbox.net>

On Thu, Feb 07, 2019 at 11:21:41PM +0100, Daniel Borkmann wrote:
> On 02/07/2019 08:27 AM, Martin Lau wrote:
> [...]
> > Following up the discussion in the iovisor conf call.
> > 
> > One of discussion was about:
> > other than tw, can __sk_buff->sk always return a
> > fullsock (PTR_TO_SOCKET_OR_NULL).  In request_sock case,
> > it is doable because it can trace back to the listener sock.
> > 
> > However, that will go back to the sock_common accessing question.
> > In particular, how to access the sock_common's fields of the
> > request_sock itself?  Those fields in the request_sock are different
> > from its listener sock.  e.g. the skc_daddr and skc_dport.
> > 
> > Also, if the sock_common fields of tw is needed, it will become weird
> > because likely a new "struct bpf_tw_sock" is needed which is OK
> > but all sock_common fields need to be copied from bpf_sock
> > to bpf_tw_sock.
> > 
> > I think reading a sk from a ctx should return the
> > most basic type PTR_TO_SOCK_COMMON_OR_NULL (unless the running
> > ctx can guarantee that it always has a fullsock).
> > Currently, it is __sk_buff->sk.  Later, sock_ops->sk...etc.
> > One single 'struct bpf_sock' and limit fullsock field access
> > at verification time.  The bpf_prog then moves down the chain
> > based on what it needs.  It could be fullsock, tcp_sock...etc.
> > 
> > I think that will be the most flexible way to write bpf_prog
> > while also avoid having duplicate fields in different
> > bpf struct in uapi.
> 
> Ok, thanks for following up and sorry for late reply, lets go with
> sock_common then. What's the plan to moving forward with accessing
> full sk in case of req sk? Separate helper or backed into the newly
> added bpf_sk_fullsock() one? Presumably latter?
I will add sk_to_full_sk() to bpf_sk_fullsock() and bpf_tcp_sock().

Thanks,
Martin

^ permalink raw reply

* Re: [PATCH v2 3/6] ethtool: introduce new ioctl for per-queue settings
From: Michal Kubecek @ 2019-02-08  7:02 UTC (permalink / raw)
  To: netdev
  Cc: Nunley, Nicholas D, Kirsher, Jeffrey T, linville@tuxdriver.com,
	nhorman@redhat.com, sassmann@redhat.com
In-Reply-To: <146CDE3B8C383A4B88452B498CB0FBBBFD9C3F87@ORSMSX157.amr.corp.intel.com>

On Thu, Feb 07, 2019 at 11:37:19PM +0000, Nunley, Nicholas D wrote:
> From: Michal Kubecek [mailto:mkubecek@suse.cz]
> > On Tue, Feb 05, 2019 at 04:01:03PM -0800, Jeff Kirsher wrote:
> > > Introduce a new ioctl for setting per-queue parameters.
> > > Users can apply commands to specific queues by setting SUB_COMMAND
> > and
> > > queue_mask with the following ethtool command:
> > >
> > >  ethtool --set-perqueue-command DEVNAME [queue_mask %x]
> > SUB_COMMAND
> > 
> > The "set" part is IMHO a bit confusing in combination with "show" type
> > subcommands.
> 
> I'm not a fan of the "set" either. This patch set had already gone
> through some review before it was passed on to me, and the command
> naming wasn't a previous point of contention and I didn't want to
> disturb the parts that weren't "broken", but since you've brought it
> up I agree that this may not be the best name. I think
> "--perqueue-command" is more appropriate. Does this seem reasonable to
> you?

That sounds good, I would say either that or "--per-queue".

Michal Kubecek

^ permalink raw reply

* Re: Kernel 5.0-rc5 regression with NAT, bisected to: netfilter: nat: remove l4proto->manip_pkt
From: Florian Westphal @ 2019-02-08  7:07 UTC (permalink / raw)
  To: Sander Eikelenboom
  Cc: Florian Westphal, Pablo Neira Ayuso, David S. Miller, netdev,
	linux-kernel
In-Reply-To: <40b70892-daf5-28d7-28b5-869911faf2bb@eikelenboom.it>

Sander Eikelenboom <linux@eikelenboom.it> wrote:
> L.S.,
> 
> While trying out a 5.0-RC5 kernel I seem to have stumbled over a regression with NAT.
> (using an nftables firewall with NAT and connection tracking).
> 
> Unfortunately it isn't too obvious since no errors are logged, but on clients it
> causes symptoms like firefox intermittently not being able to load pages with:
>     Network Protocol Error
>     An error occurred during a connection to www.example.com
>     The page you are trying to view cannot be shown because an error in the network protocol was detected.
>     Please contact the website owners to inform them of this problem.
> 
> But it's only intermittently, so i can still visit some webpages with clients, 
> could be that packet size and or fragments are at play ?
> 
> So I tried testing with git://git.kernel.org/pub/scm/linux/kernel/git/davem/net.git with 
> e8c32c32b48c2e889704d8ca0872f92eb027838e as last commit, to be sure to have the latest netdev has to offer,
> but to no avail. 
> 
> After that I tried to git bisect and ended up with:
> 
> faec18dbb0405c7d4dda025054511dc3a6696918 is the first bad commit
> commit faec18dbb0405c7d4dda025054511dc3a6696918
> Author: Florian Westphal <fw@strlen.de>
> Date:   Thu Dec 13 16:01:33 2018 +0100
> 
>     netfilter: nat: remove l4proto->manip_pkt

Thanks, this is immensely helpful.

I think I see the bug, we can't use target->dst.protonum in
nf_nat_l4proto_manip_pkt(), it will be TCP in case we're dealing
with a related icmp packet.

I will send a patch in a few hours when I get back.

^ permalink raw reply

* Re: [PATCH v2 4/6] ethtool: support per-queue sub command --show-coalesce
From: Michal Kubecek @ 2019-02-08  7:10 UTC (permalink / raw)
  To: netdev
  Cc: Nunley, Nicholas D, Kirsher, Jeffrey T, linville@tuxdriver.com,
	nhorman@redhat.com, sassmann@redhat.com
In-Reply-To: <146CDE3B8C383A4B88452B498CB0FBBBFD9C3FE9@ORSMSX157.amr.corp.intel.com>

On Thu, Feb 07, 2019 at 11:53:40PM +0000, Nunley, Nicholas D wrote:
> > > @@ -5390,7 +5438,19 @@ static int do_perqueue(struct cmd_context *ctx)
> > >  	if (i < 0)
> > >  		exit_bad_args();
> > >
> > > -	/* no sub_command support yet */
> > > +	if (strstr(args[i].opts, "--show-coalesce") != NULL) {
> > 
> > Comparing args[i].func to do_gcoalesce might be easier.
> 
> This is the one comment where I think it's better to leave the code as it is.
> To me is seems more confusing to match on a function pointer that we're never
> going to call. Unless there are more objections I'd rather keep it the way it
> is.

No problem. This is not a code where performance is crucial. In theory,
you could get into trouble if someone introduces another command
(allowing per queue settings) with name like "--show-coalesce-foo" but
that's not very likely, IMHO.

Michal Kubecek

^ permalink raw reply

* [PATCH net-next] net: phy: disregard "Clause 22 registers present" bit in get_phy_c45_devs_in_pkg
From: Heiner Kallweit @ 2019-02-08  7:12 UTC (permalink / raw)
  To: Andrew Lunn, Florian Fainelli, David Miller; +Cc: netdev@vger.kernel.org

Bit 0 in register 1.5 doesn't represent a device but is a flag that
Clause 22 registers are present. Therefore disregard this bit when
populating the device list. If code needs this information it
should read register 1.5 directly instead of accessing the device
list. Because this bit doesn't represent a device I didn't add a
MDIO_MMD_C22PRESENT constant or similar.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/net/phy/phy_device.c | 3 ++-
 include/uapi/linux/mdio.h    | 1 +
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 9369e1323..c2316a117 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -682,7 +682,8 @@ static int get_phy_c45_devs_in_pkg(struct mii_bus *bus, int addr, int dev_addr,
 	phy_reg = mdiobus_read(bus, addr, reg_addr);
 	if (phy_reg < 0)
 		return -EIO;
-	*devices_in_package |= (phy_reg & 0xffff);
+	/* Bit 0 doesn't represent a device, it indicates c22 regs presence */
+	*devices_in_package |= (phy_reg & 0xfffe);
 
 	return 0;
 }
diff --git a/include/uapi/linux/mdio.h b/include/uapi/linux/mdio.h
index 2e6e309f0..0e012b168 100644
--- a/include/uapi/linux/mdio.h
+++ b/include/uapi/linux/mdio.h
@@ -115,6 +115,7 @@
 
 /* Device present registers. */
 #define MDIO_DEVS_PRESENT(devad)	(1 << (devad))
+#define MDIO_DEVS_C22PRESENT		MDIO_DEVS_PRESENT(0)
 #define MDIO_DEVS_PMAPMD		MDIO_DEVS_PRESENT(MDIO_MMD_PMAPMD)
 #define MDIO_DEVS_WIS			MDIO_DEVS_PRESENT(MDIO_MMD_WIS)
 #define MDIO_DEVS_PCS			MDIO_DEVS_PRESENT(MDIO_MMD_PCS)
-- 
2.20.1


^ permalink raw reply related

* Re: [PATCH net-next] mlxsw: spectrum_router: Use struct_size() in kzalloc()
From: Jiri Pirko @ 2019-02-08  7:06 UTC (permalink / raw)
  To: Gustavo A. R. Silva
  Cc: Jiri Pirko, Ido Schimmel, David S. Miller, netdev, linux-kernel
In-Reply-To: <20190208034241.GA11858@embeddedor>

Fri, Feb 08, 2019 at 04:42:41AM CET, gustavo@embeddedor.com wrote:
>One of the more common cases of allocation size calculations is finding
>the size of a structure that has a zero-sized array at the end, along
>with memory for some number of elements for that array. For example:
>
>struct foo {
>    int stuff;
>    struct boo entry[];
>};
>
>size = sizeof(struct foo) + count * sizeof(struct boo);
>instance = kzalloc(size, GFP_KERNEL)
>
>Instead of leaving these open-coded and prone to type mistakes, we can
>now use the new struct_size() helper:
>
>instance = kzalloc(struct_size(instance, entry, count), GFP_KERNEL)
>
>Notice that, in this case, variable alloc_size is not necessary, hence
>it is removed.
>
>This code was detected with the help of Coccinelle.
>
>Signed-off-by: Gustavo A. R. Silva <gustavo@embeddedor.com>

Acked-by: Jiri Pirko <jiri@mellanox.com>

^ permalink raw reply

* Re: [PATCH net-next 1/2] mlxsw: spectrum_router: Offload blackhole routes
From: Ido Schimmel @ 2019-02-08  7:34 UTC (permalink / raw)
  To: David Ahern
  Cc: netdev@vger.kernel.org, davem@davemloft.net, Jiri Pirko,
	Alexander Petrovskiy, mlxsw
In-Reply-To: <18da8b4d-5966-e851-46d6-e389e40b865e@gmail.com>

On Thu, Feb 07, 2019 at 04:40:11PM -0800, David Ahern wrote:
> On 2/6/19 11:42 AM, Ido Schimmel wrote:
> > Create a new FIB entry type for blackhole routes and set it in case the
> > type of the notified route is 'RTN_BLACKHOLE'.
> > 
> > Program such routes with a discard action and mark them as offloaded
> > since the device is dropping the packets instead of the kernel.
> > 
> > Signed-off-by: Ido Schimmel <idosch@mellanox.com>
> > Acked-by: Jiri Pirko <jiri@mellanox.com>
> > ---
> >  .../ethernet/mellanox/mlxsw/spectrum_router.c | 27 +++++++++++++++++--
> >  1 file changed, 25 insertions(+), 2 deletions(-)
> > 
> 
> One of the feature requests from the FRR team (and a feature I have
> implemented) is a blackhole nexthop. The idea is that prefixes are
> installed pointing to nexthop id N. That nexthop definition can be
> atomically updated to go between a device / gateway and a blackhole.
> 
> 
>  [ prefix ] --> [ nhid 1 ] --> [ dev1 / gateway1 ]
> 
> 
>  [ prefix ] --> [ nhid 1 ] --> [ blackhole ]
> 
> 
>  [ prefix ] --> [ nhid 1 ] --> [ dev2 / gateway2 ]
> 
> Do you see this working ok with mlxsw without having to update the
> prefix entries (which can be numerous) directly?

Yes. This patch configures the route itself to drop packets, but we can
instead configure it as a remote route and configure the adjacency entry
to drop packets.

If you later want to change X routes using this blackhole nexthop to a
different one, then create the new one and tell the hardware to do the
switch in a single operation. It will basically grep over all configured
routes and do:

s/blackhole_adjacency_index/new_adjacency_index/
s/black_ecmp_size/new_ecmp_size/

See RALEU in drivers/net/ethernet/mellanox/mlxsw/reg.h

I assume that user can't put blackhole and normal nexthops in the same
group?

^ permalink raw reply

* Re: [PATCH] net: stmmac: Variable "val" in function sun8i_dwmac_set_syscon() could be uninitialized
From: Maxime Ripard @ 2019-02-08  7:44 UTC (permalink / raw)
  To: Yizhuo
  Cc: csong, zhiyunq, Giuseppe Cavallaro, Alexandre Torgue,
	Chen-Yu Tsai, netdev, linux-arm-kernel, linux-kernel
In-Reply-To: <20190207174623.16712-1-yzhai003@ucr.edu>

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

On Thu, Feb 07, 2019 at 09:46:23AM -0800, Yizhuo wrote:
> In function sun8i_dwmac_set_syscon(), local variable "val" could
> be uninitialized if function regmap_read() returns -EINVAL.
> However, it will be used directly in the if statement, which
> is potentially unsafe.
> 
> Signed-off-by: Yizhuo <yzhai003@ucr.edu>

Acked-by: Maxime Ripard <maxime.ripard@bootlin.com>

Thanks!
Maxime

-- 
Maxime Ripard, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ 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