* Re: [PATCH] drivers/net/wireless/ath/wil6210: Makefile, only -Werror when no -W* in EXTRA_CFLAGS
From: Vladimir Kondratiev @ 2013-02-27 9:46 UTC (permalink / raw)
To: Chen Gang
Cc: John W. Linville, linux-wireless, wil6210,
netdev@vger.kernel.org >> netdev,
linux-kernel@vger.kernel.org
In-Reply-To: <512DCA59.4040800@asianux.com>
On Wednesday, February 27, 2013 04:56:57 PM Chen Gang wrote:
> 于 2013年02月27日 16:45, Vladimir Kondratiev 写道:
> > On Wednesday, February 27, 2013 02:55:06 PM Chen Gang wrote:
> >>
> >> When make with EXTRA_CFLAGS=-W, it will report error.
> >> so give a check in Makefile.
> >>
> >> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> >> ---
> >> drivers/net/wireless/ath/wil6210/Makefile | 4 +++-
> >> 1 files changed, 3 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/drivers/net/wireless/ath/wil6210/Makefile b/drivers/net/wireless/ath/wil6210/Makefile
> >> index 9396dc9..d288eea 100644
> >> --- a/drivers/net/wireless/ath/wil6210/Makefile
> >> +++ b/drivers/net/wireless/ath/wil6210/Makefile
> >> @@ -9,5 +9,7 @@ wil6210-objs += wmi.o
> >> wil6210-objs += interrupt.o
> >> wil6210-objs += txrx.o
> >>
> >> -subdir-ccflags-y += -Werror
> >> +ifeq (, $(findstring -W,$(EXTRA_CFLAGS)))
> >> + subdir-ccflags-y += -Werror
> >> +endif
> >> subdir-ccflags-y += -D__CHECK_ENDIAN__
> >>
> > Acked-by: Vladimir Kondratiev <qca_vkondrat@qca.qualcomm.com>
> >
> > Well, agree; -W triggers lots of warnings, I suspect all other places that use
> > -Werror should be problematic. Worth fixing all others? Quick look for v3.8 raises:
> >
>
> thank you for your suggestion.
> after this patch applied, I will do for others (let you as signed-of-by, too).
Perhaps, it would be good idea to fight the original problem.
I mean, fix warnings where possible. Example: there are lots of
"unused parameter" ones. Where it is false warning, add __maybe_unused
attribute. Like this:
diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index c566927..83e43b7 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -163,8 +163,9 @@ extern int _cond_resched(void);
# define might_sleep() \
do { __might_sleep(__FILE__, __LINE__, 0); might_resched(); } while (0)
#else
- static inline void __might_sleep(const char *file, int line,
- int preempt_offset) { }
+ static inline void __might_sleep(const char *file __maybe_unused,
+ int line __maybe_unused,
+ int preempt_offset __maybe_unused) { }
# define might_sleep() do { might_resched(); } while (0)
#endif
^ permalink raw reply related
* [Suggestion] drivers/isdn: about the loop after call isdn_tty_send_msg
From: Chen Gang @ 2013-02-27 9:32 UTC (permalink / raw)
To: Jiri Kosina, isdn, Jiri Slaby; +Cc: Greg KH, Alan Cox, netdev
Hello Maintainers:
do we need break out of the loop after call isdn_tty_send_msg ?
p is not increased after finish calling isdn_tty_send_msg.
and then will loop back to process the p again.
do we intend to scan again the string which appended "+M..." ?
we also assume "+M..." is a NUL terminated string, is it OK ? (it seems ok)
thanks.
gchen.
in drivers/isdn/i4l/isdn_tty.c:
3408 static void
3409 isdn_tty_parse_at(modem_info *info)
3410 {
3411 atemu *m = &info->emu;
3412 char *p;
3413 char ds[ISDN_MSNLEN];
3414
3415 #ifdef ISDN_DEBUG_AT
3416 printk(KERN_DEBUG "AT: '%s'\n", m->mdmcmd);
3417 #endif
3418 for (p = &m->mdmcmd[2]; *p;) {
3419 switch (*p) {
3420 case ' ':
3421 p++;
3422 break;
...
3559 case '+':
3560 p++;
3561 switch (*p) {
3562 #ifdef CONFIG_ISDN_AUDIO
3563 case 'F':
3564 p++;
3565 if (isdn_tty_cmd_PLUSF(&p, info))
3566 return;
3567 break;
3568 case 'V':
3569 if ((!(m->mdmreg[REG_SI1] & 1)) ||
3570 (m->mdmreg[REG_L2PROT] == ISDN_PROTO_L2_MODEM))
3571 PARSE_ERROR;
3572 p++;
3573 if (isdn_tty_cmd_PLUSV(&p, info))
3574 return;
3575 break;
3576 #endif /* CONFIG_ISDN_AUDIO */
3577 case 'S': /* SUSPEND */
3578 p++;
3579 isdn_tty_get_msnstr(ds, &p);
3580 isdn_tty_suspend(ds, info, m);
3581 break;
3582 case 'R': /* RESUME */
3583 p++;
3584 isdn_tty_get_msnstr(ds, &p);
3585 isdn_tty_resume(ds, info, m);
3586 break;
3587 case 'M': /* MESSAGE */
3588 p++;
3589 isdn_tty_send_msg(info, m, p);
3590 break;
3591 default:
3592 PARSE_ERROR;
3593 }
3594 break;
3595 case '&':
3596 p++;
3597 if (isdn_tty_cmd_ATand(&p, info))
3598 return;
3599 break;
3600 default:
3601 PARSE_ERROR;
3602 }
3603 }
3604 #ifdef CONFIG_ISDN_AUDIO
3605 if (!info->vonline)
3606 #endif
3607 isdn_tty_modem_result(RESULT_OK, info);
3608 }
^ permalink raw reply
* [PATCH] drivers/isdn: beautify code, delete 'break' after 'return'
From: Chen Gang @ 2013-02-27 9:16 UTC (permalink / raw)
To: Jiri Kosina, isdn, Jiri Slaby; +Cc: Greg KH, alan, netdev
In-Reply-To: <512DCE3B.3050107@asianux.com>
delete 'break' statement after 'return' statement
Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
drivers/isdn/i4l/isdn_tty.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/isdn/i4l/isdn_tty.c b/drivers/isdn/i4l/isdn_tty.c
index d8a7d83..925a7ed 100644
--- a/drivers/isdn/i4l/isdn_tty.c
+++ b/drivers/isdn/i4l/isdn_tty.c
@@ -3425,7 +3425,6 @@ isdn_tty_parse_at(modem_info *info)
p++;
isdn_tty_cmd_ATA(info);
return;
- break;
case 'D':
/* D - Dial */
if (info->msr & UART_MSR_DCD)
--
1.7.7.6
^ permalink raw reply related
* Re: drivers/isdn: beautify code, delete 'break' after 'return'
From: Chen Gang @ 2013-02-27 9:14 UTC (permalink / raw)
To: Jiri Kosina, isdn, Jiri Slaby; +Cc: Greg KH, alan, netdev
In-Reply-To: <512DCE3B.3050107@asianux.com>
oh, sorry, the subject misses [PATCH], I will send again.
于 2013年02月27日 17:13, Chen Gang 写道:
>
> delete 'break' statement after 'return' statement
>
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
> drivers/isdn/i4l/isdn_tty.c | 1 -
> 1 files changed, 0 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/isdn/i4l/isdn_tty.c b/drivers/isdn/i4l/isdn_tty.c
> index d8a7d83..925a7ed 100644
> --- a/drivers/isdn/i4l/isdn_tty.c
> +++ b/drivers/isdn/i4l/isdn_tty.c
> @@ -3425,7 +3425,6 @@ isdn_tty_parse_at(modem_info *info)
> p++;
> isdn_tty_cmd_ATA(info);
> return;
> - break;
> case 'D':
> /* D - Dial */
> if (info->msr & UART_MSR_DCD)
>
--
Chen Gang
Asianux Corporation
^ permalink raw reply
* drivers/isdn: beautify code, delete 'break' after 'return'
From: Chen Gang @ 2013-02-27 9:13 UTC (permalink / raw)
To: Jiri Kosina, isdn, Jiri Slaby; +Cc: Greg KH, alan, netdev
delete 'break' statement after 'return' statement
Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
drivers/isdn/i4l/isdn_tty.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/isdn/i4l/isdn_tty.c b/drivers/isdn/i4l/isdn_tty.c
index d8a7d83..925a7ed 100644
--- a/drivers/isdn/i4l/isdn_tty.c
+++ b/drivers/isdn/i4l/isdn_tty.c
@@ -3425,7 +3425,6 @@ isdn_tty_parse_at(modem_info *info)
p++;
isdn_tty_cmd_ATA(info);
return;
- break;
case 'D':
/* D - Dial */
if (info->msr & UART_MSR_DCD)
--
1.7.7.6
^ permalink raw reply related
* Re: [PATCH] usb/net/asix_devices: Add USBNET HG20F9 ethernet dongle
From: Bjørn Mork @ 2013-02-27 9:07 UTC (permalink / raw)
To: gdt; +Cc: gregkh, linux-usb, netdev, linux-kernel
In-Reply-To: <20130226.172955.926853809838377302.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
>> Applied, thanks Glen.
>
> Actually, I had to revert, this doesn't even compile against
> current sources:
>
> CC [M] drivers/net/usb/asix_devices.o
> drivers/net/usb/asix_devices.c:941:14: error: ‘asix_rx_fixup’ undeclared here (not in a function)
> make[1]: *** [drivers/net/usb/asix_devices.o] Error 1
> make: *** [drivers/net/usb/asix_devices.o] Error 2
I believe this patch will make checkpatch cry
ERROR: do not use C99 // comments
as well. It would be nice if that was fixed as well before resending.
But please keep the commit message :)
Bjørn
^ permalink raw reply
* [PATCH] drivers/isdn: checkng length to be sure not memory overflow
From: Chen Gang @ 2013-02-27 9:05 UTC (permalink / raw)
To: Jiri Kosina, isdn, Jiri Slaby, Greg KH; +Cc: alan, netdev
the length of cmd.parm.cmsg.para is 50 (MAX_CAPI_PARA_LEN).
the strlen(msg) may be more than 50 (Modem-Commandbuffer, less than 255).
isdn_tty_send_msg is called by isdn_tty_parse_at
the relative parameter is m->mdmcmd (atemu *m)
the relative command may be "+M..."
so need check the length to be sure not memory overflow.
Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
drivers/isdn/i4l/isdn_tty.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/isdn/i4l/isdn_tty.c b/drivers/isdn/i4l/isdn_tty.c
index d8a7d83..c3f0f99 100644
--- a/drivers/isdn/i4l/isdn_tty.c
+++ b/drivers/isdn/i4l/isdn_tty.c
@@ -902,7 +902,7 @@ isdn_tty_send_msg(modem_info *info, atemu *m, char *msg)
int j;
int l;
- l = strlen(msg);
+ l = min(strlen(msg), sizeof(cmd.parm.cmsg.para) - 2);
if (!l) {
isdn_tty_modem_result(RESULT_ERROR, info);
return;
--
1.7.7.6
^ permalink raw reply related
* Re: [PATCH] drivers/net/wireless/ath/wil6210: Makefile, only -Werror when no -W* in EXTRA_CFLAGS
From: Chen Gang @ 2013-02-27 8:56 UTC (permalink / raw)
To: Vladimir Kondratiev
Cc: John W. Linville, linux-wireless, wil6210,
netdev@vger.kernel.org >> netdev,
linux-kernel@vger.kernel.org
In-Reply-To: <1385870.TegBXpTM9i@lx-vladimir>
于 2013年02月27日 16:45, Vladimir Kondratiev 写道:
> On Wednesday, February 27, 2013 02:55:06 PM Chen Gang wrote:
>>
>> When make with EXTRA_CFLAGS=-W, it will report error.
>> so give a check in Makefile.
>>
>> Signed-off-by: Chen Gang <gang.chen@asianux.com>
>> ---
>> drivers/net/wireless/ath/wil6210/Makefile | 4 +++-
>> 1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/wil6210/Makefile b/drivers/net/wireless/ath/wil6210/Makefile
>> index 9396dc9..d288eea 100644
>> --- a/drivers/net/wireless/ath/wil6210/Makefile
>> +++ b/drivers/net/wireless/ath/wil6210/Makefile
>> @@ -9,5 +9,7 @@ wil6210-objs += wmi.o
>> wil6210-objs += interrupt.o
>> wil6210-objs += txrx.o
>>
>> -subdir-ccflags-y += -Werror
>> +ifeq (, $(findstring -W,$(EXTRA_CFLAGS)))
>> + subdir-ccflags-y += -Werror
>> +endif
>> subdir-ccflags-y += -D__CHECK_ENDIAN__
>>
> Acked-by: Vladimir Kondratiev <qca_vkondrat@qca.qualcomm.com>
>
> Well, agree; -W triggers lots of warnings, I suspect all other places that use
> -Werror should be problematic. Worth fixing all others? Quick look for v3.8 raises:
>
thank you for your suggestion.
after this patch applied, I will do for others (let you as signed-of-by, too).
:-)
> linux$ find . -name Makefile | xargs grep -e -Werror
> ./arch/sh/lib/Makefile:ccflags-y := -Werror
> ./arch/sh/kernel/Makefile:ccflags-y := -Werror
> ./arch/sh/mm/Makefile:ccflags-y := -Werror
> ./arch/sh/cchips/hd6446x/Makefile:ccflags-y := -Werror
> ./arch/sparc/prom/Makefile:ccflags := -Werror
> ./arch/sparc/lib/Makefile:ccflags-y := -Werror
> ./arch/sparc/kernel/Makefile:ccflags-y := -Werror
> ./arch/sparc/mm/Makefile:ccflags-y := -Werror
> ./arch/alpha/lib/Makefile:ccflags-y := -Werror
> ./arch/alpha/oprofile/Makefile:ccflags-y := -Werror -Wno-sign-compare
> ./arch/alpha/mm/Makefile:ccflags-y := -Werror
> ./arch/powerpc/kvm/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
> ./arch/powerpc/lib/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
> ./arch/powerpc/kernel/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
> ./arch/powerpc/oprofile/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
> ./arch/powerpc/xmon/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
> ./arch/powerpc/mm/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
> ./arch/powerpc/sysdev/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
> ./arch/powerpc/sysdev/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
> ./arch/powerpc/sysdev/xics/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
> ./arch/powerpc/platforms/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
> ./arch/powerpc/perf/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
> ./Makefile: -Werror-implicit-function-declaration \
> ./tools/perf/Makefile: CFLAGS_WERROR := -Werror
> ./tools/perf/Makefile:ifeq ($(call try-cc,$(SOURCE_HELLO),$(CFLAGS) -Werror -fstack-protector-all,-fstack-protector-all),y)
> ./tools/perf/Makefile:ifeq ($(call try-cc,$(SOURCE_HELLO),$(CFLAGS) -Werror -Wstack-protector,-Wstack-protector),y)
> ./tools/perf/Makefile:ifeq ($(call try-cc,$(SOURCE_HELLO),$(CFLAGS) -Werror -Wvolatile-register-var,-Wvolatile-register-var),y)
> ./drivers/scsi/lpfc/Makefile:ccflags-y += -Werror
> ./drivers/scsi/aic7xxx/Makefile:ccflags-y += -Werror
> ./drivers/staging/omapdrm/Makefile:ccflags-y := -Iinclude/drm -Werror
> ./drivers/net/wireless/ath/wil6210/Makefile:subdir-ccflags-y += -Werror
>
>
>
--
Chen Gang
Asianux Corporation
^ permalink raw reply
* Re: [PATCH] drivers/net/wireless/ath/wil6210: Makefile, only -Werror when no -W* in EXTRA_CFLAGS
From: Vladimir Kondratiev @ 2013-02-27 8:45 UTC (permalink / raw)
To: Chen Gang
Cc: John W. Linville, linux-wireless, wil6210,
netdev@vger.kernel.org >> netdev
In-Reply-To: <512DADCA.9040609@asianux.com>
On Wednesday, February 27, 2013 02:55:06 PM Chen Gang wrote:
>
> When make with EXTRA_CFLAGS=-W, it will report error.
> so give a check in Makefile.
>
> Signed-off-by: Chen Gang <gang.chen@asianux.com>
> ---
> drivers/net/wireless/ath/wil6210/Makefile | 4 +++-
> 1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/wil6210/Makefile b/drivers/net/wireless/ath/wil6210/Makefile
> index 9396dc9..d288eea 100644
> --- a/drivers/net/wireless/ath/wil6210/Makefile
> +++ b/drivers/net/wireless/ath/wil6210/Makefile
> @@ -9,5 +9,7 @@ wil6210-objs += wmi.o
> wil6210-objs += interrupt.o
> wil6210-objs += txrx.o
>
> -subdir-ccflags-y += -Werror
> +ifeq (, $(findstring -W,$(EXTRA_CFLAGS)))
> + subdir-ccflags-y += -Werror
> +endif
> subdir-ccflags-y += -D__CHECK_ENDIAN__
>
Acked-by: Vladimir Kondratiev <qca_vkondrat@qca.qualcomm.com>
Well, agree; -W triggers lots of warnings, I suspect all other places that use
-Werror should be problematic. Worth fixing all others? Quick look for v3.8 raises:
linux$ find . -name Makefile | xargs grep -e -Werror
./arch/sh/lib/Makefile:ccflags-y := -Werror
./arch/sh/kernel/Makefile:ccflags-y := -Werror
./arch/sh/mm/Makefile:ccflags-y := -Werror
./arch/sh/cchips/hd6446x/Makefile:ccflags-y := -Werror
./arch/sparc/prom/Makefile:ccflags := -Werror
./arch/sparc/lib/Makefile:ccflags-y := -Werror
./arch/sparc/kernel/Makefile:ccflags-y := -Werror
./arch/sparc/mm/Makefile:ccflags-y := -Werror
./arch/alpha/lib/Makefile:ccflags-y := -Werror
./arch/alpha/oprofile/Makefile:ccflags-y := -Werror -Wno-sign-compare
./arch/alpha/mm/Makefile:ccflags-y := -Werror
./arch/powerpc/kvm/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
./arch/powerpc/lib/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
./arch/powerpc/kernel/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
./arch/powerpc/oprofile/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
./arch/powerpc/xmon/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
./arch/powerpc/mm/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
./arch/powerpc/sysdev/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
./arch/powerpc/sysdev/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
./arch/powerpc/sysdev/xics/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
./arch/powerpc/platforms/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
./arch/powerpc/perf/Makefile:subdir-ccflags-$(CONFIG_PPC_WERROR) := -Werror
./Makefile: -Werror-implicit-function-declaration \
./tools/perf/Makefile: CFLAGS_WERROR := -Werror
./tools/perf/Makefile:ifeq ($(call try-cc,$(SOURCE_HELLO),$(CFLAGS) -Werror -fstack-protector-all,-fstack-protector-all),y)
./tools/perf/Makefile:ifeq ($(call try-cc,$(SOURCE_HELLO),$(CFLAGS) -Werror -Wstack-protector,-Wstack-protector),y)
./tools/perf/Makefile:ifeq ($(call try-cc,$(SOURCE_HELLO),$(CFLAGS) -Werror -Wvolatile-register-var,-Wvolatile-register-var),y)
./drivers/scsi/lpfc/Makefile:ccflags-y += -Werror
./drivers/scsi/aic7xxx/Makefile:ccflags-y += -Werror
./drivers/staging/omapdrm/Makefile:ccflags-y := -Iinclude/drm -Werror
./drivers/net/wireless/ath/wil6210/Makefile:subdir-ccflags-y += -Werror
^ permalink raw reply
* Re: [PATCH 6/8] netfront: multi-page ring support
From: ANNIE LI @ 2013-02-27 7:39 UTC (permalink / raw)
To: Wei Liu
Cc: xen-devel@lists.xen.org, netdev@vger.kernel.org, Ian Campbell,
konrad.wilk@oracle.com
In-Reply-To: <1361882139.2109.52.camel@zion.uk.xensource.com>
On 2013-2-26 20:35, Wei Liu wrote:
> On Tue, 2013-02-26 at 06:52 +0000, ANNIE LI wrote:
>> On 2013-2-16 0:00, Wei Liu wrote:
>>> Signed-off-by: Wei Liu<wei.liu2@citrix.com>
>>> ---
>>> drivers/net/xen-netfront.c | 246 +++++++++++++++++++++++++++++++-------------
>>> 1 file changed, 174 insertions(+), 72 deletions(-)
>>>
>>> diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
>>> index 8bd75a1..de73a71 100644
>>> --- a/drivers/net/xen-netfront.c
>>> +++ b/drivers/net/xen-netfront.c
>>> @@ -67,9 +67,19 @@ struct netfront_cb {
>>>
>>> #define GRANT_INVALID_REF 0
>>>
>>> -#define NET_TX_RING_SIZE __CONST_RING_SIZE(xen_netif_tx, PAGE_SIZE)
>>> -#define NET_RX_RING_SIZE __CONST_RING_SIZE(xen_netif_rx, PAGE_SIZE)
>>> -#define TX_MAX_TARGET min_t(int, NET_TX_RING_SIZE, 256)
>>> +#define XENNET_MAX_RING_PAGE_ORDER XENBUS_MAX_RING_PAGE_ORDER
>>> +#define XENNET_MAX_RING_PAGES (1U<< XENNET_MAX_RING_PAGE_ORDER)
>>> +
>>> +
>>> +#define NET_TX_RING_SIZE(_nr_pages) \
>>> + __CONST_RING_SIZE(xen_netif_tx, PAGE_SIZE * (_nr_pages))
>>> +#define NET_RX_RING_SIZE(_nr_pages) \
>>> + __CONST_RING_SIZE(xen_netif_rx, PAGE_SIZE * (_nr_pages))
>>> +
>>> +#define XENNET_MAX_TX_RING_SIZE NET_TX_RING_SIZE(XENNET_MAX_RING_PAGES)
>>> +#define XENNET_MAX_RX_RING_SIZE NET_RX_RING_SIZE(XENNET_MAX_RING_PAGES)
>>> +
>>> +#define TX_MAX_TARGET min_t(int, NET_TX_RING_SIZE(1), 256)
>> Not using multi-page ring here?
>> In xennet_create_dev, gnttab_alloc_grant_references allocates
>> TX_MAX_TARGET number of grant reference for tx. In
>> xennet_release_tx_bufs, NET_TX_RING_SIZE(np->tx_ring_pages) numbers of
>> grants are processed. And NET_RX_RING_SIZE(np->tx_ring_pages) is totally
>> different from TX_MAX_TARGET if np->rx_ring_pages is not 1. Although
>> skb_entry_is_link helps to not release invalid grants, lots of null loop
>> seems unnecessary. I think TX_MAX_TARGET should be changed into some
>> variableconnected with np->tx_ring_pages. Or you intended to use one
>> page ring here?
>>
> Looking back my history, this limitation was introduced because if we
> have a multi-page backend and single page frontend, the backend skb
> processing could overlap.
I did not see the overlap you mentioned here in netback. Although
netback supports multi-page, netback->vif still uses single page if the
frontend only supports single page. Netfront and netback negotiate this
through xenstore in your 5/8 patch. The requests and response should not
have any overlap between netback and netfront. Am I missing something?
>
> I agree with you that this limit should be variable, but as we still use
> M:N model, the safe option is to cap this limit to 1 page.
Yes, M:N model is still used here. But the share ring should be same for
netback->vif and netfront.
Thanks
Annie
>
> Another option is to check validity of skbs before processing them. I
> will look into that as well.
>
> The same reason applies to the RX ring as well.
>
>
> Wei.
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH] core: Add ioctls to control device unicast hw addresses
From: John Fastabend @ 2013-02-27 7:34 UTC (permalink / raw)
To: Jitendra Kalsaria
Cc: John Fastabend, vyasevic@redhat.com, David Miller, netdev
In-Reply-To: <BECD8E8A1B550B48A1BF97C13991F60E2D2E1A@avmb2.qlogic.org>
On 02/26/2013 09:42 PM, Jitendra Kalsaria wrote:
>
>
> On 2/26/13 9:01 PM, "John Fastabend" <john.r.fastabend@intel.com> wrote:
>
>> On 2/26/2013 6:27 PM, Vlad Yasevich wrote:
>>> On 02/26/2013 09:07 PM, John Fastabend wrote:
>>>> On Tue, Feb 26, 2013 at 05:15:03PM -0500, David Miller wrote:
>>>>> From: John Fastabend <john.r.fastabend@intel.com>
>>>>> Date: Tue, 26 Feb 2013 13:58:39 -0800
>>>>>
>>>>>> [...]
>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> [RFC PATCH] rtnetlink: Add support for adding/removing additional
>>>>>>>>> hw
>>>>>>>>> addresses.
>>>>>>>>>
>>>>>>>>> Add an ability to add and remove HW addresses to the device
>>>>>>>>> unicast and multicast address lists. Right now, we only have
>>>>>>>>> an ioctl() to manage the multicast addresses and there is no
>>>>>>>>> way the manage the unicast list.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>>>>>>>>
>>>>>>>> This is a step in the right direction, and you're right that there
>>>>>>>> is
>>>>>>>> a difficulty in detecting whether support exists or not.
>>>>>>>>
>>>>>>>> I am so surprised that we've have ->set_rx_mode() support for
>>>>>>>> multiple
>>>>>>>> unicast MAC addresses in so many drivers all this time, yet no way
>>>>>>>> outside of FDB to make use of it at all.
>>>>>>>
>>>>>>> And even that is not always available. In most drivers it requires
>>>>>>> module parameters or other explicit configuration steps. Meanwhile
>>>>>>> set_rx_mode() doesn't seem to depend on any of those and just does
>>>>>>> the
>>>>>>> right thing.
>>>>>>>
>>>>>>> For what I was trying to do ioctl() was a really easy way out for
>>>>>>> both
>>>>>>> kernel and user space implementation, so I gave is shot.
>>>>>>>
>>>>>>> -vlad
>>>>>>>
>>>>>>
>>>>>> Don't we already support this with
>>>>>
>>>>> The whole point is that these multiple-unicast-address configuration
>>>>> facilities are inaccessible without FDB, and there is no reason
>>>>> whatsoever for that.
>>>>
>>>> Yes I see now sorry I was behind the thread.
>>>>
>>>> We could just use the fdb hooks and add a dflt routine to handle the
>>>> case where the netdev doesn't provide a specific ndo_op for us. This
>>>> boils down to calling set_rx_mode anyways through this call chain,
>>>>
>>>> ndo_fdb_add
>>>> dev_uc_add_excl
>>>> __dev_set_rx_mode
>>>> ndo_set_rx_mode
>>>>
>>>> As long as folks don't think this is too much of an abuse of the fdb
>>>> hooks. Here's an example I only compile tested this so take it as
>>>> an example only. It probably needs some cleanups and the dump routine
>>>> needs some thought not sure you want to dump all MACs always.
>>>
>>> I thought about doing, but this becomes broken on the drivers that
>>> support ndo_fdb_* functions. Those drivers mainly require additional
>>> configuration that is not necessary for dev_uc_add_excl() to work.
>>>
>>> For example, ixgbe and melanox requires VF to be on, qlogic needs a
>>> module parameter, macvlan has to be in passthrough. However,
>>> ndo_set_rx_mode() in most cases doesn't care about those settings and
>>> just works.
>>>
>>
>> The ixgbe piece could just use the dflt routines I put below. The
>> SR-IOV check is only there because when I wrote that I didn't consider
>> adding additional addresses without VFs. This was a poor example.
>>
>> The mlx4 card is checking if the device is a slave or master before
>> adding/removing addresses. My guess is this is the same type of check
>> as ixgbe and would work just fine without it.
>>
>> I'm not sure macvlan supports unicast hw addresses either way but
>> there is no reason we couldn't. The idea was we would come back and
>> write it later. Like many things I haven't got to it yet.
>>
>> Calling set_rx_mode() on qlcnic doesn't look to help you at all either
>> it appears to ignore the uc_list either way.
>
> Even though we use module parameter for fdb but it would be good it have
> set_rx_mode().
> We don't ignore uc_list but it never called set_rx_mode() when I use
> dev_uc_add_excl when added support and thought that's how it might
> designed.
>
> -Jiten
>>
Hopefully not too off topic but you probably should use dev_uc_add_excl
in fdb_add and then fix your set_rx_mode() handler to configure the
uc_list correctly. Not sure what happens today when a unicast addr
is added to the list for example with macvlan looks like a bug to me.
.John
--
John Fastabend Intel Corporation
^ permalink raw reply
* Re: [PATCH] net: dev: deinit list in unregister_netdevice_many
From: Gao feng @ 2013-02-27 7:31 UTC (permalink / raw)
To: David Miller; +Cc: netdev, eric.dumazet
In-Reply-To: <20130227.022125.374228548587492555.davem@davemloft.net>
On 2013/02/27 15:21, David Miller wrote:
>
> mroute_clean_tables() uses the list after unregister_netdevice_many().
>
You mean list_for_each_entry_safe(c, next, &mrt->mfc_cache_array[i], list)
in mroute_clean_tables()?
Isn't this list just a member of struct mfc_cache?
I can't find the other codes in mroute_clean_tables uses this list
after unregister_netdevice_many.
> Can you please just stop tinkering in this area? You are making many
> careless errors.
>
Ok,I am fine if you drop this patch,just as commit ceaaec98ad99859ac90ac6863ad0a6cd075d8e0e
said "device is freed without touching its unreg_list". there will be no serious problems
if we don't have this patch.
^ permalink raw reply
* Re: [PATCH] net: dev: deinit list in unregister_netdevice_many
From: Eric Dumazet @ 2013-02-27 7:25 UTC (permalink / raw)
To: Gao feng; +Cc: netdev, David S. Miller
In-Reply-To: <1361949146.11403.21.camel@edumazet-glaptop>
On Tue, 2013-02-26 at 23:12 -0800, Eric Dumazet wrote:
> I totally Nack this patch.
>
> 1) net-next is not open.
>
> 2) We spent enough time chasing bugs, I don't want to reintroduce new
> ones.
>
> And this patch introduce nice bugs. No thank you.
Humm... maybe I misread the patch, I'll take another look tomorrow after
some sleep.
Thanks
^ permalink raw reply
* Re: [PATCH] net: dev: deinit list in unregister_netdevice_many
From: David Miller @ 2013-02-27 7:21 UTC (permalink / raw)
To: gaofeng; +Cc: netdev, eric.dumazet
In-Reply-To: <1361948391-4906-1-git-send-email-gaofeng@cn.fujitsu.com>
mroute_clean_tables() uses the list after unregister_netdevice_many().
Can you please just stop tinkering in this area? You are making many
careless errors.
Thanks.
^ permalink raw reply
* Re: [PATCH] net: dev: deinit list in unregister_netdevice_many
From: Eric Dumazet @ 2013-02-27 7:12 UTC (permalink / raw)
To: Gao feng; +Cc: netdev, David S. Miller
In-Reply-To: <1361948391-4906-1-git-send-email-gaofeng@cn.fujitsu.com>
On Wed, 2013-02-27 at 14:59 +0800, Gao feng wrote:
> commit ceaaec98ad99859ac90ac6863ad0a6cd075d8e0e
> "net: deinit automatic LIST_HEAD" has already done
> part of the deinit list jobs.
>
> This patch push list_del into unregister_netdevice_many
> to make sure the unreg_list of netdevice object has valid
> pointer always.
>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Eric Dumazet <eric.dumazet@gmail.com>
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> ---
I totally Nack this patch.
1) net-next is not open.
2) We spent enough time chasing bugs, I don't want to reintroduce new
ones.
And this patch introduce nice bugs. No thank you.
^ permalink raw reply
* [PATCH] net: dev: deinit list in unregister_netdevice_many
From: Gao feng @ 2013-02-27 6:59 UTC (permalink / raw)
To: netdev; +Cc: Gao feng, David S. Miller, Eric Dumazet
commit ceaaec98ad99859ac90ac6863ad0a6cd075d8e0e
"net: deinit automatic LIST_HEAD" has already done
part of the deinit list jobs.
This patch push list_del into unregister_netdevice_many
to make sure the unreg_list of netdevice object has valid
pointer always.
Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
drivers/net/macvlan.c | 1 -
net/core/dev.c | 5 ++++-
net/core/rtnetlink.c | 1 -
net/mac80211/iface.c | 1 -
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index defcd8a..9b728f1 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -945,7 +945,6 @@ static int macvlan_device_event(struct notifier_block *unused,
list_for_each_entry_safe(vlan, next, &port->vlans, list)
vlan->dev->rtnl_link_ops->dellink(vlan->dev, &list_kill);
unregister_netdevice_many(&list_kill);
- list_del(&list_kill);
break;
case NETDEV_PRE_TYPE_CHANGE:
/* Forbid underlaying device to change its type. */
diff --git a/net/core/dev.c b/net/core/dev.c
index 18d8b5a..c10f322 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5747,6 +5747,9 @@ EXPORT_SYMBOL(unregister_netdevice_queue);
/**
* unregister_netdevice_many - unregister many devices
* @head: list of devices
+ *
+ * Call list_del after unregister devices to make sure
+ * the unreg_list of netdevice object has valid pointer.
*/
void unregister_netdevice_many(struct list_head *head)
{
@@ -5756,6 +5759,7 @@ void unregister_netdevice_many(struct list_head *head)
rollback_registered_many(head);
list_for_each_entry(dev, head, unreg_list)
net_set_todo(dev);
+ list_del(head);
}
}
EXPORT_SYMBOL(unregister_netdevice_many);
@@ -6172,7 +6176,6 @@ static void __net_exit default_device_exit_batch(struct list_head *net_list)
}
}
unregister_netdevice_many(&dev_kill_list);
- list_del(&dev_kill_list);
rtnl_unlock();
}
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index d8aa20f..8e35210 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1613,7 +1613,6 @@ static int rtnl_dellink(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
ops->dellink(dev, &list_kill);
unregister_netdevice_many(&list_kill);
- list_del(&list_kill);
return 0;
}
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 2c059e5..a799629 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1639,7 +1639,6 @@ void ieee80211_remove_interfaces(struct ieee80211_local *local)
}
mutex_unlock(&local->iflist_mtx);
unregister_netdevice_many(&unreg_list);
- list_del(&unreg_list);
list_for_each_entry_safe(sdata, tmp, &wdev_list, list) {
list_del(&sdata->list);
--
1.7.11.7
^ permalink raw reply related
* [PATCH] drivers/net/wireless/ath/wil6210: Makefile, only -Werror when no -W* in EXTRA_CFLAGS
From: Chen Gang @ 2013-02-27 6:55 UTC (permalink / raw)
To: qca_vkondrat, John W. Linville, linux-wireless, wil6210,
netdev@vger.kernel.org >> netdev
When make with EXTRA_CFLAGS=-W, it will report error.
so give a check in Makefile.
Signed-off-by: Chen Gang <gang.chen@asianux.com>
---
drivers/net/wireless/ath/wil6210/Makefile | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/Makefile b/drivers/net/wireless/ath/wil6210/Makefile
index 9396dc9..d288eea 100644
--- a/drivers/net/wireless/ath/wil6210/Makefile
+++ b/drivers/net/wireless/ath/wil6210/Makefile
@@ -9,5 +9,7 @@ wil6210-objs += wmi.o
wil6210-objs += interrupt.o
wil6210-objs += txrx.o
-subdir-ccflags-y += -Werror
+ifeq (, $(findstring -W,$(EXTRA_CFLAGS)))
+ subdir-ccflags-y += -Werror
+endif
subdir-ccflags-y += -D__CHECK_ENDIAN__
--
1.7.7.6
^ permalink raw reply related
* Re: [RFC PATCH] core: Add ioctls to control device unicast hw addresses
From: Jitendra Kalsaria @ 2013-02-27 5:42 UTC (permalink / raw)
To: John Fastabend, vyasevic@redhat.com; +Cc: John Fastabend, David Miller, netdev
In-Reply-To: <512D9337.6070504@intel.com>
On 2/26/13 9:01 PM, "John Fastabend" <john.r.fastabend@intel.com> wrote:
>On 2/26/2013 6:27 PM, Vlad Yasevich wrote:
>> On 02/26/2013 09:07 PM, John Fastabend wrote:
>>> On Tue, Feb 26, 2013 at 05:15:03PM -0500, David Miller wrote:
>>>> From: John Fastabend <john.r.fastabend@intel.com>
>>>> Date: Tue, 26 Feb 2013 13:58:39 -0800
>>>>
>>>>> [...]
>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>> [RFC PATCH] rtnetlink: Add support for adding/removing additional
>>>>>>>>hw
>>>>>>>> addresses.
>>>>>>>>
>>>>>>>> Add an ability to add and remove HW addresses to the device
>>>>>>>> unicast and multicast address lists. Right now, we only have
>>>>>>>> an ioctl() to manage the multicast addresses and there is no
>>>>>>>> way the manage the unicast list.
>>>>>>>>
>>>>>>>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>>>>>>>
>>>>>>> This is a step in the right direction, and you're right that there
>>>>>>>is
>>>>>>> a difficulty in detecting whether support exists or not.
>>>>>>>
>>>>>>> I am so surprised that we've have ->set_rx_mode() support for
>>>>>>> multiple
>>>>>>> unicast MAC addresses in so many drivers all this time, yet no way
>>>>>>> outside of FDB to make use of it at all.
>>>>>>
>>>>>> And even that is not always available. In most drivers it requires
>>>>>> module parameters or other explicit configuration steps. Meanwhile
>>>>>> set_rx_mode() doesn't seem to depend on any of those and just does
>>>>>>the
>>>>>> right thing.
>>>>>>
>>>>>> For what I was trying to do ioctl() was a really easy way out for
>>>>>>both
>>>>>> kernel and user space implementation, so I gave is shot.
>>>>>>
>>>>>> -vlad
>>>>>>
>>>>>
>>>>> Don't we already support this with
>>>>
>>>> The whole point is that these multiple-unicast-address configuration
>>>> facilities are inaccessible without FDB, and there is no reason
>>>> whatsoever for that.
>>>
>>> Yes I see now sorry I was behind the thread.
>>>
>>> We could just use the fdb hooks and add a dflt routine to handle the
>>> case where the netdev doesn't provide a specific ndo_op for us. This
>>> boils down to calling set_rx_mode anyways through this call chain,
>>>
>>> ndo_fdb_add
>>> dev_uc_add_excl
>>> __dev_set_rx_mode
>>> ndo_set_rx_mode
>>>
>>> As long as folks don't think this is too much of an abuse of the fdb
>>> hooks. Here's an example I only compile tested this so take it as
>>> an example only. It probably needs some cleanups and the dump routine
>>> needs some thought not sure you want to dump all MACs always.
>>
>> I thought about doing, but this becomes broken on the drivers that
>> support ndo_fdb_* functions. Those drivers mainly require additional
>> configuration that is not necessary for dev_uc_add_excl() to work.
>>
>> For example, ixgbe and melanox requires VF to be on, qlogic needs a
>> module parameter, macvlan has to be in passthrough. However,
>> ndo_set_rx_mode() in most cases doesn't care about those settings and
>> just works.
>>
>
>The ixgbe piece could just use the dflt routines I put below. The
>SR-IOV check is only there because when I wrote that I didn't consider
>adding additional addresses without VFs. This was a poor example.
>
>The mlx4 card is checking if the device is a slave or master before
>adding/removing addresses. My guess is this is the same type of check
>as ixgbe and would work just fine without it.
>
>I'm not sure macvlan supports unicast hw addresses either way but
>there is no reason we couldn't. The idea was we would come back and
>write it later. Like many things I haven't got to it yet.
>
>Calling set_rx_mode() on qlcnic doesn't look to help you at all either
>it appears to ignore the uc_list either way.
Even though we use module parameter for fdb but it would be good it have
set_rx_mode().
We don't ignore uc_list but it never called set_rx_mode() when I use
dev_uc_add_excl when added support and thought that's how it might
designed.
-Jiten
>
>is that all the callers? looks like it.
>
>> So fdb will not always work even if the driver has proper support for
>> IFF_UNICAST_FLT.
>>
>
>The fdb could work if we fix the drivers it seems to me most
>the cases where it wouldn't work are just lack of foresight or the code
>isn't there yet. I think it might be valuable to have a single API for
>both use cases.
>
>.John
>
>> -vlad
>>
>--
>To unsubscribe from this list: send the line "unsubscribe netdev" in
>the body of a message to majordomo@vger.kernel.org
>More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply
* Re: iproute-3.8.0 build error on Linux ppc64
From: nello martuscielli @ 2013-02-27 5:39 UTC (permalink / raw)
To: netdev
In-Reply-To: <CABMX=WwQNmNPsfZHvMYq9p9W_Q1RbZ=bpTCcv5AN=wD0CGSqmQ@mail.gmail.com>
sorry, it was the previous commit about -Werror
thanks,
Nell
On Wed, Feb 27, 2013 at 6:15 AM, nello martuscielli <ppc.addon@gmail.com> wrote:
> hi,
> after commit:
> http://git.kernel.org/?p=linux/kernel/git/shemminger/iproute2.git;a=commitdiff;h=6398d3a65277a2ebed085ad9deef15fea6f4ed09
>
> specifically -Wmissing-prototypes and I get this error on Linux ppc64
>
> [...]
> gcc -Wall -Wstrict-prototypes -Werror -Wmissing-prototypes
> -Wmissing-declarations -Wold-style-definition -O2 -I../include
> -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\"
> -D_GNU_SOURCE -c -o ipneigh.o ipneigh.c
> gcc -Wall -Wstrict-prototypes -Werror -Wmissing-prototypes
> -Wmissing-declarations -Wold-style-definition -O2 -I../include
> -DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\"
> -D_GNU_SOURCE -c -o ipntable.o ipntable.c
> cc1: warnings being treated as errors
> ipntable.c: In function 'print_ntable':
> ipntable.c:435:3: error: format '%llu' expects type 'long long
> unsigned int', but argument 3 has type '__u64'
> ipntable.c:488:4: error: format '%llu' expects type 'long long
> unsigned int', but argument 3 has type '__u64'
> ipntable.c:492:4: error: format '%llu' expects type 'long long
> unsigned int', but argument 3 has type '__u64'
> ipntable.c:496:4: error: format '%llu' expects type 'long long
> unsigned int', but argument 3 has type '__u64'
> ipntable.c:505:4: error: format '%llu' expects type 'long long
> unsigned int', but argument 3 has type '__u64'
> ipntable.c:509:4: error: format '%llu' expects type 'long long
> unsigned int', but argument 3 has type '__u64'
> ipntable.c:539:4: error: format '%llu' expects type 'long long
> unsigned int', but argument 3 has type '__u64'
> ipntable.c:543:4: error: format '%llu' expects type 'long long
> unsigned int', but argument 3 has type '__u64'
> ipntable.c:551:4: error: format '%llu' expects type 'long long
> unsigned int', but argument 3 has type '__u64'
> ipntable.c:563:3: error: format '%llu' expects type 'long long
> unsigned int', but argument 3 has type '__u64'
> ipntable.c:564:3: error: format '%llu' expects type 'long long
> unsigned int', but argument 3 has type '__u64'
> ipntable.c:565:3: error: format '%llu' expects type 'long long
> unsigned int', but argument 3 has type '__u64'
> ipntable.c:570:3: error: format '%llu' expects type 'long long
> unsigned int', but argument 3 has type '__u64'
> ipntable.c:571:3: error: format '%llu' expects type 'long long
> unsigned int', but argument 3 has type '__u64'
> ipntable.c:572:3: error: format '%llu' expects type 'long long
> unsigned int', but argument 3 has type '__u64'
> ipntable.c:577:3: error: format '%llu' expects type 'long long
> unsigned int', but argument 3 has type '__u64'
> ipntable.c:578:3: error: format '%llu' expects type 'long long
> unsigned int', but argument 3 has type '__u64'
> ipntable.c:583:3: error: format '%llu' expects type 'long long
> unsigned int', but argument 3 has type '__u64'
> ipntable.c:584:3: error: format '%llu' expects type 'long long
> unsigned int', but argument 3 has type '__u64'
> make[1]: *** [ipntable.o] Error 1
> make[1]: Leaving directory `/root/iproute2-3.8.0/ip'
> make: *** [all] Error 2
>
>
> my system is IBM POWER4+ with a CRUX PPC 3.0alpha (64bit).
>
>
> thanks,
> Nell
> --
> my blog: http://linuxpowerpc.blogspot.com/
--
--
Power Mac G4 AGP 450MHz - CRUX PPC (32bit)
^ permalink raw reply
* Re: understanding linux cache and free command
From: Prashant Batra @ 2013-02-27 5:35 UTC (permalink / raw)
To: Prashant Batra, netdev; +Cc: linux-newbie
In-Reply-To: <512CAF30.70805@gmail.com>
Adding netdev for a bigger audience.
On Tuesday, February 26, 2013 6:18:48 PM, Prashant Batra wrote:
> Hi All,
>
> I have created few memory cache areas using "kmem_cache_create" and
> using it to allocate some memory.
>
> What I observe after some cache_alloc's and cache_free's is that
> /proc/slabinfo entries for my objects are good, meaning the number of
> objects go back to 0 after they are freed.
>
> But linux "free" command shows that some of the objects are cached and
> that keeps on increasing. I am not able to understand this behavior.
> Is it that kernel keeps the cache objects even after they are freed?
>
> #cat /proc/slabinfo
> my_cache 200 200 4096 1 1 : tunables 24 12 8
> : slabdata 200 200 0
>
> It shows 200 allocated objects from "my_cache"
>
> and
>
> #free -lm
>
> total used free shared buffers cached
> Mem: 48340 610 47730 0 96 130
>
> the cached objects keep on growing.
>
> After 5 mins in which I am doing some alloc and free-
> total used free shared buffers cached
> Mem: 48340 616 47723 0 96 137
>
> /proc/slabinfo remains the same.
>
> Any help is appreciated.
>
> Thanks,
> Prashant
^ permalink raw reply
* [patch] irda: small read beyond end of array in debug code
From: Dan Carpenter @ 2013-02-27 5:15 UTC (permalink / raw)
To: Samuel Ortiz; +Cc: David S. Miller, netdev, kernel-janitors
charset comes from skb->data. It's a number in the 0-255 range.
If we have debugging turned on then this could cause a read beyond
the end of the array.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/net/irda/iriap.c b/net/irda/iriap.c
index e71e85b..29340a9 100644
--- a/net/irda/iriap.c
+++ b/net/irda/iriap.c
@@ -495,8 +495,11 @@ static void iriap_getvaluebyclass_confirm(struct iriap_cb *self,
/* case CS_ISO_8859_9: */
/* case CS_UNICODE: */
default:
- IRDA_DEBUG(0, "%s(), charset %s, not supported\n",
- __func__, ias_charset_types[charset]);
+ IRDA_DEBUG(0, "%s(), charset [%d] %s, not supported\n",
+ __func__, charset,
+ charset < ARRAY_SIZE(ias_charset_types) ?
+ ias_charset_types[charset] :
+ "(unknown)");
/* Aborting, close connection! */
iriap_disconnect_request(self);
^ permalink raw reply related
* iproute-3.8.0 build error on Linux ppc64
From: nello martuscielli @ 2013-02-27 5:15 UTC (permalink / raw)
To: netdev
In-Reply-To: <CABMX=WwSNeTq_ascBRxJtQrsErR-cFODZQh_wrim6K8weyLDtQ@mail.gmail.com>
hi,
after commit:
http://git.kernel.org/?p=linux/kernel/git/shemminger/iproute2.git;a=commitdiff;h=6398d3a65277a2ebed085ad9deef15fea6f4ed09
specifically -Wmissing-prototypes and I get this error on Linux ppc64
[...]
gcc -Wall -Wstrict-prototypes -Werror -Wmissing-prototypes
-Wmissing-declarations -Wold-style-definition -O2 -I../include
-DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\"
-D_GNU_SOURCE -c -o ipneigh.o ipneigh.c
gcc -Wall -Wstrict-prototypes -Werror -Wmissing-prototypes
-Wmissing-declarations -Wold-style-definition -O2 -I../include
-DRESOLVE_HOSTNAMES -DLIBDIR=\"/usr/lib\" -DCONFDIR=\"/etc/iproute2\"
-D_GNU_SOURCE -c -o ipntable.o ipntable.c
cc1: warnings being treated as errors
ipntable.c: In function 'print_ntable':
ipntable.c:435:3: error: format '%llu' expects type 'long long
unsigned int', but argument 3 has type '__u64'
ipntable.c:488:4: error: format '%llu' expects type 'long long
unsigned int', but argument 3 has type '__u64'
ipntable.c:492:4: error: format '%llu' expects type 'long long
unsigned int', but argument 3 has type '__u64'
ipntable.c:496:4: error: format '%llu' expects type 'long long
unsigned int', but argument 3 has type '__u64'
ipntable.c:505:4: error: format '%llu' expects type 'long long
unsigned int', but argument 3 has type '__u64'
ipntable.c:509:4: error: format '%llu' expects type 'long long
unsigned int', but argument 3 has type '__u64'
ipntable.c:539:4: error: format '%llu' expects type 'long long
unsigned int', but argument 3 has type '__u64'
ipntable.c:543:4: error: format '%llu' expects type 'long long
unsigned int', but argument 3 has type '__u64'
ipntable.c:551:4: error: format '%llu' expects type 'long long
unsigned int', but argument 3 has type '__u64'
ipntable.c:563:3: error: format '%llu' expects type 'long long
unsigned int', but argument 3 has type '__u64'
ipntable.c:564:3: error: format '%llu' expects type 'long long
unsigned int', but argument 3 has type '__u64'
ipntable.c:565:3: error: format '%llu' expects type 'long long
unsigned int', but argument 3 has type '__u64'
ipntable.c:570:3: error: format '%llu' expects type 'long long
unsigned int', but argument 3 has type '__u64'
ipntable.c:571:3: error: format '%llu' expects type 'long long
unsigned int', but argument 3 has type '__u64'
ipntable.c:572:3: error: format '%llu' expects type 'long long
unsigned int', but argument 3 has type '__u64'
ipntable.c:577:3: error: format '%llu' expects type 'long long
unsigned int', but argument 3 has type '__u64'
ipntable.c:578:3: error: format '%llu' expects type 'long long
unsigned int', but argument 3 has type '__u64'
ipntable.c:583:3: error: format '%llu' expects type 'long long
unsigned int', but argument 3 has type '__u64'
ipntable.c:584:3: error: format '%llu' expects type 'long long
unsigned int', but argument 3 has type '__u64'
make[1]: *** [ipntable.o] Error 1
make[1]: Leaving directory `/root/iproute2-3.8.0/ip'
make: *** [all] Error 2
my system is IBM POWER4+ with a CRUX PPC 3.0alpha (64bit).
thanks,
Nell
--
my blog: http://linuxpowerpc.blogspot.com/
^ permalink raw reply
* Re: [RFC PATCH] core: Add ioctls to control device unicast hw addresses
From: John Fastabend @ 2013-02-27 5:01 UTC (permalink / raw)
To: vyasevic; +Cc: John Fastabend, David Miller, netdev
In-Reply-To: <512D6F27.90209@redhat.com>
On 2/26/2013 6:27 PM, Vlad Yasevich wrote:
> On 02/26/2013 09:07 PM, John Fastabend wrote:
>> On Tue, Feb 26, 2013 at 05:15:03PM -0500, David Miller wrote:
>>> From: John Fastabend <john.r.fastabend@intel.com>
>>> Date: Tue, 26 Feb 2013 13:58:39 -0800
>>>
>>>> [...]
>>>>
>>>>>>>
>>>>>>>
>>>>>>> [RFC PATCH] rtnetlink: Add support for adding/removing additional hw
>>>>>>> addresses.
>>>>>>>
>>>>>>> Add an ability to add and remove HW addresses to the device
>>>>>>> unicast and multicast address lists. Right now, we only have
>>>>>>> an ioctl() to manage the multicast addresses and there is no
>>>>>>> way the manage the unicast list.
>>>>>>>
>>>>>>> Signed-off-by: Vlad Yasevich <vyasevic@redhat.com>
>>>>>>
>>>>>> This is a step in the right direction, and you're right that there is
>>>>>> a difficulty in detecting whether support exists or not.
>>>>>>
>>>>>> I am so surprised that we've have ->set_rx_mode() support for
>>>>>> multiple
>>>>>> unicast MAC addresses in so many drivers all this time, yet no way
>>>>>> outside of FDB to make use of it at all.
>>>>>
>>>>> And even that is not always available. In most drivers it requires
>>>>> module parameters or other explicit configuration steps. Meanwhile
>>>>> set_rx_mode() doesn't seem to depend on any of those and just does the
>>>>> right thing.
>>>>>
>>>>> For what I was trying to do ioctl() was a really easy way out for both
>>>>> kernel and user space implementation, so I gave is shot.
>>>>>
>>>>> -vlad
>>>>>
>>>>
>>>> Don't we already support this with
>>>
>>> The whole point is that these multiple-unicast-address configuration
>>> facilities are inaccessible without FDB, and there is no reason
>>> whatsoever for that.
>>
>> Yes I see now sorry I was behind the thread.
>>
>> We could just use the fdb hooks and add a dflt routine to handle the
>> case where the netdev doesn't provide a specific ndo_op for us. This
>> boils down to calling set_rx_mode anyways through this call chain,
>>
>> ndo_fdb_add
>> dev_uc_add_excl
>> __dev_set_rx_mode
>> ndo_set_rx_mode
>>
>> As long as folks don't think this is too much of an abuse of the fdb
>> hooks. Here's an example I only compile tested this so take it as
>> an example only. It probably needs some cleanups and the dump routine
>> needs some thought not sure you want to dump all MACs always.
>
> I thought about doing, but this becomes broken on the drivers that
> support ndo_fdb_* functions. Those drivers mainly require additional
> configuration that is not necessary for dev_uc_add_excl() to work.
>
> For example, ixgbe and melanox requires VF to be on, qlogic needs a
> module parameter, macvlan has to be in passthrough. However,
> ndo_set_rx_mode() in most cases doesn't care about those settings and
> just works.
>
The ixgbe piece could just use the dflt routines I put below. The
SR-IOV check is only there because when I wrote that I didn't consider
adding additional addresses without VFs. This was a poor example.
The mlx4 card is checking if the device is a slave or master before
adding/removing addresses. My guess is this is the same type of check
as ixgbe and would work just fine without it.
I'm not sure macvlan supports unicast hw addresses either way but
there is no reason we couldn't. The idea was we would come back and
write it later. Like many things I haven't got to it yet.
Calling set_rx_mode() on qlcnic doesn't look to help you at all either
it appears to ignore the uc_list either way.
is that all the callers? looks like it.
> So fdb will not always work even if the driver has proper support for
> IFF_UNICAST_FLT.
>
The fdb could work if we fix the drivers it seems to me most
the cases where it wouldn't work are just lack of foresight or the code
isn't there yet. I think it might be valuable to have a single API for
both use cases.
.John
> -vlad
>
^ permalink raw reply
* RE: [PATCH 4/4] sctp: fix association hangs due to partial delivery errors
From: Roberts, Lee A. @ 2013-02-27 4:38 UTC (permalink / raw)
To: vyasevic@redhat.com; +Cc: netdev@vger.kernel.org
In-Reply-To: <512D70D0.1060208@redhat.com>
Vlad,
> -----Original Message-----
> From: Vlad Yasevich [mailto:vyasevic@redhat.com]
> Sent: Tuesday, February 26, 2013 7:35 PM
> To: Roberts, Lee A.
> Cc: netdev@vger.kernel.org
> Subject: Re: [PATCH 4/4] sctp: fix association hangs due to partial delivery errors
>
> On 02/26/2013 09:36 AM, Lee A. Roberts wrote:
> > From: "Lee A. Roberts" <lee.roberts@hp.com>
> >
> > Resolve SCTP association hangs observed during SCTP stress
> > testing. Observable symptoms include communications hangs
> > with data being held in the association reassembly and/or lobby
> > (ordering) queues. Close examination of reassembly queue shows
> > missing packets.
> >
> > In sctp_ulpq_tail_data(), use return values 0,1 to indicate whether
> > a complete event (with MSG_EOR set) was delivered. A return value
> > of -ENOMEM continues to indicate an out-of-memory condition was
> > encountered.
> >
> > In sctp_ulpq_retrieve_partial() and sctp_ulpq_retrieve_first(),
> > correct message reassembly logic for SCTP partial delivery.
> > Change logic to ensure that as much data as possible is sent
> > with the initial partial delivery and that following partial
> > deliveries contain all available data.
> >
> > In sctp_ulpq_partial_delivery(), attempt partial delivery only
> > if the data on the head of the reassembly queue is at or before
> > the cumulative TSN ACK point.
> >
> > In sctp_ulpq_renege(), use the modified return values from
> > sctp_ulpq_tail_data() to choose whether to attempt partial
> > delivery or to attempt to drain the reassembly queue as a
> > means to reduce memory pressure. Remove call to
> > sctp_tsnmap_mark(), as this is handled correctly in call to
> > sctp_ulpq_tail_data().
> >
> > Signed-off-by: Lee A. Roberts <lee.roberts@hp.com>
> > ---
> > net/sctp/ulpqueue.c | 54 ++++++++++++++++++++++++++++++++++++++++-----------
> > 1 file changed, 43 insertions(+), 11 deletions(-)
> >
> > diff --git a/net/sctp/ulpqueue.c b/net/sctp/ulpqueue.c
> > index f221fbb..482e3ea 100644
> > --- a/net/sctp/ulpqueue.c
> > +++ b/net/sctp/ulpqueue.c
> > @@ -106,6 +106,7 @@ int sctp_ulpq_tail_data(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
> > {
> > struct sk_buff_head temp;
> > struct sctp_ulpevent *event;
> > + int event_eor = 0;
> >
> > /* Create an event from the incoming chunk. */
> > event = sctp_ulpevent_make_rcvmsg(chunk->asoc, chunk, gfp);
> > @@ -127,10 +128,12 @@ int sctp_ulpq_tail_data(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
> > /* Send event to the ULP. 'event' is the sctp_ulpevent for
> > * very first SKB on the 'temp' list.
> > */
> > - if (event)
> > + if (event) {
> > + event_eor = (event->msg_flags & MSG_EOR) ? 1 : 0;
> > sctp_ulpq_tail_event(ulpq, event);
> > + }
>
> You can re-use the check just before this one to catch the EOR
> condition. You already do
> if ((event) && (event->msg_flags & MSG_EOR)){
>
> right after you try to get a reassembled event.
>
> Everything else looks good.
> -vlad
>
It depends on what we want the return value to mean. In the case where we've reneged
to be able to put in a packet, we know that the incoming packet is the next one we're
looking for. In which case, the earlier test is correct, since this will be the next
event.
In general, the event that's being passed to sctp_ulpq_order() might not be the
next event. In that case, sctp_ulpq_order() will call sctp_ulpq_store_ordered()
and return NULL. Only at the last "if (event)" can we really be sure that an event
is being passed to the ULP.
In general, do we want to return 1 if a complete event is actually passed to the ULP?
Or, do we want to return 1 if we've seen a complete event come from reassembly?
- Lee
> >
> > - return 0;
> > + return event_eor;
> > }
> >
> > /* Add a new event for propagation to the ULP. */
> > @@ -540,14 +543,19 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_partial(struct sctp_ulpq
> *ulpq)
> > ctsn = cevent->tsn;
> >
> > switch (cevent->msg_flags & SCTP_DATA_FRAG_MASK) {
> > + case SCTP_DATA_FIRST_FRAG:
> > + if (!first_frag)
> > + return NULL;
> > + goto done;
> > case SCTP_DATA_MIDDLE_FRAG:
> > if (!first_frag) {
> > first_frag = pos;
> > next_tsn = ctsn + 1;
> > last_frag = pos;
> > - } else if (next_tsn == ctsn)
> > + } else if (next_tsn == ctsn) {
> > next_tsn++;
> > - else
> > + last_frag = pos;
> > + } else
> > goto done;
> > break;
> > case SCTP_DATA_LAST_FRAG:
> > @@ -651,6 +659,14 @@ static struct sctp_ulpevent *sctp_ulpq_retrieve_first(struct sctp_ulpq *ulpq)
> > } else
> > goto done;
> > break;
> > +
> > + case SCTP_DATA_LAST_FRAG:
> > + if (!first_frag)
> > + return NULL;
> > + else
> > + goto done;
> > + break;
> > +
> > default:
> > return NULL;
> > }
> > @@ -1025,16 +1041,28 @@ void sctp_ulpq_partial_delivery(struct sctp_ulpq *ulpq,
> > struct sctp_ulpevent *event;
> > struct sctp_association *asoc;
> > struct sctp_sock *sp;
> > + __u32 ctsn;
> > + struct sk_buff *skb;
> >
> > asoc = ulpq->asoc;
> > sp = sctp_sk(asoc->base.sk);
> >
> > /* If the association is already in Partial Delivery mode
> > - * we have noting to do.
> > + * we have nothing to do.
> > */
> > if (ulpq->pd_mode)
> > return;
> >
> > + /* Data must be at or below the Cumulative TSN ACK Point to
> > + * start partial delivery.
> > + */
> > + skb = skb_peek(&asoc->ulpq.reasm);
> > + if (skb != NULL) {
> > + ctsn = sctp_skb2event(skb)->tsn;
> > + if (!TSN_lte(ctsn, sctp_tsnmap_get_ctsn(&asoc->peer.tsn_map)))
> > + return;
> > + }
> > +
> > /* If the user enabled fragment interleave socket option,
> > * multiple associations can enter partial delivery.
> > * Otherwise, we can only enter partial delivery if the
> > @@ -1077,12 +1105,16 @@ void sctp_ulpq_renege(struct sctp_ulpq *ulpq, struct sctp_chunk *chunk,
> > }
> > /* If able to free enough room, accept this chunk. */
> > if (chunk && (freed >= needed)) {
> > - __u32 tsn;
> > - tsn = ntohl(chunk->subh.data_hdr->tsn);
> > - sctp_tsnmap_mark(&asoc->peer.tsn_map, tsn, chunk->transport);
> > - sctp_ulpq_tail_data(ulpq, chunk, gfp);
> > -
> > - sctp_ulpq_partial_delivery(ulpq, gfp);
> > + int retval;
> > + retval = sctp_ulpq_tail_data(ulpq, chunk, gfp);
> > + /*
> > + * Enter partial delivery if chunk has not been
> > + * delivered; otherwise, drain the reassembly queue.
> > + */
> > + if (retval <= 0)
> > + sctp_ulpq_partial_delivery(ulpq, chunk, gfp);
> > + else if (retval == 1)
> > + sctp_ulpq_reasm_drain(ulpq);
> > }
> >
> > sk_mem_reclaim(asoc->base.sk);
> >
^ permalink raw reply
* Re: Fw: [Bug 54491] New: UFO (UDP fragmentation offload) does not work if the payload size specified is within the range ( (MTU-28) to (MTU-8) )
From: Cong Wang @ 2013-02-27 3:20 UTC (permalink / raw)
To: netdev
In-Reply-To: <20130226074606.4d16ebfb@nehalam.linuxnetplumber.net>
On Tue, 26 Feb 2013 at 15:46 GMT, Stephen Hemminger <stephen@networkplumber.org> wrote:
> For IPv4/UDP if the payload size specified is within the range ( (MTU-28)
> to (MTU-8) ) packet gets segmented in kernel even if the following netdev
> features are set:
>
> NETIF_F_SG
> NETIF_F_GEN_CSUM
> NETIF_F_UFO
> NETIF_F_TSO
> NETIF_F_GSO
> NETIF_F_GSO_ROBUST
>
> Analysis:
> udp_sendmsg() computes packet length by adding UDP header size to payload size:
> “ulen += sizeof(struct udphdr)”
> Then the function calls ip_make_skb(), however IP header size is not added to
> the packet length.
> ip_make_skb() in turn calls __ip_append_data(). The same packet length is used
> here.
> __ip_append_data() compares the packet length with MTU. The packet length still
> does not include IP header size (20 bytes).
> In result, when payload size “is within the range ( (MTU-28) to (MTU-8) )”
> standard branch is taken rather than ip_ufo_append_data(). Gso_size is not
> computed, therefore IP fragmentation is triggered from ip_finish_output().
>
I haven't looked into this deeply, it sounds like we need this fix:
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c
index 5e12dca..3ccb704 100644
--- a/net/ipv4/ip_output.c
+++ b/net/ipv4/ip_output.c
@@ -845,7 +845,7 @@ static int __ip_append_data(struct sock *sk,
csummode = CHECKSUM_PARTIAL;
cork->length += length;
- if (((length > mtu) || (skb &&skb_is_gso(skb))) &&
+ if (((length + fragheaderlen > mtu) || (skb &&skb_is_gso(skb))) &&
(sk->sk_protocol == IPPROTO_UDP) &&
(rt->dst.dev->features & NETIF_F_UFO) && !rt->dst.header_len) {
err = ip_ufo_append_data(sk, queue, getfrag, from,length,
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox