Netdev List
 help / color / mirror / Atom feed
* [PATCH -next] net: netcp: drop kfree for memory allocated with devm_kzalloc
From: Wei Yongjun @ 2016-10-22 12:32 UTC (permalink / raw)
  To: Wingman Kwok, Murali Karicheri; +Cc: Wei Yongjun, netdev

From: Wei Yongjun <weiyongjun1@huawei.com>

It's not necessary to free memory allocated with devm_kzalloc in the
remove path and using kfree leads to a double free.

Fixes: 84640e27f230 ("net: netcp: Add Keystone NetCP core ethernet
driver")
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/net/ethernet/ti/netcp_core.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
index 11609d5..2fb5b6d 100644
--- a/drivers/net/ethernet/ti/netcp_core.c
+++ b/drivers/net/ethernet/ti/netcp_core.c
@@ -2058,7 +2058,6 @@ static void netcp_delete_interface(struct netcp_device *netcp_device,
 		if (module->release)
 			module->release(intf_modpriv->module_priv);
 		list_del(&intf_modpriv->intf_list);
-		kfree(intf_modpriv);
 	}
 	WARN(!list_empty(&netcp->module_head), "%s interface module list is not empty!\n",
 	     ndev->name);
@@ -2153,7 +2152,6 @@ static int netcp_remove(struct platform_device *pdev)
 		dev_dbg(&pdev->dev, "Removing module \"%s\"\n", module->name);
 		module->remove(netcp_device, inst_modpriv->module_priv);
 		list_del(&inst_modpriv->inst_list);
-		kfree(inst_modpriv);
 	}
 
 	/* now that all modules are removed, clean up the interfaces */

^ permalink raw reply related

* Get A Part Time Job Today
From: Steven Moore @ 2016-10-22 11:54 UTC (permalink / raw)
  To: Recipients

Hello
 
 
My Name is Steven Moore.We seek the service of a Secret Shopper in our apex-consult Organization. The assignment will pay  per duty .You can make $200 everyday , depends on how fast you able to take up your work. I am sure you cannot spend more than 1-2 hours per day on your assignment . No Money or Fee is required to become secret shopper.
 
 
 
KINDLY SEND YOUR INFORMATION BELOW IF INTERESTED:
 
 
 
Your Name:
Street Address: ( Not PO. BOX )
Apartment Number:
City: State:
Zip Code:  
Country:
Cell/Home phone number:
Age:
Present working Status:
 
You can also vist our website to Regrister online:
 
 
http://www.apex-consult.net/
 
 
Regards,
Steven Moore
231101xyz@gmail.com
 

^ permalink raw reply

* [PATCH -next] net: dsa: mv88e6xxx: use setup_timer to simplify the code
From: Wei Yongjun @ 2016-10-22 14:28 UTC (permalink / raw)
  To: Andrew Lunn, Vivien Didelot, Florian Fainelli; +Cc: Wei Yongjun, netdev

From: Wei Yongjun <weiyongjun1@huawei.com>

Use setup_timer function instead of initializing timer with the function
and data fields.

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/net/dsa/mv88e6xxx/chip.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index ac03297..52d29b8 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -639,9 +639,8 @@ static void mv88e6xxx_ppu_state_init(struct mv88e6xxx_chip *chip)
 {
 	mutex_init(&chip->ppu_mutex);
 	INIT_WORK(&chip->ppu_work, mv88e6xxx_ppu_reenable_work);
-	init_timer(&chip->ppu_timer);
-	chip->ppu_timer.data = (unsigned long)chip;
-	chip->ppu_timer.function = mv88e6xxx_ppu_reenable_timer;
+	setup_timer(&chip->ppu_timer, mv88e6xxx_ppu_reenable_timer,
+		    (unsigned long)chip);
 }
 
 static void mv88e6xxx_ppu_state_destroy(struct mv88e6xxx_chip *chip)

^ permalink raw reply related

* [PATCH -next] net: eth: altera: Fix error return code in altera_tse_probe()
From: Wei Yongjun @ 2016-10-22 14:28 UTC (permalink / raw)
  To: Vince Bridgers; +Cc: Wei Yongjun, netdev, nios2-dev

From: Wei Yongjun <weiyongjun1@huawei.com>

Fix to return error code -EINVAL from the error handling
case instead of 0, as done elsewhere in this function.

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/net/ethernet/altera/altera_tse_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index cc9787e..8e92084 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -1329,11 +1329,13 @@ static int altera_tse_probe(struct platform_device *pdev)
 		if (upper_32_bits(priv->rxdescmem_busaddr)) {
 			dev_dbg(priv->device,
 				"SGDMA bus addresses greater than 32-bits\n");
+			ret = -EINVAL;
 			goto err_free_netdev;
 		}
 		if (upper_32_bits(priv->txdescmem_busaddr)) {
 			dev_dbg(priv->device,
 				"SGDMA bus addresses greater than 32-bits\n");
+			ret = -EINVAL;
 			goto err_free_netdev;
 		}
 	} else if (priv->dmaops &&

^ permalink raw reply related

* [PATCH -next] rocker: fix error return code in rocker_world_check_init()
From: Wei Yongjun @ 2016-10-22 14:31 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: Wei Yongjun, netdev

From: Wei Yongjun <weiyongjun1@huawei.com>

Fix to return error code -EINVAL from the error handling
case instead of 0, as done elsewhere in this function.

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/net/ethernet/rocker/rocker_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/rocker/rocker_main.c b/drivers/net/ethernet/rocker/rocker_main.c
index 2e81b70..0f3265b 100644
--- a/drivers/net/ethernet/rocker/rocker_main.c
+++ b/drivers/net/ethernet/rocker/rocker_main.c
@@ -1471,7 +1471,7 @@ static int rocker_world_check_init(struct rocker_port *rocker_port)
 	if (rocker->wops) {
 		if (rocker->wops->mode != mode) {
 			dev_err(&rocker->pdev->dev, "hardware has ports in different worlds, which is not supported\n");
-			return err;
+			return -EINVAL;
 		}
 		return 0;
 	}

^ permalink raw reply related

* [PATCH -next] net: ns83820: use dev_kfree_skb_irq instead of kfree_skb
From: Wei Yongjun @ 2016-10-22 14:34 UTC (permalink / raw)
  To: Javier Martinez Canillas, Jarod Wilson; +Cc: Wei Yongjun, netdev

From: Wei Yongjun <weiyongjun1@huawei.com>

It is not allowed to call kfree_skb() from hardware interrupt
context or with interrupts being disabled, spin_lock_irqsave()
make sure always in irq disable context. So the kfree_skb()
should be replaced with dev_kfree_skb_irq().

This is detected by Coccinelle semantic patch.

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/net/ethernet/natsemi/ns83820.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/natsemi/ns83820.c b/drivers/net/ethernet/natsemi/ns83820.c
index a34631e..93c4bdc 100644
--- a/drivers/net/ethernet/natsemi/ns83820.c
+++ b/drivers/net/ethernet/natsemi/ns83820.c
@@ -919,7 +919,7 @@ static void rx_irq(struct net_device *ndev)
 				ndev->stats.rx_dropped++;
 			}
 		} else {
-			kfree_skb(skb);
+			dev_kfree_skb_irq(skb);
 		}
 
 		nr++;

^ permalink raw reply related

* [PATCH -next] amd-xgbe: Fix error return code in xgbe_probe()
From: Wei Yongjun @ 2016-10-22 14:35 UTC (permalink / raw)
  To: Tom Lendacky; +Cc: Wei Yongjun, netdev

From: Wei Yongjun <weiyongjun1@huawei.com>

Fix to return error code -ENODEV from the DMA is not supported error
handling case instead of 0, as done elsewhere in this function.

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/net/ethernet/amd/xgbe/xgbe-main.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-main.c b/drivers/net/ethernet/amd/xgbe/xgbe-main.c
index 667e120..6997f11 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-main.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-main.c
@@ -613,6 +613,7 @@ static int xgbe_probe(struct platform_device *pdev)
 	attr = device_get_dma_attr(dev);
 	if (attr == DEV_DMA_NOT_SUPPORTED) {
 		dev_err(dev, "DMA is not supported");
+		ret = -ENODEV;
 		goto err_io;
 	}
 	pdata->coherent = (attr == DEV_DMA_COHERENT);

^ permalink raw reply related

* [PATCH -next] net: ena: use setup_timer() and mod_timer()
From: Wei Yongjun @ 2016-10-22 14:36 UTC (permalink / raw)
  To: Netanel Belgazal, Saeed Bishara, Zorik Machulsky, David S. Miller,
	Wei Yongjun, Jarod Wilson, Rami Rosen
  Cc: Wei Yongjun, netdev

From: Wei Yongjun <weiyongjun1@huawei.com>

Use setup_timer() instead of init_timer(), being the preferred/standard
way to set a timer up.

Also, quoting the mod_timer() function comment:
-> mod_timer() is a more efficient way to update the expire field of an
   active timer (if the timer is inactive it will be activated).

Use setup_timer and mod_timer to setup and arm a timer, to make the code
cleaner and easier to read.

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/net/ethernet/amazon/ena/ena_netdev.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/amazon/ena/ena_netdev.c b/drivers/net/ethernet/amazon/ena/ena_netdev.c
index 2a55ab0..cc8b13e 100644
--- a/drivers/net/ethernet/amazon/ena/ena_netdev.c
+++ b/drivers/net/ethernet/amazon/ena/ena_netdev.c
@@ -3013,12 +3013,9 @@ static int ena_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	adapter->last_keep_alive_jiffies = jiffies;
 
-	init_timer(&adapter->timer_service);
-	adapter->timer_service.expires = round_jiffies(jiffies + HZ);
-	adapter->timer_service.function = ena_timer_service;
-	adapter->timer_service.data = (unsigned long)adapter;
-
-	add_timer(&adapter->timer_service);
+	setup_timer(&adapter->timer_service, ena_timer_service,
+		    (unsigned long)adapter);
+	mod_timer(&adapter->timer_service, round_jiffies(jiffies + HZ));
 
 	dev_info(&pdev->dev, "%s found at mem %lx, mac addr %pM Queues %d\n",
 		 DEVICE_NAME, (long)pci_resource_start(pdev, 0),

^ permalink raw reply related

* [PATCH -next] net: netcp: add missing of_node_put() in netcp_probe()
From: Wei Yongjun @ 2016-10-22 14:40 UTC (permalink / raw)
  To: Wingman Kwok, Murali Karicheri; +Cc: Wei Yongjun, netdev

From: Wei Yongjun <weiyongjun1@huawei.com>

This node pointer is returned by of_get_child_by_name() with refcount
incremented in this function. of_node_put() on it before exitting this
function.

This is detected by Coccinelle semantic patch.

Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
 drivers/net/ethernet/ti/netcp_core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/ethernet/ti/netcp_core.c b/drivers/net/ethernet/ti/netcp_core.c
index 11609d5..eaa68bc 100644
--- a/drivers/net/ethernet/ti/netcp_core.c
+++ b/drivers/net/ethernet/ti/netcp_core.c
@@ -2121,6 +2121,8 @@ static int netcp_probe(struct platform_device *pdev)
 		}
 	}
 
+	of_node_put(interfaces);
+
 	/* Add the device instance to the list */
 	list_add_tail(&netcp_device->device_list, &netcp_devices);
 
@@ -2133,6 +2135,8 @@ static int netcp_probe(struct platform_device *pdev)
 		netcp_delete_interface(netcp_device, netcp_intf->ndev);
 	}
 
+	of_node_put(interfaces);
+
 probe_quit:
 	pm_runtime_put_sync(&pdev->dev);
 	pm_runtime_disable(&pdev->dev);

^ permalink raw reply related

* Re: [PATCH -next] rocker: fix error return code in rocker_world_check_init()
From: Jiri Pirko @ 2016-10-22 15:34 UTC (permalink / raw)
  To: Wei Yongjun; +Cc: Wei Yongjun, netdev, davem
In-Reply-To: <1477146666-29671-1-git-send-email-weiyj.lk@gmail.com>

Sat, Oct 22, 2016 at 04:31:06PM CEST, weiyj.lk@gmail.com wrote:
>From: Wei Yongjun <weiyongjun1@huawei.com>
>
>Fix to return error code -EINVAL from the error handling
>case instead of 0, as done elsewhere in this function.
>
>Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>

Fixes: e420114eef4a ("rocker: introduce worlds infrastructure")
Acked-by: Jiri Pirko <jiri@mellanox.com>

Dave, you can queue this to -net.

^ permalink raw reply

* Re: [PATCH] flow_dissector: avoid uninitialized variable access
From: Eric Garver @ 2016-10-22 15:57 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jiri Pirko, David S. Miller, Alexander Duyck, Tom Herbert,
	Jiri Pirko, Hadar Hen Zion, Gao Feng, Amir Vadai, netdev,
	linux-kernel
In-Reply-To: <3516805.IZ705sLgKU@wuerfel>

On Sat, Oct 22, 2016 at 12:16:29AM +0200, Arnd Bergmann wrote:
> On Friday, October 21, 2016 11:05:45 PM CEST Arnd Bergmann wrote:
> > 
> > Can you explain why "dissector_uses_key(flow_dissector,
> > FLOW_DISSECTOR_KEY_VLAN) && skb_vlan_tag_present(skb)" implies
> > "eth_type_vlan(proto))"?
> > 
> > If I add uninitialized_var() here, I would at least put that in
> > a comment here.
> 
> Found it now myself: if skb_vlan_tag_present(skb), then we don't
> access 'vlan', otherwise we know it is initialized because
> eth_type_vlan(proto) has to be true.
>  
> > On a related note, I also don't see how
> > "dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_VLAN)"
> > implies that skb is non-NULL. I guess this is related to the
> > first one.
> 
> I'm still unsure about this one.

Only skb_flow_dissect_flow_keys_buf() calls this function with skb ==
NULL. It uses flow_keys_buf_dissector_keys which does not specify
FLOW_DISSECTOR_KEY_VLAN, so the if statement is false.

A similar assumption is made for FLOW_DISSECTOR_KEY_ETH_ADDRS higher up.

> I also found something else that is suspicious: 'vlan' points
> to the local _vlan variable, but that has gone out of scope
> by the time we access the pointer, which doesn't seem safe.

I see no harm in moving _vlan to the same scope as vlan.

^ permalink raw reply

* [PATCH v1 1/1] ISDN: eicon: replace custom hex_dump_to_buffer()
From: Andy Shevchenko @ 2016-10-22 17:37 UTC (permalink / raw)
  To: Armin Schindler, netdev, Karsten Keil; +Cc: Andy Shevchenko

Instead of custom approach re-use generic helper to convert bytes to hex
format.

The output is slightly changed, namely string starts from the first dword
value.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/isdn/hardware/eicon/message.c | 34 ++++------------------------------
 1 file changed, 4 insertions(+), 30 deletions(-)

diff --git a/drivers/isdn/hardware/eicon/message.c b/drivers/isdn/hardware/eicon/message.c
index 1a1d997..8684899 100644
--- a/drivers/isdn/hardware/eicon/message.c
+++ b/drivers/isdn/hardware/eicon/message.c
@@ -1147,45 +1147,19 @@ static byte test_c_ind_mask_bit(PLCI *plci, word b)
 
 static void dump_c_ind_mask(PLCI *plci)
 {
-	word i, j, k;
-	dword d;
-	char *p;
+	word i, j;
 	char buf[40];
 
 	for (i = 0; i < C_IND_MASK_DWORDS; i += 4)
 	{
-		p = buf + 36;
-		*p = '\0';
-		for (j = 0; j < 4; j++)
-		{
-			if (i + j < C_IND_MASK_DWORDS)
-			{
-				d = plci->c_ind_mask_table[i + j];
-				for (k = 0; k < 8; k++)
-				{
-					*(--p) = hex_asc_lo(d);
-					d >>= 4;
-				}
-			}
-			else if (i != 0)
-			{
-				for (k = 0; k < 8; k++)
-					*(--p) = ' ';
-			}
-			*(--p) = ' ';
-		}
-		dbug(1, dprintf("c_ind_mask =%s", (char *) p));
+		j = min_t(word, C_IND_MASK_DWORDS - i, 4) * 4;
+		hex_dump_to_buffer(plci->c_ind_mask_table[i], j, 16, 4, buf, sizeof(buf), false);
+		dbug(1, dprintf("c_ind_mask =%s", buf));
 	}
 }
 
-
-
-
-
 #define dump_plcis(a)
 
-
-
 /*------------------------------------------------------------------*/
 /* translation function for each message                            */
 /*------------------------------------------------------------------*/
-- 
2.9.3

^ permalink raw reply related

* Re: [PATCH] flow_dissector: avoid uninitialized variable access
From: Tom Herbert @ 2016-10-22 18:20 UTC (permalink / raw)
  To: Arnd Bergmann, Jiri Pirko, David S. Miller, Alexander Duyck,
	Tom Herbert, Jiri Pirko, Hadar Hen Zion, Gao Feng, Amir Vadai,
	Linux Kernel Network Developers, LKML
In-Reply-To: <20161022155752.GD26044@egarver>

On Sat, Oct 22, 2016 at 8:57 AM, Eric Garver <e@erig.me> wrote:
> On Sat, Oct 22, 2016 at 12:16:29AM +0200, Arnd Bergmann wrote:
>> On Friday, October 21, 2016 11:05:45 PM CEST Arnd Bergmann wrote:
>> >
>> > Can you explain why "dissector_uses_key(flow_dissector,
>> > FLOW_DISSECTOR_KEY_VLAN) && skb_vlan_tag_present(skb)" implies
>> > "eth_type_vlan(proto))"?
>> >
>> > If I add uninitialized_var() here, I would at least put that in
>> > a comment here.
>>
>> Found it now myself: if skb_vlan_tag_present(skb), then we don't
>> access 'vlan', otherwise we know it is initialized because
>> eth_type_vlan(proto) has to be true.
>>
>> > On a related note, I also don't see how
>> > "dissector_uses_key(flow_dissector, FLOW_DISSECTOR_KEY_VLAN)"
>> > implies that skb is non-NULL. I guess this is related to the
>> > first one.
>>
>> I'm still unsure about this one.
>
> Only skb_flow_dissect_flow_keys_buf() calls this function with skb ==
> NULL. It uses flow_keys_buf_dissector_keys which does not specify
> FLOW_DISSECTOR_KEY_VLAN, so the if statement is false.
>
> A similar assumption is made for FLOW_DISSECTOR_KEY_ETH_ADDRS higher up.
>
This is a serious problem. We can't rely on the callers to know which
keys they are allowed to use to avoid crashing the kernel. We should
fix those to check if skb is NULL, add a comment to the head of the
function warning people to never assume skb is non-NULL, and also
maybe add a degenerative check that both data argument and skb are not
NULL.

Tom

>> I also found something else that is suspicious: 'vlan' points
>> to the local _vlan variable, but that has gone out of scope
>> by the time we access the pointer, which doesn't seem safe.
>
> I see no harm in moving _vlan to the same scope as vlan.

^ permalink raw reply

* Re: [PATCH net-next 6/6] net: use core MTU range checking in misc drivers
From: Stefan Richter @ 2016-10-22 18:51 UTC (permalink / raw)
  To: Jarod Wilson
  Cc: Sabrina Dubroca, linux-kernel, netdev, Faisal Latif, linux-rdma,
	Cliff Whickman, Robin Holt, Jes Sorensen, Marek Lindner,
	Simon Wunderlich, Antonio Quartulli
In-Reply-To: <20161022113607.55832988@kant>

On Oct 22 Stefan Richter wrote:
> On Oct 19 Jarod Wilson wrote:
> > On Thu, Oct 20, 2016 at 12:38:46AM +0200, Stefan Richter wrote:  
> > > On Oct 19 Sabrina Dubroca wrote:  
> > > > 2016-10-18, 22:33:33 -0400, Jarod Wilson wrote:
[...]
> > > > > @@ -1481,6 +1471,8 @@ static int fwnet_probe(struct fw_unit *unit,
> > > > >  	max_mtu = (1 << (card->max_receive + 1))
> > > > >  		  - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;
> > > > >  	net->mtu = min(1500U, max_mtu);
> > > > > +	net->min_mtu = ETH_MIN_MTU;
> > > > > +	net->max_mtu = net->mtu;    
> > > > 
> > > > But that will now prevent increasing the MTU above the initial value?  
> > > 
> > > Indeed, therefore NAK.  
> > 
> > However, there's an explicit calculation for 'max_mtu' right there that I
> > glazed right over. It would seem perhaps *that* should be used for
> > net->max_mtu here, no?  
> 
> No.  This 'max_mtu' here is not the absolute maximum.  It is only an
> initial MTU which has the property that link fragmentation is not
> going to happen (if all other peers will at least as capable as this
> node).

Besides, card->max_receive is about what the card can receive (at the IEEE
1394 link layer), not about what the card can send.
-- 
Stefan Richter
-======----- =-=- =-==-
http://arcgraph.de/sr/

^ permalink raw reply

* Re: [Patch net] net: saving irq context for peernet2id()
From: Paul Moore @ 2016-10-22 19:02 UTC (permalink / raw)
  To: Cong Wang
  Cc: Stephen Smalley, Linux Kernel Network Developers, Elad Raz,
	Richard Guy Briggs
In-Reply-To: <CAM_iQpWfVWmG3egCXwbHSPEibtnh6G62xcFC9XXBNdojBqYCOg@mail.gmail.com>

On Fri, Oct 21, 2016 at 11:26 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Fri, Oct 21, 2016 at 1:03 PM, Paul Moore <paul@paul-moore.com> wrote:
>> On Fri, Oct 21, 2016 at 2:02 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>> On Fri, Oct 21, 2016 at 9:19 AM, Paul Moore <paul@paul-moore.com> wrote:
>>>> On Thu, Oct 20, 2016 at 7:35 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
>>>>> This is what I did in the follow up patch. I attach the updated version
>>>>> in this email for you to review ...
>>>>
>>>> I think there is still some confusion.  The second patch you posted
>>>> still has two queues with potentially duplicated (minus the length
>>>> tweaks) skbs.
>>>
>>> The current code without my patch is already this, the only difference
>>> is there is no queue for multicast case, duplication is already there.
>>
>> The difference is the period of time where the skbs are duplicated.
>> You patch duplicates the skb and then queues them, I'm suggesting
>> putting a single skb in the queue and then only duplicating it once it
>> has been pulled off the queue.
>
> I never disagree, the only thing you never explain is why we must do
> it in this patch rather than a patch later?

It seems obvious: if you do the skb_copy() before you queue the skbs
you are doubling the amount of memory used which is undesirable.

-- 
paul moore
www.paul-moore.com

^ permalink raw reply

* Re: [PATCH net-next v2 7/9] net: use core MTU range checking in misc drivers
From: Stefan Richter @ 2016-10-22 19:16 UTC (permalink / raw)
  To: Jarod Wilson
  Cc: linux-kernel, netdev, linux-rdma, Faisal Latif, Cliff Whickman,
	Robin Holt, Jes Sorensen, Marek Lindner, Simon Wunderlich,
	Antonio Quartulli, Sathya Prakash, Chaitra P B,
	Suganath Prabu Subramani, MPT-FusionLinux.pdl, Sebastian Reichel,
	Felipe Balbi, Arvid Brodin, Remi Denis-Courmont
In-Reply-To: <20161020175524.6184-8-jarod@redhat.com>

On Oct 20 Jarod Wilson wrote:
> firewire-net:
> - set min/max_mtu
> - remove fwnet_change_mtu
[...]
> --- a/drivers/firewire/net.c
> +++ b/drivers/firewire/net.c
[...]
> @@ -1478,9 +1467,10 @@ static int fwnet_probe(struct fw_unit *unit,
>  	 * Use the RFC 2734 default 1500 octets or the maximum payload
>  	 * as initial MTU
>  	 */
> -	max_mtu = (1 << (card->max_receive + 1))
> -		  - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;
> -	net->mtu = min(1500U, max_mtu);
> +	net->max_mtu = (1 << (card->max_receive + 1))
> +		       - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;
> +	net->mtu = min(1500U, net->max_mtu);
> +	net->min_mtu = ETH_MIN_MTU;
>  
>  	/* Set our hardware address while we're at it */
>  	ha = (union fwnet_hwaddr *)net->dev_addr;

Please preserve the current behavior, i.e. do not enforce any particular
upper bound.  (Especially none based on the local link layer controller's
max_receive parameter.)

BTW, after having read RFC 2734, RFC 3146, and the code, I am convinced
that net->mtu should be initialized to 1500, not less.  But such a change
should be done in a separate patch.
-- 
Stefan Richter
-======----- =-=- =-==-
http://arcgraph.de/sr/

^ permalink raw reply

* Re: [PATCH net-next v2 7/9] net: use core MTU range checking in misc drivers
From: Stefan Richter @ 2016-10-22 19:27 UTC (permalink / raw)
  To: Jarod Wilson; +Cc: linux-kernel, netdev, linux1394-devel
In-Reply-To: <20161022211606.3b5d137d@kant>

Adding Cc: linux1394-devel, dropping several Ccs, no additional comment.

On Oct 22 Stefan Richter wrote:
> On Oct 20 Jarod Wilson wrote:
> > firewire-net:
> > - set min/max_mtu
> > - remove fwnet_change_mtu  
> [...]
> > --- a/drivers/firewire/net.c
> > +++ b/drivers/firewire/net.c
> > @@ -1349,15 +1349,6 @@ static netdev_tx_t fwnet_tx(struct sk_buff
> > *skb, struct net_device *net) return NETDEV_TX_OK;
> >  }
> >  
> > -static int fwnet_change_mtu(struct net_device *net, int new_mtu)
> > -{
> > -	if (new_mtu < 68)
> > -		return -EINVAL;
> > -
> > -	net->mtu = new_mtu;
> > -	return 0;
> > -}
> > -
> >  static const struct ethtool_ops fwnet_ethtool_ops = {
> >  	.get_link	= ethtool_op_get_link,
> >  };
> > @@ -1366,7 +1357,6 @@ static const struct net_device_ops
> > fwnet_netdev_ops = { .ndo_open       = fwnet_open,
> >  	.ndo_stop	= fwnet_stop,
> >  	.ndo_start_xmit = fwnet_tx,
> > -	.ndo_change_mtu = fwnet_change_mtu,
> >  };
> >  
> >  static void fwnet_init_dev(struct net_device *net)
> > @@ -1435,7 +1425,6 @@ static int fwnet_probe(struct fw_unit *unit,
> >  	struct net_device *net;
> >  	bool allocated_netdev = false;
> >  	struct fwnet_device *dev;
> > -	unsigned max_mtu;
> >  	int ret;
> >  	union fwnet_hwaddr *ha;
> >  
> > @@ -1478,9 +1467,10 @@ static int fwnet_probe(struct fw_unit *unit,
> >  	 * Use the RFC 2734 default 1500 octets or the maximum payload
> >  	 * as initial MTU
> >  	 */
> > -	max_mtu = (1 << (card->max_receive + 1))
> > -		  - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;
> > -	net->mtu = min(1500U, max_mtu);
> > +	net->max_mtu = (1 << (card->max_receive + 1))
> > +		       - sizeof(struct rfc2734_header) - IEEE1394_GASP_HDR_SIZE;
> > +	net->mtu = min(1500U, net->max_mtu);
> > +	net->min_mtu = ETH_MIN_MTU;
> >  
> >  	/* Set our hardware address while we're at it */
> >  	ha = (union fwnet_hwaddr *)net->dev_addr;
> 
> Please preserve the current behavior, i.e. do not enforce any particular
> upper bound.  (Especially none based on the local link layer controller's
> max_receive parameter.)
> 
> BTW, after having read RFC 2734, RFC 3146, and the code, I am convinced
> that net->mtu should be initialized to 1500, not less.  But such a change
> should be done in a separate patch.
-- 
Stefan Richter
-======----- =-=- =-==-
http://arcgraph.de/sr/

^ permalink raw reply

* Re: [PATCH] netns: revert "netns: avoid disabling irq for netns id"
From: Paul Moore @ 2016-10-22 19:29 UTC (permalink / raw)
  To: Cong Wang; +Cc: Linux Kernel Network Developers, Elad Raz, Stephen Smalley
In-Reply-To: <CAM_iQpXd=8nHeCVKx4a5kBNf0mGb88i5eL=8+oaZcK-wKGVsXg@mail.gmail.com>

On Fri, Oct 21, 2016 at 11:38 PM, Cong Wang <xiyou.wangcong@gmail.com> wrote:
> On Fri, Oct 21, 2016 at 6:49 PM, Paul Moore <pmoore@redhat.com> wrote:
>> Eventually we should be able to reintroduce this code once we have
>> rewritten the audit multicast code to queue messages much the same
>> way we do for unicast messages.  A tracking issue for this can be
>> found below:
>
> NAK.
>
> 1) This will be forgotten by Paul.

The way things are going right now, this argument is going to devolve
into a "yes he will"/"no I won't" so I'll just repeat that I've
created a tracking issue for this so I won't forget (and included a
link, repeated below, in the commit description) and I think I have a
reasonable history of following through on things.

* https://github.com/linux-audit/audit-kernel/issues/23

> 2) There is already a fix which is considered as a rework by Paul.

Already discussed this in the other thread, I'm not going to go into
detail here, just a quick summary: the fix provided by Cong Wang
doubles the message queue's memory consumption and changes some
fundamentals in how multicast messages are handled.  The memory
issues, while still an objectionable blocker, are easily resolved, but
moving the netlink multicast send is something I want to make sure is
tested/baked for a bit (it's 4.10 merge window material as far as I'm
concerned).

At this point I think it is worth mentioning that we are in this
position due to a lack of testing; if Cong Wang had tested his
original patch with SELinux we might not be dealing with this
regression now.  A more measured approach seems very reasonable.

> 3) -rc2 is Paul's personal deadline, not ours.

The current 4.9-rc kernels are broken and cause errors when SELinux is
enabled, while I understand SELinux is not a priority (or a secondary,
or tertiary, or N-ary concern) for many on the netdev list, it is
still an important part of the kernel and this regression needs to be
treated seriously and corrected soon.

SELinux/audit has run into interaction issues with the network stack
before, and we've worked together to sort things out; I'm hopeful
cooler heads will prevail and we can do the same here.

-- 
paul moore
security @ redhat

^ permalink raw reply

* Re: [PATCH] netns: revert "netns: avoid disabling irq for netns id"
From: David Miller @ 2016-10-22 20:16 UTC (permalink / raw)
  To: pmoore; +Cc: xiyou.wangcong, netdev, e, sds
In-Reply-To: <CAGH-Kgv6BXrTLPdsLfYnoU8nVeh7fTv89xPZixOS9qqPg8=PXQ@mail.gmail.com>


I'm tired of hearing you guys go back and forth with each other and
will simply apply the revert.

Thanks.

^ permalink raw reply

* Re: [PATCH net 0/2] net: dsa: bcm_sf2: Do not rely on kexec_in_progress
From: David Miller @ 2016-10-22 20:18 UTC (permalink / raw)
  To: f.fainelli; +Cc: netdev, linux-kernel, ebiederm, kexec
In-Reply-To: <1477084916-26327-1-git-send-email-f.fainelli@gmail.com>

From: Florian Fainelli <f.fainelli@gmail.com>
Date: Fri, 21 Oct 2016 14:21:54 -0700

> These are the two patches following the discussing we had on
> kexec_in_progress.
> 
> Feel free to apply or discard them, thanks!

Applied, thanks.

^ permalink raw reply

* Re: [Patch net] ipv4: use the right lock for ping_group_range
From: David Miller @ 2016-10-22 20:24 UTC (permalink / raw)
  To: xiyou.wangcong; +Cc: netdev, edumazet, salo
In-Reply-To: <1476998386-20935-1-git-send-email-xiyou.wangcong@gmail.com>

From: Cong Wang <xiyou.wangcong@gmail.com>
Date: Thu, 20 Oct 2016 14:19:46 -0700

> This reverts commit a681574c99be23e4d20b769bf0e543239c364af5
> ("ipv4: disable BH in set_ping_group_range()") because we never
> read ping_group_range in BH context (unlike local_port_range).
> 
> Then, since we already have a lock for ping_group_range, those
> using ip_local_ports.lock for ping_group_range are clearly typos.
> 
> We might consider to share a same lock for both ping_group_range
> and local_port_range w.r.t. space saving, but that should be for
> net-next.
> 
> Fixes: a681574c99be ("ipv4: disable BH in set_ping_group_range()")
> Fixes: ba6b918ab234 ("ping: move ping_group_range out of CONFIG_SYSCTL")
> Cc: Eric Dumazet <edumazet@google.com>
> Cc: Eric Salo <salo@google.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@gmail.com>

Applied and queued up for -stable, thanks.

^ permalink raw reply

* [PATCH net-next] flow_dissector: fix vlan tag handling
From: Arnd Bergmann @ 2016-10-22 20:30 UTC (permalink / raw)
  To: Tom Herbert
  Cc: Jiri Pirko, David S. Miller, Alexander Duyck, Jiri Pirko,
	Hadar Hen Zion, Gao Feng, Amir Vadai,
	Linux Kernel Network Developers, LKML, Linus Torvalds
In-Reply-To: <CALx6S35oHmGNw=7taBLxY8ZJHso-b8eRokz3QFA5j_8tW5N8yg@mail.gmail.com>

gcc warns about an uninitialized pointer dereference in the vlan
priority handling:

net/core/flow_dissector.c: In function '__skb_flow_dissect':
net/core/flow_dissector.c:281:61: error: 'vlan' may be used uninitialized in this function [-Werror=maybe-uninitialized]

As pointed out by Jiri Pirko, the variable is never actually used
without being initialized first as the only way it end up uninitialized
is with skb_vlan_tag_present(skb)==true, and that means it does not
get accessed.

However, the warning hints at some related issues that I'm addressing
here:

- the second check for the vlan tag is different from the first one
  that tests the skb for being NULL first, causing both the warning
  and a possible NULL pointer dereference that was not entirely fixed.
- The same patch that introduced the NULL pointer check dropped an
  earlier optimization that skipped the repeated check of the
  protocol type
- The local '_vlan' variable is referenced through the 'vlan' pointer
  but the variable has gone out of scope by the time that it is
  accessed, causing undefined behavior as the stack may have been
  overwritten.

Caching the result of the 'skb && skb_vlan_tag_present(skb)' check
in a local variable allows the compiler to further optimize the
later check. With those changes, the warning also disappears.

Fixes: 3805a938a6c2 ("flow_dissector: Check skb for VLAN only if skb specified.")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---

diff --git a/net/core/flow_dissector.c b/net/core/flow_dissector.c
index 44e6ba9d3a6b..17be1b66cc41 100644
--- a/net/core/flow_dissector.c
+++ b/net/core/flow_dissector.c
@@ -246,13 +246,10 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 	case htons(ETH_P_8021AD):
 	case htons(ETH_P_8021Q): {
 		const struct vlan_hdr *vlan;
+		struct vlan_hdr _vlan;
+		bool vlan_tag_present = (skb && skb_vlan_tag_present(skb));
 
-		if (skb && skb_vlan_tag_present(skb))
-			proto = skb->protocol;
-
-		if (eth_type_vlan(proto)) {
-			struct vlan_hdr _vlan;
-
+		if (!vlan_tag_present || eth_type_vlan(skb->protocol)) {
 			vlan = __skb_header_pointer(skb, nhoff, sizeof(_vlan),
 						    data, hlen, &_vlan);
 			if (!vlan)
@@ -270,7 +267,7 @@ bool __skb_flow_dissect(const struct sk_buff *skb,
 							     FLOW_DISSECTOR_KEY_VLAN,
 							     target_container);
 
-			if (skb_vlan_tag_present(skb)) {
+			if (vlan_tag_present) {
 				key_vlan->vlan_id = skb_vlan_tag_get_id(skb);
 				key_vlan->vlan_priority =
 					(skb_vlan_tag_get_prio(skb) >> VLAN_PRIO_SHIFT);

^ permalink raw reply related

* Re: [PATCH net-next v6 0/3] udp: refactor memory accounting
From: David Miller @ 2016-10-22 20:50 UTC (permalink / raw)
  To: pabeni-H+wXaHxf7aLQT0dZR+AlfA
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, jmorris-gx6/JNMH7DfYtjvyW6yDsg,
	trond.myklebust-7I+n7zu2hftEKMMhf/gKZA,
	aduyck-nYU0QVwCCFFWk0Htik3J/w, daniel-FeC+5ew28dpmcu3hnIyYJQ,
	edumazet-hpIqsD4AKlfQT0dZR+AlfA, tom-BjP2VixgY4xUbtYUoyoikg,
	hannes-tFNcAqjVMyqKXQKiL6tip0B+6BGkLq7r,
	ecree-s/n/eUQHGBpZroRs9YW3xA, linux-nfs-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <cover.1477043395.git.pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

From: Paolo Abeni <pabeni-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Date: Fri, 21 Oct 2016 13:55:44 +0200

> This patch series refactor the udp memory accounting, replacing the
> generic implementation with a custom one, in order to remove the needs for
> locking the socket on the enqueue and dequeue operations. The socket backlog
> usage is dropped, as well.
 ...

Series applied, thank you.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH net-next 0/2] Add BPF numa id helper
From: David Miller @ 2016-10-22 20:52 UTC (permalink / raw)
  To: daniel; +Cc: alexei.starovoitov, edumazet, netdev
In-Reply-To: <cover.1477002095.git.daniel@iogearbox.net>

From: Daniel Borkmann <daniel@iogearbox.net>
Date: Fri, 21 Oct 2016 00:30:25 +0200

> This patch set adds a helper for retrieving current numa node
> id and a test case for SO_REUSEPORT.

Series applied.

^ permalink raw reply

* Re: [PATCH] net: skip genenerating uevents for network namespaces that are exiting
From: David Miller @ 2016-10-22 21:00 UTC (permalink / raw)
  To: avagin; +Cc: ebiederm, containers, linux-kernel, netdev
In-Reply-To: <1477017986-18889-1-git-send-email-avagin@openvz.org>

From: Andrei Vagin <avagin@openvz.org>
Date: Thu, 20 Oct 2016 19:46:26 -0700

> @@ -1525,6 +1530,9 @@ void netdev_unregister_kobject(struct net_device *ndev)
>  {
>  	struct device *dev = &(ndev->dev);
>  
> +	if (!list_empty(&dev_net(ndev)->exit_list))
> +		dev->kobj.uevent_suppress = 1;
> +

Please use "dev_set_uevent_suppress(dev, 1);"

^ 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