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

WARNING: drivers/net/ksz884x.o(.data+0x18): Section mismatch in reference from the variable pci_device_driver to the function .init.text:pcidev_init()
The variable pci_device_driver references
the function __init pcidev_init()
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/ksz884x.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/ksz884x.c b/drivers/net/ksz884x.c
index 49ea870..515b0d7 100644
--- a/drivers/net/ksz884x.c
+++ b/drivers/net/ksz884x.c
@@ -7277,7 +7277,7 @@ static struct pci_device_id pcidev_table[] = {
 
 MODULE_DEVICE_TABLE(pci, pcidev_table);
 
-static struct pci_driver pci_device_driver = {
+static struct pci_driver pci_device_driver __refdata = {
 #ifdef CONFIG_PM
 	.suspend	= pcidev_suspend,
 	.resume		= pcidev_resume,
-- 
1.7.4.rc0

^ permalink raw reply related

* Re: [net-next-2.6 PATCH v2 0/3] Series short description
From: John Fastabend @ 2011-01-03  2:27 UTC (permalink / raw)
  To: David Miller; +Cc: netdev@vger.kernel.org, Liu, Lucy, shmulikr@broadcom.com
In-Reply-To: <20101231.104941.189703153.davem@davemloft.net>

On 12/31/2010 10:49 AM, David Miller wrote:
> From: John Fastabend <john.r.fastabend@intel.com>
> Date: Thu, 30 Dec 2010 11:25:40 -0800
> 
>> Version 2 adds the recommended tlv attributes to the ets struct this will
>> be needed for offloaded LLDP engines. Host controlled LLDP agents will most
>> likely not use these fields.
>>
>> Also Shmulik Ravid caught an error with a spin_lock on kmalloc failure.
> 
> BTW, even though I replied "applied" to v1 of these patches I made
> sure to actully apply v2 :-)
> --

Thanks David.

^ permalink raw reply

* [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


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