Netdev List
 help / color / mirror / Atom feed
* [PATCH] smsc-ircc2: Fix section mismatch derived from smsc_ircc_pnp_driver variable
From: Sedat Dilek @ 2011-01-03  2:28 UTC (permalink / raw)
  To: samuel, netdev, linux-kernel; +Cc: Sedat Dilek

>From my build.log:

drivers/net/irda/smsc-ircc2.o(.data+0x18): Section mismatch in reference from the variable smsc_ircc_pnp_driver to the function .init.text:smsc_ircc_pnp_probe()
The variable smsc_ircc_pnp_driver references
the function __init smsc_ircc_pnp_probe()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console,

This patch fixes the warning.

Tested with linux-next (next-20101231)

Signed-off-by: Sedat Dilek <sedat.dilek@gmail.com>
---
 drivers/net/irda/smsc-ircc2.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/irda/smsc-ircc2.c b/drivers/net/irda/smsc-ircc2.c
index 8c57bfb..0ebd4a8 100644
--- a/drivers/net/irda/smsc-ircc2.c
+++ b/drivers/net/irda/smsc-ircc2.c
@@ -397,7 +397,7 @@ static int __init smsc_ircc_pnp_probe(struct pnp_dev *dev,
 	return 0;
 }
 
-static struct pnp_driver smsc_ircc_pnp_driver = {
+static struct pnp_driver smsc_ircc_pnp_driver __refdata = {
 	.name		= "smsc-ircc2",
 	.id_table	= smsc_ircc_pnp_table,
 	.probe		= smsc_ircc_pnp_probe,
-- 
1.7.4.rc0


^ permalink raw reply related

* [PATCH] depca: Fix section mismatch derived from depca_isa_driver variable
From: Sedat Dilek @ 2011-01-03  2:33 UTC (permalink / raw)
  To: netdev, linux-kernel; +Cc: Sedat Dilek

>From my build.log:

WARNING: drivers/net/depca.o(.data+0x0): Section mismatch in reference from the variable depca_isa_driver to the function .init.text:depca_isa_probe()
The variable depca_isa_driver references
the function __init depca_isa_probe()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console,

This patch fixes the warning.

Tested with linux-next (next-20101231)

Signed-off-by: Sedat Dilek <sedat.dilek@gmail.com>
---
 drivers/net/depca.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/depca.c b/drivers/net/depca.c
index 91b3846..dfaf746 100644
--- a/drivers/net/depca.c
+++ b/drivers/net/depca.c
@@ -405,7 +405,7 @@ static int __devexit depca_isa_remove(struct platform_device *pdev)
 	return depca_device_remove(&pdev->dev);
 }
 
-static struct platform_driver depca_isa_driver = {
+static struct platform_driver depca_isa_driver __refdata = {
 	.probe  = depca_isa_probe,
 	.remove = __devexit_p(depca_isa_remove),
 	.driver	= {
-- 
1.7.4.rc0

^ permalink raw reply related

* Re: [net-next-2.6 PATCH v2 2/4] dcbnl: adding DCBX feature flags get-set
From: John Fastabend @ 2011-01-03  2:38 UTC (permalink / raw)
  To: Shmulik Ravid
  Cc: davem@davemloft.net, Eilon Greenstein, Liu, Lucy,
	netdev@vger.kernel.org
In-Reply-To: <1293984061.29378.101.camel@lb-tlvb-shmulik.il.broadcom.com>

On 1/2/2011 8:01 AM, Shmulik Ravid wrote:
> 
>> One more nit ;)
>>
>>> +
>>> +	ret = nla_parse_nested(data, DCB_FEATCFG_ATTR_MAX, tb[DCB_ATTR_FEATCFG],
>>> +			       dcbnl_featcfg_nest);
>>> +	if (ret) {
>>> +		ret = -EINVAL;
>>> +		goto err_out;
>>> +	}
>>
>> Why do you set EINVAL here if you use the returned error code from nla_parse_nested you get a more descriptive error. See ./lib/nlattr.c:nla_parse()/validate_nla().
>>
>> [...]
>>
>>> +static int dcbnl_setfeatcfg(struct net_device *netdev, struct nlattr **tb,
>>> +			    u32 pid, u32 seq, u16 flags)
>>> +{
>>> +	struct nlattr *data[DCB_FEATCFG_ATTR_MAX + 1];
>>> +	int ret = -EINVAL;
>>> +	u8 value;
>>> +	int i;
>>> +
>>> +	if (!tb[DCB_ATTR_FEATCFG] || !netdev->dcbnl_ops->setfeatcfg)
>>> +		return ret;
>>> +
>>> +	ret = nla_parse_nested(data, DCB_FEATCFG_ATTR_MAX, tb[DCB_ATTR_FEATCFG],
>>> +			       dcbnl_featcfg_nest);
>>> +
>>> +	if (ret) {
>>> +		ret = -EINVAL;
>>> +		goto err;
>>> +	}
>>
>> Same here.
>>
> 
> I'll send a patch with the improved return values for the the new dcbnl
> routines. While I'm at it, is it safe to fix on the same lines the
> older already established dcbnl routines?

This should be safe I would not expect using more accurate error values could hurt any existing applications. Be sure to make it a separate patch though.

John

^ permalink raw reply

* Re: [net-next-2.6 02/08] r8169: identify different registers.
From: Ben Hutchings @ 2011-01-03  2:52 UTC (permalink / raw)
  To: Francois Romieu; +Cc: davem, netdev, Hayes Wang, David Woodhouse
In-Reply-To: <20110102233617.GC5780@electric-eye.fr.zoreil.com>

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

On Mon, 2011-01-03 at 00:36 +0100, Francois Romieu wrote:
> Documentation (sort of).
> 
> The location are the same, the values are the same but it is
> just accidental. Note that the 810x could cope with a smaller
> value as it does not support jumbo frames.
> 
> Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
> Cc: Hayes <hayeswang@realtek.com>
> ---
>  drivers/net/r8169.c |   22 ++++++++++++++--------
>  1 files changed, 14 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> index 3124462..8aa92ad 100644
> --- a/drivers/net/r8169.c
> +++ b/drivers/net/r8169.c
[...]
> @@ -3036,7 +3042,7 @@ static void rtl_hw_start_8168bef(void __iomem *ioaddr, struct pci_dev *pdev)
>  {
>  	rtl_hw_start_8168bb(ioaddr, pdev);
>  
> -	RTL_W8(EarlyTxThres, EarlyTxThld);
> +	RTL_W8(MaxTxPacketSize, 0x3f);
[...]

Shouldn't the value here be written as TxPacketMax?

Ben.

-- 
Ben Hutchings, Debian Developer and kernel team member


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

^ permalink raw reply

* Re: [net-next-2.6 02/08] r8169: identify different registers.
From: David Miller @ 2011-01-03  2:58 UTC (permalink / raw)
  To: benh; +Cc: romieu, netdev, hayeswang, dwmw2
In-Reply-To: <1294023131.3167.138.camel@localhost>

From: Ben Hutchings <benh@debian.org>
Date: Mon, 03 Jan 2011 02:52:11 +0000

> On Mon, 2011-01-03 at 00:36 +0100, Francois Romieu wrote:
>> Documentation (sort of).
>> 
>> The location are the same, the values are the same but it is
>> just accidental. Note that the 810x could cope with a smaller
>> value as it does not support jumbo frames.
>> 
>> Signed-off-by: Francois Romieu <romieu@fr.zoreil.com>
>> Cc: Hayes <hayeswang@realtek.com>
>> ---
>>  drivers/net/r8169.c |   22 ++++++++++++++--------
>>  1 files changed, 14 insertions(+), 8 deletions(-)
>> 
>> diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
>> index 3124462..8aa92ad 100644
>> --- a/drivers/net/r8169.c
>> +++ b/drivers/net/r8169.c
> [...]
>> @@ -3036,7 +3042,7 @@ static void rtl_hw_start_8168bef(void __iomem *ioaddr, struct pci_dev *pdev)
>>  {
>>  	rtl_hw_start_8168bb(ioaddr, pdev);
>>  
>> -	RTL_W8(EarlyTxThres, EarlyTxThld);
>> +	RTL_W8(MaxTxPacketSize, 0x3f);
> [...]
> 
> Shouldn't the value here be written as TxPacketMax?

Yep, looks that way to me too.

Otherwise why add the new TxPacketMax definition :-)

^ permalink raw reply

* [patch] mac80211: potential null dereference in mesh forwarding
From: Dan Carpenter @ 2011-01-03  5:43 UTC (permalink / raw)
  To: John W. Linville
  Cc: Johannes Berg, David S. Miller, linux-wireless, netdev,
	kernel-janitors

The printk() is supposed to be ratelimited but we should always goto out
when fwd_skb is NULL.  Otherwise it gets dereferenced on the next line.

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 5e9d3bc..dc8b566 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1831,8 +1831,9 @@ ieee80211_rx_h_mesh_fwding(struct ieee80211_rx_data *rx)
 
 			fwd_skb = skb_copy(skb, GFP_ATOMIC);
 
-			if (!fwd_skb && net_ratelimit()) {
-				printk(KERN_DEBUG "%s: failed to clone mesh frame\n",
+			if (!fwd_skb) {
+				if (net_ratelimit())
+					printk(KERN_DEBUG "%s: failed to clone mesh frame\n",
 						   sdata->name);
 				goto out;
 			}

^ permalink raw reply related

* Re: [net-next-2.6 PATCH v2 3/3] net_sched: implement a root container qdisc sch_mclass
From: John Fastabend @ 2011-01-03  5:43 UTC (permalink / raw)
  To: Jarek Poplawski
  Cc: davem@davemloft.net, netdev@vger.kernel.org, hadi@cyberus.ca,
	shemminger@vyatta.com, tgraf@infradead.org,
	eric.dumazet@gmail.com, bhutchings@solarflare.com,
	nhorman@tuxdriver.com
In-Reply-To: <4D1D17C9.3040500@gmail.com>

On 12/30/2010 3:37 PM, Jarek Poplawski wrote:
> John Fastabend wrote:
>> This implements a mclass 'multi-class' queueing discipline that by
>> default creates multiple mq qdisc's one for each traffic class. Each
>> mq qdisc then owns a range of queues per the netdev_tc_txq mappings.
> 
> Is it really necessary to add one more abstraction layer for this,
> probably not most often used (or even asked by users), functionality?
> Why mclass can't simply do these few things more instead of attaching
> (and changing) mq?
> 

The statistics work nicely when the mq qdisc is used. 

qdisc mclass 8002: root  tc 4 map 0 1 2 3 0 1 2 3 1 1 1 1 1 1 1 1
             queues:(0:1) (2:3) (4:5) (6:15)
 Sent 140 bytes 2 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
qdisc mq 8003: parent 8002:1
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
qdisc mq 8004: parent 8002:2
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
qdisc mq 8005: parent 8002:3
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
qdisc mq 8006: parent 8002:4
 Sent 140 bytes 2 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
qdisc sfq 8007: parent 8005:1 limit 127p quantum 1514b
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0
qdisc sfq 8008: parent 8005:2 limit 127p quantum 1514b
 Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
 backlog 0b 0p requeues 0

The mclass gives the statistics for the interface and then statistics on the mq qdisc gives statistics for each traffic class. Also, when using the 'mq qdisc' with this abstraction other qdisc can be grafted onto the queue. For example the sch_sfq is used in the above example.

Although I am not too hung up on this use case it does seem to be a good abstraction to me. Is it strictly necessary though no and looking at the class statistics of mclass could be used to get stats per traffic class.

> ...
>> diff --git a/include/net/sch_generic.h b/include/net/sch_generic.h
>> index 0af57eb..723ee52 100644
>> --- a/include/net/sch_generic.h
>> +++ b/include/net/sch_generic.h
>> @@ -50,6 +50,7 @@ struct Qdisc {
>>  #define TCQ_F_INGRESS		4
>>  #define TCQ_F_CAN_BYPASS	8
>>  #define TCQ_F_MQROOT		16
>> +#define TCQ_F_MQSAFE		32
> 
> If every other qdisc added a flag for qdiscs it likes...
> 

then we run out of bits and get unneeded complexity. I think I will drop the MQSAFE bit completely and let user space catch this. The worst that should happen is the noop qdisc is used.

>> @@ -709,7 +709,13 @@ static void attach_default_qdiscs(struct net_device *dev)
>>  		dev->qdisc = txq->qdisc_sleeping;
>>  		atomic_inc(&dev->qdisc->refcnt);
>>  	} else {
>> -		qdisc = qdisc_create_dflt(txq, &mq_qdisc_ops, TC_H_ROOT);
>> +		if (dev->num_tc)
> 
> Actually, where this num_tc is expected to be set? I can see it inside
> mclass only, with unsetting on destruction, but probably I miss something.

Either through mclass as you noted or a driver could set the num_tc. One of the RFC's I sent out has ixgbe setting the num_tc when DCB was enabled.

>> +			qdisc = qdisc_create_dflt(txq, &mclass_qdisc_ops,
>> +						  TC_H_ROOT);
>> +		else
>> +			qdisc = qdisc_create_dflt(txq, &mq_qdisc_ops,
>> +						  TC_H_ROOT);
>> +
>> +static int mclass_init(struct Qdisc *sch, struct nlattr *opt)
>> +{
>> +	struct net_device *dev = qdisc_dev(sch);
>> +	struct mclass_sched *priv = qdisc_priv(sch);
>> +	struct netdev_queue *dev_queue;
>> +	struct Qdisc *qdisc;
>> +	int i, err = -EOPNOTSUPP;
>> +	struct tc_mclass_qopt *qopt = NULL;
>> +
>> +	/* Unwind attributes on failure */
>> +	u8 unwnd_tc = dev->num_tc;
>> +	u8 unwnd_map[16];
> 
> [TC_MAX_QUEUE] ?

Actually TC_BITMASK+1 is probably more accurate. This array maps the skb priority to a traffic class after the priority is masked with TC_BITMASK.

> 
>> +	struct netdev_tc_txq unwnd_txq[16];
>> +

Although unwnd_txq should be TC_MAX_QUEUE.

>> +	if (sch->parent != TC_H_ROOT)
>> +		return -EOPNOTSUPP;
>> +
>> +	if (!netif_is_multiqueue(dev))
>> +		return -EOPNOTSUPP;
>> +
>> +	if (nla_len(opt) < sizeof(*qopt))
>> +		return -EINVAL;
>> +	qopt = nla_data(opt);
>> +
>> +	memcpy(unwnd_map, dev->prio_tc_map, sizeof(unwnd_map));
>> +	memcpy(unwnd_txq, dev->tc_to_txq, sizeof(unwnd_txq));
>> +
>> +	/* If the mclass options indicate that hardware should own
>> +	 * the queue mapping then run ndo_setup_tc if this can not
>> +	 * be done fail immediately.
>> +	 */
>> +	if (qopt->hw && dev->netdev_ops->ndo_setup_tc) {
>> +		priv->hw_owned = 1;
>> +		if (dev->netdev_ops->ndo_setup_tc(dev, qopt->num_tc))
>> +			return -EINVAL;
>> +	} else if (!qopt->hw) {
>> +		if (mclass_parse_opt(dev, qopt))
>> +			return -EINVAL;
>> +
>> +		if (netdev_set_num_tc(dev, qopt->num_tc))
>> +			return -ENOMEM;
>> +
>> +		for (i = 0; i < qopt->num_tc; i++)
>> +			netdev_set_tc_queue(dev, i,
>> +					    qopt->count[i], qopt->offset[i]);
>> +	} else {
>> +		return -EINVAL;
>> +	}
>> +
>> +	/* Always use supplied priority mappings */
>> +	for (i = 0; i < 16; i++) {
> 
> i < qopt->num_tc ?

Nope, TC_BITMASK+1 here. If we only have 4 tcs for example we still need to map all 16 priority values to a tc.

> 
>> +		if (netdev_set_prio_tc_map(dev, i, qopt->prio_tc_map[i])) {
>> +			err = -EINVAL;
>> +			goto tc_err;
>> +		}
>> +	}
>> +
>> +	/* pre-allocate qdisc, attachment can't fail */
>> +	priv->qdiscs = kcalloc(qopt->num_tc,
>> +			       sizeof(priv->qdiscs[0]), GFP_KERNEL);
>> +	if (priv->qdiscs == NULL) {
>> +		err = -ENOMEM;
>> +		goto tc_err;
>> +	}
>> +
>> +	for (i = 0; i < dev->num_tc; i++) {
>> +		dev_queue = netdev_get_tx_queue(dev, dev->tc_to_txq[i].offset);
> 
> Are these offsets etc. validated?

Yes, as your next email noted.

Thanks,
John

^ permalink raw reply

* Re: [net-next-2.6 PATCH v2 3/3] net_sched: implement a root container qdisc sch_mclass
From: John Fastabend @ 2011-01-03  5:46 UTC (permalink / raw)
  To: Jarek Poplawski
  Cc: davem@davemloft.net, netdev@vger.kernel.org, hadi@cyberus.ca,
	shemminger@vyatta.com, tgraf@infradead.org,
	eric.dumazet@gmail.com, bhutchings@solarflare.com,
	nhorman@tuxdriver.com
In-Reply-To: <20101231092543.GA7809@ff.dom.local>

On 12/31/2010 1:25 AM, Jarek Poplawski wrote:
> On 2010-12-21 20:29, John Fastabend wrote:
>> This implements a mclass 'multi-class' queueing discipline that by
>> default creates multiple mq qdisc's one for each traffic class. Each
>> mq qdisc then owns a range of queues per the netdev_tc_txq mappings.
> 
> Btw, you could also consider better name (mqprio?) because there're
> many 'multi-class' queueing disciplines around.
> 

OK.

>> +static int mclass_parse_opt(struct net_device *dev, struct tc_mclass_qopt *qopt)
>> +{
>> +	int i, j;
>> +
>> +	/* Verify TC offset and count are sane */
> 
> if (qopt->num_tc > TC_MAX_QUEUE) ?
> 	return -EINVAL;

This would be caught later when netdev_set_num_tc() fails although probably best to catch all failures in this function as early as possible.

> 
>> +	for (i = 0; i < qopt->num_tc; i++) {
>> +		int last = qopt->offset[i] + qopt->count[i];
>> +		if (last > dev->num_tx_queues)
> 
> if (last >= dev->num_tx_queues) ?
> 
>> +			return -EINVAL;
>> +		for (j = i + 1; j < qopt->num_tc; j++) {
>> +			if (last > qopt->offset[j])
> 
> if (last >= qopt->offset[j]) ?
	
I believe the below works as expected. The offset needs to be verified (this I missed) but offset+count can be equal to num_tx_queue indicating the last queue is in use. With 8 tx queues and num_tc=2 a valid configuration is, tc1 offset of 0 and a count of 7 with tc2 offset of 7 and count of 1.


        /* Verify num_tc is in max range */
        if (qopt->num_tc > TC_MAX_QUEUE)
                return -EINVAL;

        for (i = 0; i < qopt->num_tc; i++) {
                /* Verify the queue offset is in the num tx range */
                if (qopt->offset[i] >= dev->num_tx_queues)
                        return -EINVAL;
                /* Verify the queue count is in tx range being equal to the
                 * num_tx_queues indicates the last queue is in use.
                 */
                else if (qopt->offset[i] + qopt->count[i] > dev->num_tx_queues)
                        return -EINVAL;

                /* Verify that the offset and counts do not overlap */
                for (j = i + 1; j < qopt->num_tc; j++) {
                        if (last > qopt->offset[j])
                                return -EINVAL;
                }
        }


Thanks for the review!

John.

> 
> Jarek P.


^ permalink raw reply

* Re: [patch] mac80211: potential null dereference in mesh forwarding
From: Eric Dumazet @ 2011-01-03  7:45 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: John W. Linville, Johannes Berg, David S. Miller, linux-wireless,
	netdev, kernel-janitors
In-Reply-To: <20110103054355.GP1886@bicker>

Le lundi 03 janvier 2011 à 08:43 +0300, Dan Carpenter a écrit :
> The printk() is supposed to be ratelimited but we should always goto out
> when fwd_skb is NULL.  Otherwise it gets dereferenced on the next line.
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> 
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 5e9d3bc..dc8b566 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -1831,8 +1831,9 @@ ieee80211_rx_h_mesh_fwding(struct ieee80211_rx_data *rx)
>  
>  			fwd_skb = skb_copy(skb, GFP_ATOMIC);
>  
> -			if (!fwd_skb && net_ratelimit()) {
> -				printk(KERN_DEBUG "%s: failed to clone mesh frame\n",
> +			if (!fwd_skb) {
> +				if (net_ratelimit())
> +					printk(KERN_DEBUG "%s: failed to clone mesh frame\n",
>  						   sdata->name);
>  				goto out;
>  			}

Already discovered/coped by Milton Miller.




^ permalink raw reply

* [PATCHv2 NEXT 2/2] netxen: update driver version 4.0.75
From: Amit Kumar Salecha @ 2011-01-03  7:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, ameen.rahman, anirban.chakraborty
In-Reply-To: <1294041525-15553-1-git-send-email-amit.salecha@qlogic.com>

Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 drivers/net/netxen/netxen_nic.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/netxen/netxen_nic.h b/drivers/net/netxen/netxen_nic.h
index 4e54587..a113805 100644
--- a/drivers/net/netxen/netxen_nic.h
+++ b/drivers/net/netxen/netxen_nic.h
@@ -53,8 +53,8 @@
 
 #define _NETXEN_NIC_LINUX_MAJOR 4
 #define _NETXEN_NIC_LINUX_MINOR 0
-#define _NETXEN_NIC_LINUX_SUBVERSION 74
-#define NETXEN_NIC_LINUX_VERSIONID  "4.0.74"
+#define _NETXEN_NIC_LINUX_SUBVERSION 75
+#define NETXEN_NIC_LINUX_VERSIONID  "4.0.75"
 
 #define NETXEN_VERSION_CODE(a, b, c)	(((a) << 24) + ((b) << 16) + (c))
 #define _major(v)	(((v) >> 24) & 0xff)
-- 
1.7.3.2


^ permalink raw reply related

* [PATCHv2 NEXT 1/2] netxen: enable LRO based on NETIF_F_LRO
From: Amit Kumar Salecha @ 2011-01-03  7:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, ameen.rahman, anirban.chakraborty, Sucheta Chakraborty
In-Reply-To: <1294041525-15553-1-git-send-email-amit.salecha@qlogic.com>

From: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>

o Enable/disable LRO in device based on NETIF_F_LRO flag, instead of using
  driver private flag.
o Disable LRO, if rx csum offloading is off.

David Miller,
	You should use netdev_info() instead of dev_info().

Signed-off-by: Sucheta Chakraborty <sucheta.chakraborty@qlogic.com>
Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
---
 drivers/net/netxen/netxen_nic.h         |    1 +
 drivers/net/netxen/netxen_nic_ethtool.c |   26 ++++++++++++++++++++++++--
 drivers/net/netxen/netxen_nic_hw.c      |    5 -----
 drivers/net/netxen/netxen_nic_main.c    |    4 +---
 4 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/net/netxen/netxen_nic.h b/drivers/net/netxen/netxen_nic.h
index 8e8a978..4e54587 100644
--- a/drivers/net/netxen/netxen_nic.h
+++ b/drivers/net/netxen/netxen_nic.h
@@ -1132,6 +1132,7 @@ typedef struct {
 #define NETXEN_NIC_MSI_ENABLED		0x02
 #define NETXEN_NIC_MSIX_ENABLED		0x04
 #define NETXEN_NIC_LRO_ENABLED		0x08
+#define NETXEN_NIC_LRO_DISABLED		0x00
 #define NETXEN_NIC_BRIDGE_ENABLED       0X10
 #define NETXEN_NIC_DIAG_ENABLED		0x20
 #define NETXEN_IS_MSI_FAMILY(adapter) \
diff --git a/drivers/net/netxen/netxen_nic_ethtool.c b/drivers/net/netxen/netxen_nic_ethtool.c
index b30de24..587498e 100644
--- a/drivers/net/netxen/netxen_nic_ethtool.c
+++ b/drivers/net/netxen/netxen_nic_ethtool.c
@@ -720,7 +720,21 @@ static u32 netxen_nic_get_rx_csum(struct net_device *dev)
 static int netxen_nic_set_rx_csum(struct net_device *dev, u32 data)
 {
 	struct netxen_adapter *adapter = netdev_priv(dev);
-	adapter->rx_csum = !!data;
+
+	if (data) {
+		adapter->rx_csum = data;
+		return 0;
+	}
+
+	if (dev->features & NETIF_F_LRO) {
+		if (netxen_config_hw_lro(adapter, NETXEN_NIC_LRO_DISABLED))
+			return -EIO;
+
+		dev->features &= ~NETIF_F_LRO;
+		netxen_send_lro_cleanup(adapter);
+		netdev_info(dev, "disabling LRO as rx_csum is off\n");
+	}
+	adapter->rx_csum = data;
 	return 0;
 }
 
@@ -893,11 +907,19 @@ static int netxen_nic_set_flags(struct net_device *netdev, u32 data)
 	if (!(adapter->capabilities & NX_FW_CAPABILITY_HW_LRO))
 		return -EINVAL;
 
+	if (!adapter->rx_csum) {
+		netdev_info(netdev, "rx csum is off, cannot toggle LRO\n");
+		return -EINVAL;
+	}
+
+	if (!!(data & ETH_FLAG_LRO) == !!(netdev->features & NETIF_F_LRO))
+		return 0;
+
 	if (data & ETH_FLAG_LRO) {
 		hw_lro = NETXEN_NIC_LRO_ENABLED;
 		netdev->features |= NETIF_F_LRO;
 	} else {
-		hw_lro = 0;
+		hw_lro = NETXEN_NIC_LRO_DISABLED;
 		netdev->features &= ~NETIF_F_LRO;
 	}
 
diff --git a/drivers/net/netxen/netxen_nic_hw.c b/drivers/net/netxen/netxen_nic_hw.c
index e42d26e..5cef718 100644
--- a/drivers/net/netxen/netxen_nic_hw.c
+++ b/drivers/net/netxen/netxen_nic_hw.c
@@ -809,9 +809,6 @@ int netxen_config_hw_lro(struct netxen_adapter *adapter, int enable)
 	u64 word;
 	int rv = 0;
 
-	if ((adapter->flags & NETXEN_NIC_LRO_ENABLED) == enable)
-		return 0;
-
 	memset(&req, 0, sizeof(nx_nic_req_t));
 
 	req.qhdr = cpu_to_le64(NX_HOST_REQUEST << 23);
@@ -827,8 +824,6 @@ int netxen_config_hw_lro(struct netxen_adapter *adapter, int enable)
 			"configure hw lro request\n");
 	}
 
-	adapter->flags ^= NETXEN_NIC_LRO_ENABLED;
-
 	return rv;
 }
 
diff --git a/drivers/net/netxen/netxen_nic_main.c b/drivers/net/netxen/netxen_nic_main.c
index 58a3643..33fac32 100644
--- a/drivers/net/netxen/netxen_nic_main.c
+++ b/drivers/net/netxen/netxen_nic_main.c
@@ -762,8 +762,6 @@ netxen_check_options(struct netxen_adapter *adapter)
 	if (adapter->fw_version >= NETXEN_VERSION_CODE(4, 0, 222))
 		adapter->capabilities = NXRD32(adapter, CRB_FW_CAPABILITIES_1);
 
-	adapter->flags &= ~NETXEN_NIC_LRO_ENABLED;
-
 	if (adapter->ahw.port_type == NETXEN_NIC_XGBE) {
 		adapter->num_rxd = DEFAULT_RCV_DESCRIPTORS_10G;
 		adapter->num_jumbo_rxd = MAX_JUMBO_RCV_DESCRIPTORS_10G;
@@ -990,7 +988,7 @@ __netxen_nic_up(struct netxen_adapter *adapter, struct net_device *netdev)
 	if (NX_IS_REVISION_P3(adapter->ahw.revision_id))
 		netxen_config_intr_coalesce(adapter);
 
-	if (adapter->capabilities & NX_FW_CAPABILITY_HW_LRO)
+	if (netdev->features & NETIF_F_LRO)
 		netxen_config_hw_lro(adapter, NETXEN_NIC_LRO_ENABLED);
 
 	netxen_napi_enable(adapter);
-- 
1.7.3.2


^ permalink raw reply related

* [PATCHv2 0/2]netxen: updates
From: Amit Kumar Salecha @ 2011-01-03  7:58 UTC (permalink / raw)
  To: davem; +Cc: netdev, ameen.rahman, anirban.chakraborty


Hi,
 Series v2 of 2 patches to fix LRO in netxen nic driver. Apply these on
 net-next branch.

-Amit

^ permalink raw reply

* [PATCH] trivial: Fix typo fault in netdevice.h
From: Michal Simek @ 2011-01-03  8:54 UTC (permalink / raw)
  To: davem; +Cc: linux-kernel, netdev, Michal Simek

Signed-off-by: Michal Simek <monstr@monstr.eu>
---
 include/linux/netdevice.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index d8fd2c2..c773bc8 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -683,7 +683,7 @@ struct netdev_rx_queue {
  *	   neither operation.
  *
  * void (*ndo_vlan_rx_register)(struct net_device *dev, struct vlan_group *grp);
- *	If device support VLAN receive accleration
+ *	If device support VLAN receive acceleration
  *	(ie. dev->features & NETIF_F_HW_VLAN_RX), then this function is called
  *	when vlan groups for the device changes.  Note: grp is NULL
  *	if no vlan's groups are being used.
-- 
1.5.5.6

^ permalink raw reply related

* Re: [PATCH] new UDPCP Communication Protocol
From: Stefani Seibold @ 2011-01-03  9:08 UTC (permalink / raw)
  To: Jesper Juhl
  Cc: Eric Dumazet, linux-kernel, akpm, davem, netdev, shemminger,
	daniel.baluta, jochen
In-Reply-To: <alpine.LNX.2.00.1101022356550.11481@swampdragon.chaosbits.net>

Am Montag, den 03.01.2011, 00:04 +0100 schrieb Jesper Juhl:
> On Sun, 2 Jan 2011, Stefani Seibold wrote:
> 
> > Am Sonntag, den 02.01.2011, 23:49 +0100 schrieb Eric Dumazet:
> > > Le dimanche 02 janvier 2011 à 23:39 +0100, stefani@seibold.net a écrit :
> > > > +
> > > > +/*
> > > > + * Create a new destination descriptor for the given IPV4 address and port
> > > > + */
> > > > +static struct udpcp_dest *new_dest(struct sock *sk, __be32 addr, __be16 port)
> > > > +{
> > > > +	struct udpcp_dest *dest;
> > > > +	struct udpcp_sock *usk = udpcp_sk(sk);
> > > > +
> > > > +	if (usk->connections >= udpcp_max_connections)
> > > > +		return NULL;
> > > > +
> > > > +	dest = kzalloc(sizeof(*dest), sk->sk_allocation);
> > > > +
> > > > +	if (dest) {
> > > > +		usk->connections++;
> > > > +		skb_queue_head_init(&dest->xmit);
> > > > +		dest->addr = addr;
> > > > +		dest->port = port;
> > > > +		dest->ackmode = UDPCP_ACK;
> > > > +		list_add_tail(&dest->list, &usk->destlist);
> > > > +	}
> > > > +
> > > > +	return dest;
> > > > +}
> > > > +
> > > 
> > > Hmm, so 'connections' is increased, never decreased.
> > > 
> > > This seems a fatal flaw in this protocol, since a malicious user can
> > > easily fill the list with garbage, and block regular communications.
> > 
> > You are right, there is now way to detect which connection is no longer
> > needed. I have not designed this protocol, so i cannot fix it. 
> > 
> > But in our environment this will be used together with an firewall
> > and/or ipsec. In this case it it safe.
> > 
> 
> Hmm, the first thing that springs into my head as a possible band-aid 
> (which is probbaly wrong for many reasons I've not considered, so feel 
> free to shoot it down) is; couldn't we use a timer (set to some outrageous 
> high value by default and admin tunable) that would decrement 
> 'connections' (discount dead connections) when there has not been any 
> acctivity for a huge period of time? Kill off connections that have been 
> idle for ages.
> 

This will not work for two reasons:

- First there is no way to detect a dead connection. A connection can
stay for a very long time without data transfer.

- Second it will not save against a attack where all communication slots
will be eaten by an attacker and then new valid connections will be not
handled.

The only thing what is possible to make an ioctl call which allows the
user land client to cancel connections. 

But this will be in my opinion dead code, because white lists of trusted
address must be fostered and this will make the upgrading of a
infrastructure to complicate.

^ permalink raw reply

* Re: [*v3 PATCH 02/22] IPVS: netns to services part 1
From: Hans Schillstrom @ 2011-01-03  9:14 UTC (permalink / raw)
  To: Jan Engelhardt
  Cc: hans@schillstrom.com, horms@verge.net.au, ja@ssi.bg,
	daniel.lezcano@free.fr, wensong@linux-vs.org,
	lvs-devel@vger.kernel.org, netdev@vger.kernel.org,
	netfilter-devel@vger.kernel.org
In-Reply-To: <alpine.LNX.2.01.1101011547040.9065@obet.zrqbmnf.qr>

On Sat, 2011-01-01 at 15:57 +0100, Jan Engelhardt wrote:
> On Thursday 2010-12-30 11:50, hans@schillstrom.com wrote:
> >+/*
> >+ * Get net ptr from skb in traffic cases
> >+ * use skb_sknet when call is from userland (ioctl or netlink)
> >+ */
> >+static inline struct net *skb_net(struct sk_buff *skb) {
> >+#ifdef CONFIG_NET_NS
> >+#ifdef CONFIG_IP_VS_DEBUG
> >+	/*
> >+	 * This is used for debug only.
> >+	 * Start with the most likely hit
> >+	 * End with BUG
> >+	 */
> >+	if (likely(skb->dev && skb->dev->nd_net))
> >+		return dev_net(skb->dev);
> >+	if (skb_dst(skb)->dev)
> >+		return dev_net(skb_dst(skb)->dev);
> >+	WARN(skb->sk,"Maybe skb_sknet should be used instead in %s() line:%d\n",
> >+		      __func__, __LINE__);
> >+	if (likely(skb->sk && skb->sk->sk_net))
> >+		return sock_net(skb->sk);
> >+	pr_err("There is no net ptr to find in the skb in %s() line:%d\n",
> >+		__func__, __LINE__);
> >+	BUG();
> >+#else
> >+	return dev_net(skb->dev ? : skb_dst(skb)->dev);
> >+#endif
> >+#else
> >+	return &init_net;
> >+#endif
> >+}
> 
> Whether NETNS is disabled or not, dev_net(skb->dev) can be used in either case,
> so the extra return &init_net case is not really required AFAICS.

OK, but the debug part will not compile without the ifdef since there is
some symbols that is #ifdef CONFIG_NET_NS.
"&init_net" is faster than the " ? : "
And the rest I guess is obvious.

> 
> >@@ -3446,43 +3471,48 @@ static struct pernet_operations ipvs_control_ops = {
> >-	smp_wmb();
> >+
> >+	smp_wmb();	/* Do wee really need it now ? */
> 
> not "whee" :)



^ permalink raw reply

* Re: [PATCH] new UDPCP Communication Protocol
From: Eric Dumazet @ 2011-01-03  9:27 UTC (permalink / raw)
  To: Stefani Seibold
  Cc: Jesper Juhl, linux-kernel, akpm, davem, netdev, shemminger,
	daniel.baluta, jochen
In-Reply-To: <1294045732.19666.6.camel@wall-e>

Le lundi 03 janvier 2011 à 10:08 +0100, Stefani Seibold a écrit :

> This will not work for two reasons:
> 
> - First there is no way to detect a dead connection. A connection can
> stay for a very long time without data transfer.
> 
> - Second it will not save against a attack where all communication slots
> will be eaten by an attacker and then new valid connections will be not
> handled.
> 
> The only thing what is possible to make an ioctl call which allows the
> user land client to cancel connections. 
> 
> But this will be in my opinion dead code, because white lists of trusted
> address must be fostered and this will make the upgrading of a
> infrastructure to complicate.
> 
> 

Yep, and as UDP messages can easily spoofed, this means you need more
than a list of trusted addresses. You also need to encapsulate the thing
in an secured layer.

Stefani, your implementation has very litle chance being added in
standard kernel, because it is not correctly layered, or documented.

Copying hundred (thousand ?) of lines from existing code only shows
there is a design error in your proposal. It means every time we have to
make a change in this code, we'll have to do it twice.

SUNRPC uses UDP/TCP sockets, and use callbacks to existing UDP/TCP code,
maybe you should take a look to implement an UDPCP stack in kernel.

For instance, a pure socket API seems not the correct choice for UDPCP,
since a transmit should give a report to user, of frame being
delivered/aknowledged or not to/by the remote side ?

With send(), this means you have only one message in transit, no
asynchronous handling.

At least you forgot to document the API, and restrictions.

^ permalink raw reply

* Re: [PATCH V8 08/13] posix clocks: cleanup the CLOCK_DISPTACH macro
From: Peter Zijlstra @ 2011-01-03  9:29 UTC (permalink / raw)
  To: Richard Cochran
  Cc: linux-kernel, linux-api, netdev, Alan Cox, Arnd Bergmann,
	Christoph Lameter, David Miller, John Stultz, Krzysztof Halasa,
	Rodolfo Giometti, Thomas Gleixner
In-Reply-To: <503cd1fa268867573001cfc9bb5681ee3b5b32fa.1293820862.git.richard.cochran@omicron.at>

On Fri, 2010-12-31 at 20:15 +0100, Richard Cochran wrote:
> +#define CLOCK_DISPATCH(clock, call, arglist) dispatch_##call arglist

How about you run something like:

 :% s/CLOCK_DISPATCH([^,]*, \([^,]*\), \([^)]*)\))/dispatch_\1\2/g

and remove that cruft all together?

^ permalink raw reply

* Re: [PATCH 1/1 V3] bridge: fix br_multicast_ipv6_rcv for paged skbs
From: Johannes Berg @ 2011-01-03  9:34 UTC (permalink / raw)
  To: Tomas Winkler; +Cc: davem, netdev, Stephen Hemminger
In-Reply-To: <1293999538-9298-1-git-send-email-tomas.winkler@intel.com>

On Sun, 2011-01-02 at 22:18 +0200, Tomas Winkler wrote:

>  	icmp6h = icmp6_hdr(skb2);
>  
>  	switch (icmp6h->icmp6_type) {
> @@ -1516,7 +1517,12 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
>  	switch (icmp6h->icmp6_type) {
>  	case ICMPV6_MGM_REPORT:
>  	    {
> -		struct mld_msg *mld = (struct mld_msg *)icmp6h;
> +		struct mld_msg *mld;
> +		if (!pskb_may_pull(skb2, sizeof(*mld))) {
> +			err = -EINVAL;
> +			goto out;
> +		}
> +		mld = (struct mld_msg *)icmp6h;

This (and the second instance) is incorrect afaict -- the pointer
"icmp6h" should be reloaded after the pskb_may_pull(), no?

Also, the "out_nopush" thing is pointless since the push is completely
unnecessary as "skb2 != skb" is always true.

johannes


^ permalink raw reply

* RE: [PATCH 1/1 V3] bridge: fix br_multicast_ipv6_rcv for paged skbs
From: Winkler, Tomas @ 2011-01-03  9:43 UTC (permalink / raw)
  To: Johannes Berg
  Cc: davem@davemloft.net, netdev@vger.kernel.org, Stephen Hemminger
In-Reply-To: <1294047254.4165.1.camel@jlt3.sipsolutions.net>



> -----Original Message-----
> From: Johannes Berg [mailto:johannes@sipsolutions.net]
> Sent: Monday, January 03, 2011 11:34 AM
> To: Winkler, Tomas
> Cc: davem@davemloft.net; netdev@vger.kernel.org; Stephen Hemminger
> Subject: Re: [PATCH 1/1 V3] bridge: fix br_multicast_ipv6_rcv for paged skbs
> 
> On Sun, 2011-01-02 at 22:18 +0200, Tomas Winkler wrote:
> 
> >  	icmp6h = icmp6_hdr(skb2);
> >
> >  	switch (icmp6h->icmp6_type) {
> > @@ -1516,7 +1517,12 @@ static int br_multicast_ipv6_rcv(struct net_bridge
> *br,
> >  	switch (icmp6h->icmp6_type) {
> >  	case ICMPV6_MGM_REPORT:
> >  	    {
> > -		struct mld_msg *mld = (struct mld_msg *)icmp6h;
> > +		struct mld_msg *mld;
> > +		if (!pskb_may_pull(skb2, sizeof(*mld))) {
> > +			err = -EINVAL;
> > +			goto out;
> > +		}
> > +		mld = (struct mld_msg *)icmp6h;
> 
> This (and the second instance) is incorrect afaict -- the pointer
> "icmp6h" should be reloaded after the pskb_may_pull(), no?

mld_msg is bigger than icmp6h by sizeof(in6_addr) so we have to try pull again a bigger chunk. 

> 
> Also, the "out_nopush" thing is pointless since the push is completely
> unnecessary as "skb2 != skb" is always true.

You are right if skb_clone doesn't return the same pointer then yes. Shame, but I'm not a sbk expert. I'm diving into it now.

Thanks
Tomas
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

^ permalink raw reply

* Re: [PATCH] new UDPCP Communication Protocol
From: Stefani Seibold @ 2011-01-03  9:54 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Jesper Juhl, linux-kernel, akpm, davem, netdev, shemminger,
	daniel.baluta, jochen
In-Reply-To: <1294046867.2892.101.camel@edumazet-laptop>

Am Montag, den 03.01.2011, 10:27 +0100 schrieb Eric Dumazet:
> Le lundi 03 janvier 2011 à 10:08 +0100, Stefani Seibold a écrit :
> 
> > This will not work for two reasons:
> > 
> > - First there is no way to detect a dead connection. A connection can
> > stay for a very long time without data transfer.
> > 
> > - Second it will not save against a attack where all communication slots
> > will be eaten by an attacker and then new valid connections will be not
> > handled.
> > 
> > The only thing what is possible to make an ioctl call which allows the
> > user land client to cancel connections. 
> > 
> > But this will be in my opinion dead code, because white lists of trusted
> > address must be fostered and this will make the upgrading of a
> > infrastructure to complicate.
> > 
> > 
> 
> Yep, and as UDP messages can easily spoofed, this means you need more
> than a list of trusted addresses. You also need to encapsulate the thing
> in an secured layer.
> 
> Stefani, your implementation has very litle chance being added in
> standard kernel, because it is not correctly layered, or documented.
> 
> Copying hundred (thousand ?) of lines from existing code only shows
> there is a design error in your proposal. It means every time we have to
> make a change in this code, we'll have to do it twice.
> 

I copied about 400 of 3000 lines with was heavy modified to need my
needs. And i use only document features of the linux IP stack. So it is
normal to have duplicate code for the basics.

How can you do a routing, how can you determinate the MTU of the route.
This are basics. Look into other code how this things will be handled is
in my opinion the right way, since there a no function provide to do
this.

Otherwise you can say the same about all the filesystem or PCI
drvivers , which do also a lot in the same way. But since this is the
way to do it, it is the right way.

> SUNRPC uses UDP/TCP sockets, and use callbacks to existing UDP/TCP code,
> maybe you should take a look to implement an UDPCP stack in kernel.
> 

I have looked around the whole LINUX source code, also in the SUNRPC
sockets and i did not found anything which meet my needs.

> For instance, a pure socket API seems not the correct choice for UDPCP,
> since a transmit should give a report to user, of frame being
> delivered/aknowledged or not to/by the remote side ?
> 

This will be done through the error queue. The user client will receive
the unhandled packets back.

> With send(), this means you have only one message in transit, no
> asynchronous handling.
> 

No, the messages will be queued. You can have more than a messages in
the send queue.

> At least you forgot to document the API, and restrictions.
> 

API documentation is still there, i can these provide under
Documentation/udpcp.txt if you like.

Here is the API documentation:

Socket interface programming manual

The socket interface is a derivate of the UDP sockets. All setsockopt(),
getsockopt() and ioctl() kernel system calls  which are valid for UDP
sockets should work on UDPCP sockets. There are some extensions to the
sockopt and ioctl interface for the UDPCP sockets.

Include the C header file <net/udpcp.h> to use the UDPCP socket options
and ioctl calls.

A UDPCP can be opened with socket(PF_INET, SOCK_DGRAM, PF_UDPCP). All
operation which are valid for UDP sockets can also performed with UDPCP
sockets.

sockopt

The setsockopt and getsockopt are defined as following:

int getsockopt(int sockfd, int level, int optname, void *optval,
socklen_t *optlen);

int setsockopt(int sockfd, int level, int optname, const void *optval,
socklen_t optlen);

The level parameter for the UDPCP socket is SOL_UDPCP, where the
following options are defined:

UDPCP_OPT_TRANSFER_MODE - set default transfer mode. The optval is one
of the following:

UDPCP_NOACK - no ACK for the transmitted message is requiered

UDPCP_ACK - a ACK for each transmitted message fragment is requiered

UDPCP_SINGLE_ACK - only a ACK for the last transmitted message fragment
is requiered

UDPCP_OPT_CHECKSUM_MODE - set the default checksum mode. The optval is
one of the following:

UDPCP_NOCHECKSUM - no checksum for the transmitted message is required

UDPCP_CHECKSUM - a checksum test for the transmitted message is required

UDPCP_OPT_TX_TIMEOUT - the timeout for a awaited ACK in milliseconds.

The optval should between >= 1 and max. UDPCP_MAX_WAIT_SEC * 1000

UDPCP_OPT_RX_TIMEOUT - timeout for a outstanding incoming message
fragment in milliseconds.

The optval should between >= 1 and max. UDPCP_MAX_WAIT_SEC * 1000

UDPCP_OPT_MAXTRY - the number of tries to send a message fragment.

The optval should between >= 1 and <= 10

UDPCP_OPT_OUTSTANDING_ACKS: the number of outstanding acks.

The optval should between >=1 and <= 255

All optlen parameters are int's. There the optlen should be
sizeof(optlen).

The values UDPCP_NOACK, UDPCP_ACK, UDPCP_SINGLE_ACK, UDPCP_NOCHECKSUM
and

UDPCP_CHECKSUM can also passed as control message with sendmsg(). For
details look at the manual page for sendmsg().

ioctl interface

The ioctl function call is defined as

int ioctl(int d, int request, ...)

For UDPCP sockets there are the following request commands defined:

UDPCP_IOCTL_GET_STATISTICS

This command returns the statistics of the socket in a struct
udpcp_statistics. The address of this struct must be passed as third
argument.

UDPCP_IOCTL_RESET_STATISTICS

This command resets the statistics of the socket

UDPCP_IOCTL_SYNC 

This command waits until all message fragments are transmitted. If the
third argument is not zero, this is the max. timeout value in
milliseconds, otherwise this call can block indefinitely.

^ permalink raw reply

* RE: [PATCH 1/1 V3] bridge: fix br_multicast_ipv6_rcv for paged skbs
From: Johannes Berg @ 2011-01-03 10:04 UTC (permalink / raw)
  To: Winkler, Tomas
  Cc: davem@davemloft.net, netdev@vger.kernel.org, Stephen Hemminger
In-Reply-To: <6F5C1D715B2DA5498A628E6B9C124F04019BF9E404@hasmsx504.ger.corp.intel.com>

On Mon, 2011-01-03 at 11:43 +0200, Winkler, Tomas wrote:

> > > -		struct mld_msg *mld = (struct mld_msg *)icmp6h;
> > > +		struct mld_msg *mld;
> > > +		if (!pskb_may_pull(skb2, sizeof(*mld))) {
> > > +			err = -EINVAL;
> > > +			goto out;
> > > +		}
> > > +		mld = (struct mld_msg *)icmp6h;
> > 
> > This (and the second instance) is incorrect afaict -- the pointer
> > "icmp6h" should be reloaded after the pskb_may_pull(), no?
> 
> mld_msg is bigger than icmp6h by sizeof(in6_addr) so we have to try pull again a bigger chunk. 

Right, I know, the pskb_may_pull() is needed, but I believe you need to
re-calculate icmp6h here.

> > Also, the "out_nopush" thing is pointless since the push is completely
> > unnecessary as "skb2 != skb" is always true.
> 
> You are right if skb_clone doesn't return the same pointer then yes.
> Shame, but I'm not a sbk expert. I'm diving into it now.

I'm pretty sure it's guaranteed to return a new pointer.

johannes


^ permalink raw reply

* RE: [PATCH 1/1 V3] bridge: fix br_multicast_ipv6_rcv for paged skbs
From: Winkler, Tomas @ 2011-01-03 10:17 UTC (permalink / raw)
  To: Johannes Berg
  Cc: davem@davemloft.net, netdev@vger.kernel.org, Stephen Hemminger
In-Reply-To: <1294049058.4165.3.camel@jlt3.sipsolutions.net>



> -----Original Message-----
> From: Johannes Berg [mailto:johannes@sipsolutions.net]
> Sent: Monday, January 03, 2011 12:04 PM
> To: Winkler, Tomas
> Cc: davem@davemloft.net; netdev@vger.kernel.org; Stephen Hemminger
> Subject: RE: [PATCH 1/1 V3] bridge: fix br_multicast_ipv6_rcv for paged skbs
> 
> On Mon, 2011-01-03 at 11:43 +0200, Winkler, Tomas wrote:
> 
> > > > -		struct mld_msg *mld = (struct mld_msg *)icmp6h;
> > > > +		struct mld_msg *mld;
> > > > +		if (!pskb_may_pull(skb2, sizeof(*mld))) {
> > > > +			err = -EINVAL;
> > > > +			goto out;
> > > > +		}
> > > > +		mld = (struct mld_msg *)icmp6h;
> > >
> > > This (and the second instance) is incorrect afaict -- the pointer
> > > "icmp6h" should be reloaded after the pskb_may_pull(), no?
> >
> > mld_msg is bigger than icmp6h by sizeof(in6_addr) so we have to try pull
> again a bigger chunk.
> 
> Right, I know, the pskb_may_pull() is needed, but I believe you need to
> re-calculate icmp6h here.

You are right, it can be moved to new memory buffer.

Probably something like that will do it:

if (!pskb_may_pull(skb2, sizeof(*mld))) {
	err = -EINVAL;
	goto out;
}
mld = (struct mld_msg *)skb_transport_header(skb2) 

> 
> > > Also, the "out_nopush" thing is pointless since the push is completely
> > > unnecessary as "skb2 != skb" is always true.
> >
> > You are right if skb_clone doesn't return the same pointer then yes.
> > Shame, but I'm not a sbk expert. I'm diving into it now.
> 
> I'm pretty sure it's guaranteed to return a new pointer.

Right, it either returns skb + 1 or new one from the cache. We can drop the nopush section.

Thanks
Tomas
---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

^ permalink raw reply

* [PATCH] ll_temac: Fix section mismatch from the temac_of_probe
From: Michal Simek @ 2011-01-03 10:32 UTC (permalink / raw)
  To: grant.likely-s3s/WqlpOiPyB63q8FvJNQ
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, davem-fT/PcQaiUtIeIZ0/mPfg9Q

Replace __init by __devinit.

Warning message:
WARNING: vmlinux.o(.data+0xbc14): Section mismatch in reference from the variable
temac_of_driver to the function .init.text:temac_of_probe()
The variable temac_of_driver references
the function __init temac_of_probe()
If the reference is valid then annotate the
variable with __init* or __refdata (see linux/init.h) or name the variable:
*_template, *_timer, *_sht, *_ops, *_probe, *_probe_one, *_console,

Signed-off-by: Michal Simek <monstr-pSz03upnqPeHXe+LvDLADg@public.gmane.org>
---
 drivers/net/ll_temac_main.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ll_temac_main.c b/drivers/net/ll_temac_main.c
index 9f8e702..beb6ed8 100644
--- a/drivers/net/ll_temac_main.c
+++ b/drivers/net/ll_temac_main.c
@@ -952,7 +952,7 @@ static const struct attribute_group temac_attr_group = {
 	.attrs = temac_device_attrs,
 };
 
-static int __init
+static int __devinit
 temac_of_probe(struct platform_device *op, const struct of_device_id *match)
 {
 	struct device_node *np;
-- 
1.5.5.6

^ permalink raw reply related

* [PATCH V4] bridge: fix br_multicast_ipv6_rcv for paged skbs
From: Tomas Winkler @ 2011-01-03 10:38 UTC (permalink / raw)
  To: davem; +Cc: netdev, Tomas Winkler, Johannes Berg, Stephen Hemminger
In-Reply-To: <1294051080-29492-1-git-send-email-tomas.winkler@intel.com>

use pskb_may_pull to access ipv6 header correctly for paged skbs
It was omitted in the bridge code leading to crash in blind
__skb_pull

since the skb is cloned undonditionally we also simplify the
the exit path

this fixes bug https://bugzilla.kernel.org/show_bug.cgi?id=25202

Dec 15 14:36:40 User-PC hostapd: wlan0: STA 00:15:00:60:5d:34 IEEE 802.11: authenticated
Dec 15 14:36:40 User-PC hostapd: wlan0: STA 00:15:00:60:5d:34 IEEE 802.11: associated (aid 2)
Dec 15 14:36:40 User-PC hostapd: wlan0: STA 00:15:00:60:5d:34 RADIUS: starting accounting session 4D0608A3-00000005
Dec 15 14:36:41 User-PC kernel: [175576.120287] ------------[ cut here ]------------
Dec 15 14:36:41 User-PC kernel: [175576.120452] kernel BUG at include/linux/skbuff.h:1178!
Dec 15 14:36:41 User-PC kernel: [175576.120609] invalid opcode: 0000 [#1] SMP
Dec 15 14:36:41 User-PC kernel: [175576.120749] last sysfs file: /sys/devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/block/sda/uevent
Dec 15 14:36:41 User-PC kernel: [175576.121035] Modules linked in: oprofile binfmt_misc bridge stp llc parport_pc ppdev arc4 iwlagn snd_hda_codec_realtek iwlcore i915 snd_hda_intel mac80211 joydev snd_hda_codec snd_hwdep snd_pcm snd_seq_midi drm_kms_helper snd_rawmidi drm snd_seq_midi_event snd_seq snd_timer snd_seq_device cfg80211 eeepc_wmi usbhid psmouse intel_agp i2c_algo_bit intel_gtt uvcvideo agpgart videodev sparse_keymap snd shpchp v4l1_compat lp hid video serio_raw soundcore output snd_page_alloc ahci libahci atl1c
Dec 15 14:36:41 User-PC kernel: [175576.122712]
Dec 15 14:36:41 User-PC kernel: [175576.122769] Pid: 0, comm: kworker/0:0 Tainted: G        W   2.6.37-rc5-wl+ #3 1015PE/1016P
Dec 15 14:36:41 User-PC kernel: [175576.123012] EIP: 0060:[<f83edd65>] EFLAGS: 00010283 CPU: 1
Dec 15 14:36:41 User-PC kernel: [175576.123193] EIP is at br_multicast_rcv+0xc95/0xe1c [bridge]
Dec 15 14:36:41 User-PC kernel: [175576.123362] EAX: 0000001c EBX: f5626318 ECX: 00000000 EDX: 00000000
Dec 15 14:36:41 User-PC kernel: [175576.123550] ESI: ec512262 EDI: f5626180 EBP: f60b5ca0 ESP: f60b5bd8
Dec 15 14:36:41 User-PC kernel: [175576.123737]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
Dec 15 14:36:41 User-PC kernel: [175576.123902] Process kworker/0:0 (pid: 0, ti=f60b4000 task=f60a8000 task.ti=f60b0000)
Dec 15 14:36:41 User-PC kernel: [175576.124137] Stack:
Dec 15 14:36:41 User-PC kernel: [175576.124181]  ec556500 f6d06800 f60b5be8 c01087d8 ec512262 00000030 00000024 f5626180
Dec 15 14:36:41 User-PC kernel: [175576.124181]  f572c200 ef463440 f5626300 3affffff f6d06dd0 e60766a4 000000c4 f6d06860
Dec 15 14:36:41 User-PC kernel: [175576.124181]  ffffffff ec55652c 00000001 f6d06844 f60b5c64 c0138264 c016e451 c013e47d
Dec 15 14:36:41 User-PC kernel: [175576.124181] Call Trace:
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c01087d8>] ? sched_clock+0x8/0x10
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c0138264>] ? enqueue_entity+0x174/0x440
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c016e451>] ? sched_clock_cpu+0x131/0x190
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c013e47d>] ? select_task_rq_fair+0x2ad/0x730
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c0524fc1>] ? nf_iterate+0x71/0x90
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f83e4914>] ? br_handle_frame_finish+0x184/0x220 [bridge]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f83e4790>] ? br_handle_frame_finish+0x0/0x220 [bridge]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f83e46e9>] ? br_handle_frame+0x189/0x230 [bridge]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f83e4790>] ? br_handle_frame_finish+0x0/0x220 [bridge]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f83e4560>] ? br_handle_frame+0x0/0x230 [bridge]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c04ff026>] ? __netif_receive_skb+0x1b6/0x5b0
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c04f7a30>] ? skb_copy_bits+0x110/0x210
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c0503a7f>] ? netif_receive_skb+0x6f/0x80
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f82cb74c>] ? ieee80211_deliver_skb+0x8c/0x1a0 [mac80211]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f82cc836>] ? ieee80211_rx_handlers+0xeb6/0x1aa0 [mac80211]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c04ff1f0>] ? __netif_receive_skb+0x380/0x5b0
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c016e242>] ? sched_clock_local+0xb2/0x190
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c012b688>] ? default_spin_lock_flags+0x8/0x10
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c05d83df>] ? _raw_spin_lock_irqsave+0x2f/0x50
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f82cd621>] ? ieee80211_prepare_and_rx_handle+0x201/0xa90 [mac80211]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f82ce154>] ? ieee80211_rx+0x2a4/0x830 [mac80211]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f815a8d6>] ? iwl_update_stats+0xa6/0x2a0 [iwlcore]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f8499212>] ? iwlagn_rx_reply_rx+0x292/0x3b0 [iwlagn]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c05d83df>] ? _raw_spin_lock_irqsave+0x2f/0x50
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f8483697>] ? iwl_rx_handle+0xe7/0x350 [iwlagn]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f8486ab7>] ? iwl_irq_tasklet+0xf7/0x5c0 [iwlagn]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c01aece1>] ? __rcu_process_callbacks+0x201/0x2d0
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c0150d05>] ? tasklet_action+0xc5/0x100
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c0150a07>] ? __do_softirq+0x97/0x1d0
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c05d910c>] ? nmi_stack_correct+0x2f/0x34
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c0150970>] ? __do_softirq+0x0/0x1d0
Dec 15 14:36:41 User-PC kernel: [175576.124181]  <IRQ>
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c01508f5>] ? irq_exit+0x65/0x70
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c05df062>] ? do_IRQ+0x52/0xc0
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c01036b0>] ? common_interrupt+0x30/0x38
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c03a1fc2>] ? intel_idle+0xc2/0x160
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c04daebb>] ? cpuidle_idle_call+0x6b/0x100
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c0101dea>] ? cpu_idle+0x8a/0xf0
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c05d2702>] ? start_secondary+0x1e8/0x1ee

Cc: David Miller <davem@davemloft.net>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
V2: Implement David Miller's suggestion 
V3: Fix the mld_msg access:
	Length check was wrong and psk_may_pull performs itself the length check
V4: mld_msg pointer need to be recalculated after pull
    simplify the exit path

 net/bridge/br_multicast.c |   29 +++++++++++++++++++----------
 1 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index f19e347..67296b7 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1430,7 +1430,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
 				 struct net_bridge_port *port,
 				 struct sk_buff *skb)
 {
-	struct sk_buff *skb2 = skb;
+	struct sk_buff *skb2;
 	struct ipv6hdr *ip6h;
 	struct icmp6hdr *icmp6h;
 	u8 nexthdr;
@@ -1469,15 +1469,16 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
 	if (!skb2)
 		return -ENOMEM;
 
+
+	err = -EINVAL;
+	if (!pskb_may_pull(skb2, offset + sizeof(struct icmp6hdr)))
+		goto out;
+
 	len -= offset - skb_network_offset(skb2);
 
 	__skb_pull(skb2, offset);
 	skb_reset_transport_header(skb2);
 
-	err = -EINVAL;
-	if (!pskb_may_pull(skb2, sizeof(*icmp6h)))
-		goto out;
-
 	icmp6h = icmp6_hdr(skb2);
 
 	switch (icmp6h->icmp6_type) {
@@ -1516,7 +1517,12 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
 	switch (icmp6h->icmp6_type) {
 	case ICMPV6_MGM_REPORT:
 	    {
-		struct mld_msg *mld = (struct mld_msg *)icmp6h;
+		struct mld_msg *mld;
+		if (!pskb_may_pull(skb2, sizeof(*mld))) {
+			err = -EINVAL;
+			goto out;
+		}
+		mld = (struct mld_msg *)skb_transport_header(skb2);
 		BR_INPUT_SKB_CB(skb2)->mrouters_only = 1;
 		err = br_ip6_multicast_add_group(br, port, &mld->mld_mca);
 		break;
@@ -1529,15 +1535,18 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
 		break;
 	case ICMPV6_MGM_REDUCTION:
 	    {
-		struct mld_msg *mld = (struct mld_msg *)icmp6h;
+		struct mld_msg *mld;
+		if (!pskb_may_pull(skb2, sizeof(*mld))) {
+			err = -EINVAL;
+			goto out;
+		}
+		mld = (struct mld_msg *)skb_transport_header(skb2);
 		br_ip6_multicast_leave_group(br, port, &mld->mld_mca);
 	    }
 	}
 
 out:
-	__skb_push(skb2, offset);
-	if (skb2 != skb)
-		kfree_skb(skb2);
+	kfree_skb(skb2);
 	return err;
 }
 #endif
-- 
1.7.3.4

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related

* [PATCH V4] bridge: fix br_multicast_ipv6_rcv for paged skbs
From: Tomas Winkler @ 2011-01-03 10:37 UTC (permalink / raw)
  To: davem; +Cc: netdev, Tomas Winkler, Johannes Berg, Stephen Hemminger

use pskb_may_pull to access ipv6 header correctly for paged skbs
It was omitted in the bridge code leading to crash in blind
__skb_pull

since the skb is cloned undonditionally we also simplify the
the exit path

this fixes bug https://bugzilla.kernel.org/show_bug.cgi?id=25202

Dec 15 14:36:40 User-PC hostapd: wlan0: STA 00:15:00:60:5d:34 IEEE 802.11: authenticated
Dec 15 14:36:40 User-PC hostapd: wlan0: STA 00:15:00:60:5d:34 IEEE 802.11: associated (aid 2)
Dec 15 14:36:40 User-PC hostapd: wlan0: STA 00:15:00:60:5d:34 RADIUS: starting accounting session 4D0608A3-00000005
Dec 15 14:36:41 User-PC kernel: [175576.120287] ------------[ cut here ]------------
Dec 15 14:36:41 User-PC kernel: [175576.120452] kernel BUG at include/linux/skbuff.h:1178!
Dec 15 14:36:41 User-PC kernel: [175576.120609] invalid opcode: 0000 [#1] SMP
Dec 15 14:36:41 User-PC kernel: [175576.120749] last sysfs file: /sys/devices/pci0000:00/0000:00:1f.2/host0/target0:0:0/0:0:0:0/block/sda/uevent
Dec 15 14:36:41 User-PC kernel: [175576.121035] Modules linked in: approvals binfmt_misc bridge stp llc parport_pc ppdev arc4 iwlagn snd_hda_codec_realtek iwlcore i915 snd_hda_intel mac80211 joydev snd_hda_codec snd_hwdep snd_pcm snd_seq_midi drm_kms_helper snd_rawmidi drm snd_seq_midi_event snd_seq snd_timer snd_seq_device cfg80211 eeepc_wmi usbhid psmouse intel_agp i2c_algo_bit intel_gtt uvcvideo agpgart videodev sparse_keymap snd shpchp v4l1_compat lp hid video serio_raw soundcore output snd_page_alloc ahci libahci atl1c
Dec 15 14:36:41 User-PC kernel: [175576.122712]
Dec 15 14:36:41 User-PC kernel: [175576.122769] Pid: 0, comm: kworker/0:0 Tainted: G        W   2.6.37-rc5-wl+ #3 1015PE/1016P
Dec 15 14:36:41 User-PC kernel: [175576.123012] EIP: 0060:[<f83edd65>] EFLAGS: 00010283 CPU: 1
Dec 15 14:36:41 User-PC kernel: [175576.123193] EIP is at br_multicast_rcv+0xc95/0xe1c [bridge]
Dec 15 14:36:41 User-PC kernel: [175576.123362] EAX: 0000001c EBX: f5626318 ECX: 00000000 EDX: 00000000
Dec 15 14:36:41 User-PC kernel: [175576.123550] ESI: ec512262 EDI: f5626180 EBP: f60b5ca0 ESP: f60b5bd8
Dec 15 14:36:41 User-PC kernel: [175576.123737]  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
Dec 15 14:36:41 User-PC kernel: [175576.123902] Process kworker/0:0 (pid: 0, ti=f60b4000 task=f60a8000 task.ti=f60b0000)
Dec 15 14:36:41 User-PC kernel: [175576.124137] Stack:
Dec 15 14:36:41 User-PC kernel: [175576.124181]  ec556500 f6d06800 f60b5be8 c01087d8 ec512262 00000030 00000024 f5626180
Dec 15 14:36:41 User-PC kernel: [175576.124181]  f572c200 ef463440 f5626300 3affffff f6d06dd0 e60766a4 000000c4 f6d06860
Dec 15 14:36:41 User-PC kernel: [175576.124181]  ffffffff ec55652c 00000001 f6d06844 f60b5c64 c0138264 c016e451 c013e47d
Dec 15 14:36:41 User-PC kernel: [175576.124181] Call Trace:
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c01087d8>] ? sched_clock+0x8/0x10
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c0138264>] ? enqueue_entity+0x174/0x440
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c016e451>] ? sched_clock_cpu+0x131/0x190
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c013e47d>] ? select_task_rq_fair+0x2ad/0x730
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c0524fc1>] ? nf_iterate+0x71/0x90
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f83e4914>] ? br_handle_frame_finish+0x184/0x220 [bridge]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f83e4790>] ? br_handle_frame_finish+0x0/0x220 [bridge]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f83e46e9>] ? br_handle_frame+0x189/0x230 [bridge]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f83e4790>] ? br_handle_frame_finish+0x0/0x220 [bridge]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f83e4560>] ? br_handle_frame+0x0/0x230 [bridge]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c04ff026>] ? __netif_receive_skb+0x1b6/0x5b0
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c04f7a30>] ? skb_copy_bits+0x110/0x210
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c0503a7f>] ? netif_receive_skb+0x6f/0x80
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f82cb74c>] ? ieee80211_deliver_skb+0x8c/0x1a0 [mac80211]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f82cc836>] ? ieee80211_rx_handlers+0xeb6/0x1aa0 [mac80211]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c04ff1f0>] ? __netif_receive_skb+0x380/0x5b0
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c016e242>] ? sched_clock_local+0xb2/0x190
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c012b688>] ? default_spin_lock_flags+0x8/0x10
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c05d83df>] ? _raw_spin_lock_irqsave+0x2f/0x50
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f82cd621>] ? ieee80211_prepare_and_rx_handle+0x201/0xa90 [mac80211]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f82ce154>] ? ieee80211_rx+0x2a4/0x830 [mac80211]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f815a8d6>] ? iwl_update_stats+0xa6/0x2a0 [iwlcore]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f8499212>] ? iwlagn_rx_reply_rx+0x292/0x3b0 [iwlagn]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c05d83df>] ? _raw_spin_lock_irqsave+0x2f/0x50
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f8483697>] ? iwl_rx_handle+0xe7/0x350 [iwlagn]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<f8486ab7>] ? iwl_irq_tasklet+0xf7/0x5c0 [iwlagn]
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c01aece1>] ? __rcu_process_callbacks+0x201/0x2d0
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c0150d05>] ? tasklet_action+0xc5/0x100
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c0150a07>] ? __do_softirq+0x97/0x1d0
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c05d910c>] ? nmi_stack_correct+0x2f/0x34
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c0150970>] ? __do_softirq+0x0/0x1d0
Dec 15 14:36:41 User-PC kernel: [175576.124181]  <IRQ>
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c01508f5>] ? irq_exit+0x65/0x70
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c05df062>] ? do_IRQ+0x52/0xc0
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c01036b0>] ? common_interrupt+0x30/0x38
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c03a1fc2>] ? intel_idle+0xc2/0x160
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c04daebb>] ? cpuidle_idle_call+0x6b/0x100
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c0101dea>] ? cpu_idle+0x8a/0xf0
Dec 15 14:36:41 User-PC kernel: [175576.124181]  [<c05d2702>] ? start_secondary+0x1e8/0x1ee

Cc: David Miller <davem@davemloft.net>
Cc: Johannes Berg <johannes@sipsolutions.net>
Cc: Stephen Hemminger <shemminger@vyatta.com>
Signed-off-by: Tomas Winkler <tomas.winkler@intel.com>
---
V2: Implement David Miller's suggestion 
V3: Fix the mld_msg access:
	Length check was wrong and psk_may_pull performs itself the length check
V4: mld_msg pointer need to be recalculated after pull
    simplify the exit path

 net/bridge/br_multicast.c |   29 +++++++++++++++++++----------
 1 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/net/bridge/br_multicast.c b/net/bridge/br_multicast.c
index f19e347..67296b7 100644
--- a/net/bridge/br_multicast.c
+++ b/net/bridge/br_multicast.c
@@ -1430,7 +1430,7 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
 				 struct net_bridge_port *port,
 				 struct sk_buff *skb)
 {
-	struct sk_buff *skb2 = skb;
+	struct sk_buff *skb2;
 	struct ipv6hdr *ip6h;
 	struct icmp6hdr *icmp6h;
 	u8 nexthdr;
@@ -1469,15 +1469,16 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
 	if (!skb2)
 		return -ENOMEM;
 
+
+	err = -EINVAL;
+	if (!pskb_may_pull(skb2, offset + sizeof(struct icmp6hdr)))
+		goto out;
+
 	len -= offset - skb_network_offset(skb2);
 
 	__skb_pull(skb2, offset);
 	skb_reset_transport_header(skb2);
 
-	err = -EINVAL;
-	if (!pskb_may_pull(skb2, sizeof(*icmp6h)))
-		goto out;
-
 	icmp6h = icmp6_hdr(skb2);
 
 	switch (icmp6h->icmp6_type) {
@@ -1516,7 +1517,12 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
 	switch (icmp6h->icmp6_type) {
 	case ICMPV6_MGM_REPORT:
 	    {
-		struct mld_msg *mld = (struct mld_msg *)icmp6h;
+		struct mld_msg *mld;
+		if (!pskb_may_pull(skb2, sizeof(*mld))) {
+			err = -EINVAL;
+			goto out;
+		}
+		mld = (struct mld_msg *)skb_transport_header(skb2);
 		BR_INPUT_SKB_CB(skb2)->mrouters_only = 1;
 		err = br_ip6_multicast_add_group(br, port, &mld->mld_mca);
 		break;
@@ -1529,15 +1535,18 @@ static int br_multicast_ipv6_rcv(struct net_bridge *br,
 		break;
 	case ICMPV6_MGM_REDUCTION:
 	    {
-		struct mld_msg *mld = (struct mld_msg *)icmp6h;
+		struct mld_msg *mld;
+		if (!pskb_may_pull(skb2, sizeof(*mld))) {
+			err = -EINVAL;
+			goto out;
+		}
+		mld = (struct mld_msg *)skb_transport_header(skb2);
 		br_ip6_multicast_leave_group(br, port, &mld->mld_mca);
 	    }
 	}
 
 out:
-	__skb_push(skb2, offset);
-	if (skb2 != skb)
-		kfree_skb(skb2);
+	kfree_skb(skb2);
 	return err;
 }
 #endif
-- 
1.7.3.4

---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


^ permalink raw reply related


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