* Re: [PATCH] s2io: Fix warnings due to -Wunused-but-set-variable.
From: Jon Mason @ 2011-04-12 16:00 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20110411.160143.258100330.davem@davemloft.net>
On Mon, Apr 11, 2011 at 04:01:43PM -0700, David Miller wrote:
>
> Most of these are cases where we are trying to read back a register
> after a write to ensure completion.
>
> Simply pre-fixing the readl() or readq() with "(void)" is sufficient
> because these are volatile operations and the compiler cannot eliminate
> them just because no real assignment takes place.
>
> The case of free_rxd_blk()'s assignments to "struct buffAdd *ba" is a
> real spurious assignment as this variable is completely otherwise
> unused.
>
> Signed-off-by: David S. Miller <davem@davemloft.net>
Acked-by: Jon Mason <jdmason@kudzu.us>
> ---
> drivers/net/s2io.c | 5 +----
> drivers/net/s2io.h | 10 ++++------
> 2 files changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/net/s2io.c b/drivers/net/s2io.c
> index ca8e75e..2d5cc61 100644
> --- a/drivers/net/s2io.c
> +++ b/drivers/net/s2io.c
> @@ -2244,13 +2244,12 @@ static int verify_xena_quiescence(struct s2io_nic *sp)
> static void fix_mac_address(struct s2io_nic *sp)
> {
> struct XENA_dev_config __iomem *bar0 = sp->bar0;
> - u64 val64;
> int i = 0;
>
> while (fix_mac[i] != END_SIGN) {
> writeq(fix_mac[i++], &bar0->gpio_control);
> udelay(10);
> - val64 = readq(&bar0->gpio_control);
> + (void) readq(&bar0->gpio_control);
> }
> }
>
> @@ -2727,7 +2726,6 @@ static void free_rxd_blk(struct s2io_nic *sp, int ring_no, int blk)
> int j;
> struct sk_buff *skb;
> struct RxD_t *rxdp;
> - struct buffAdd *ba;
> struct RxD1 *rxdp1;
> struct RxD3 *rxdp3;
> struct mac_info *mac_control = &sp->mac_control;
> @@ -2751,7 +2749,6 @@ static void free_rxd_blk(struct s2io_nic *sp, int ring_no, int blk)
> memset(rxdp, 0, sizeof(struct RxD1));
> } else if (sp->rxd_mode == RXD_MODE_3B) {
> rxdp3 = (struct RxD3 *)rxdp;
> - ba = &mac_control->rings[ring_no].ba[blk][j];
> pci_unmap_single(sp->pdev,
> (dma_addr_t)rxdp3->Buffer0_ptr,
> BUF0_LEN,
> diff --git a/drivers/net/s2io.h b/drivers/net/s2io.h
> index 628fd27..800b3a4 100644
> --- a/drivers/net/s2io.h
> +++ b/drivers/net/s2io.h
> @@ -1002,18 +1002,16 @@ static inline void writeq(u64 val, void __iomem *addr)
> #define LF 2
> static inline void SPECIAL_REG_WRITE(u64 val, void __iomem *addr, int order)
> {
> - u32 ret;
> -
> if (order == LF) {
> writel((u32) (val), addr);
> - ret = readl(addr);
> + (void) readl(addr);
> writel((u32) (val >> 32), (addr + 4));
> - ret = readl(addr + 4);
> + (void) readl(addr + 4);
> } else {
> writel((u32) (val >> 32), (addr + 4));
> - ret = readl(addr + 4);
> + (void) readl(addr + 4);
> writel((u32) (val), addr);
> - ret = readl(addr);
> + (void) readl(addr);
> }
> }
>
> --
> 1.7.4.3
>
> --
> 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: 2.6.39-rc2 boot crash
From: Eric B Munson @ 2011-04-12 15:59 UTC (permalink / raw)
To: Patrick McHardy
Cc: Evgeniy Polyakov, David Miller, dave, linux-kernel, gregkh,
NetDev
In-Reply-To: <4DA47247.20700@trash.net>
[-- Attachment #1: Type: text/plain, Size: 2006 bytes --]
On Tue, 12 Apr 2011, Patrick McHardy wrote:
> On 12.04.2011 14:49, Patrick McHardy wrote:
> > On 12.04.2011 00:06, Evgeniy Polyakov wrote:
> >> Hi.
> >>
> >> On Mon, Apr 11, 2011 at 05:07:47PM -0400, Eric B Munson (emunson@mgebm.net) wrote:
> >>>> I can't figure this out, the only thing that should have changed is the
> >>>> time the initial PROC_CN_MCAST_LISTEN message is received. Apparently
> >>>> at that point connector is not fully initialized yet. Please post your
> >>>> config and the full boot log. Thanks.
> >>>>
> >>>
> >>> I am still seeing this on Linus' tree, is there anything more I can do to help
> >>> track the problem?
> >
> > Sorry, I had a hardware failure, I'm back working on this now.
> >
> >> Patrick, do you need my assist on this bug?
> >
> > Thanks, but I can meanwhile reproduce the problem, so I think I
> > should have a fix soon.
>
> I think this patch should fix the problem. Eric, could you please
> give it a try?
This has me up and running again, thanks!
Tested-by: Eric B Munson <emunson@mgebm.net>
>
>
>
> commit ad676e0dbbe8658ce46e192f449689bf3011bdf5
> Author: Patrick McHardy <kaber@trash.net>
> Date: Tue Apr 12 17:37:04 2011 +0200
>
> connector: fix skb double free in cn_rx_skb()
>
> When a skb is delivered to a registered callback, cn_call_callback()
> incorrectly returns -ENODEV after freeing the skb, causing cn_rx_skb()
> to free the skb a second time.
>
> Reported-by: Eric B Munson <emunson@mgebm.net>
> Signed-off-by: Patrick McHardy <kaber@trash.net>
>
> diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
> index d770058..219d88a 100644
> --- a/drivers/connector/connector.c
> +++ b/drivers/connector/connector.c
> @@ -142,6 +142,7 @@ static int cn_call_callback(struct sk_buff *skb)
> cbq->callback(msg, nsp);
> kfree_skb(skb);
> cn_queue_release_callback(cbq);
> + err = 0;
> }
>
> return err;
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
^ permalink raw reply
* Re: 2.6.39-rc2 boot crash
From: Patrick McHardy @ 2011-04-12 15:39 UTC (permalink / raw)
To: Evgeniy Polyakov
Cc: Eric B Munson, David Miller, dave, linux-kernel, gregkh,
ksrinivasan, NetDev
In-Reply-To: <4DA44A73.3060801@trash.net>
[-- Attachment #1: Type: text/plain, Size: 871 bytes --]
On 12.04.2011 14:49, Patrick McHardy wrote:
> On 12.04.2011 00:06, Evgeniy Polyakov wrote:
>> Hi.
>>
>> On Mon, Apr 11, 2011 at 05:07:47PM -0400, Eric B Munson (emunson@mgebm.net) wrote:
>>>> I can't figure this out, the only thing that should have changed is the
>>>> time the initial PROC_CN_MCAST_LISTEN message is received. Apparently
>>>> at that point connector is not fully initialized yet. Please post your
>>>> config and the full boot log. Thanks.
>>>>
>>>
>>> I am still seeing this on Linus' tree, is there anything more I can do to help
>>> track the problem?
>
> Sorry, I had a hardware failure, I'm back working on this now.
>
>> Patrick, do you need my assist on this bug?
>
> Thanks, but I can meanwhile reproduce the problem, so I think I
> should have a fix soon.
I think this patch should fix the problem. Eric, could you please
give it a try?
[-- Attachment #2: cn.diff --]
[-- Type: text/x-patch, Size: 838 bytes --]
commit ad676e0dbbe8658ce46e192f449689bf3011bdf5
Author: Patrick McHardy <kaber@trash.net>
Date: Tue Apr 12 17:37:04 2011 +0200
connector: fix skb double free in cn_rx_skb()
When a skb is delivered to a registered callback, cn_call_callback()
incorrectly returns -ENODEV after freeing the skb, causing cn_rx_skb()
to free the skb a second time.
Reported-by: Eric B Munson <emunson@mgebm.net>
Signed-off-by: Patrick McHardy <kaber@trash.net>
diff --git a/drivers/connector/connector.c b/drivers/connector/connector.c
index d770058..219d88a 100644
--- a/drivers/connector/connector.c
+++ b/drivers/connector/connector.c
@@ -142,6 +142,7 @@ static int cn_call_callback(struct sk_buff *skb)
cbq->callback(msg, nsp);
kfree_skb(skb);
cn_queue_release_callback(cbq);
+ err = 0;
}
return err;
^ permalink raw reply related
* Re: [net-next PATCH 1/3] vxge: always enable hardware time stamp
From: Jon Mason @ 2011-04-12 15:36 UTC (permalink / raw)
To: David Miller; +Cc: netdev
In-Reply-To: <20110410.185845.70188298.davem@davemloft.net>
On Sun, Apr 10, 2011 at 06:58:45PM -0700, David Miller wrote:
> From: Jon Mason <jdmason@kudzu.us>
> Date: Fri, 8 Apr 2011 16:11:21 -0500
>
> > Hardware time stamp calculation can only be enabled by the privileged
> > function. Enable it always by default and simply use the ethtool
> > interface to set a flag to indicate whether or not the respective
> > function driver should indicate the timestamp along with the received
> > packet.
> >
> > Also, make certain fields in vxge_hw_device_config bit-fields to reduce
> > the size of the struct.
> >
> > Signed-off-by: Jon Mason <jdmason@kudzu.us>
>
> Doesn't this have some performance or latency impact?
It is all done in hardware by replacing the CRC with the HWTS value.
So, no perf or latency issues there. It still only handles the HWTS
in receive if it is enabled in software via the ioctl.
>
> I think it should be stay off by default, people who want this know
> they want it and can turn it on if they want to.
^ permalink raw reply
* [PATCH 2/2] net/natsami: store MAC into perm_addr
From: Otavio Salvador @ 2011-04-12 15:30 UTC (permalink / raw)
To: netdev; +Cc: Otavio Salvador
In-Reply-To: <1302622241-11871-1-git-send-email-otavio@ossystems.com.br>
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
---
drivers/net/natsemi.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/net/natsemi.c b/drivers/net/natsemi.c
index aa2813e..1074231 100644
--- a/drivers/net/natsemi.c
+++ b/drivers/net/natsemi.c
@@ -860,6 +860,9 @@ static int __devinit natsemi_probe1 (struct pci_dev *pdev,
prev_eedata = eedata;
}
+ /* Store MAC Address in perm_addr */
+ memcpy(dev->perm_addr, dev->dev_addr, ETH_ALEN);
+
dev->base_addr = (unsigned long __force) ioaddr;
dev->irq = irq;
--
1.7.4.1
^ permalink raw reply related
* [PATCH 1/2] net/sis900: store MAC into perm_addr for SiS 900, 630E, 635 and 96x variants
From: Otavio Salvador @ 2011-04-12 15:30 UTC (permalink / raw)
To: netdev; +Cc: Otavio Salvador
Signed-off-by: Otavio Salvador <otavio@ossystems.com.br>
---
drivers/net/sis900.c | 23 +++++++++++++++++++----
1 files changed, 19 insertions(+), 4 deletions(-)
diff --git a/drivers/net/sis900.c b/drivers/net/sis900.c
index cb317cd..484f795 100644
--- a/drivers/net/sis900.c
+++ b/drivers/net/sis900.c
@@ -240,7 +240,8 @@ static const struct ethtool_ops sis900_ethtool_ops;
* @net_dev: the net device to get address for
*
* Older SiS900 and friends, use EEPROM to store MAC address.
- * MAC address is read from read_eeprom() into @net_dev->dev_addr.
+ * MAC address is read from read_eeprom() into @net_dev->dev_addr and
+ * @net_dev->perm_addr.
*/
static int __devinit sis900_get_mac_addr(struct pci_dev * pci_dev, struct net_device *net_dev)
@@ -261,6 +262,9 @@ static int __devinit sis900_get_mac_addr(struct pci_dev * pci_dev, struct net_de
for (i = 0; i < 3; i++)
((u16 *)(net_dev->dev_addr))[i] = read_eeprom(ioaddr, i+EEPROMMACAddr);
+ /* Store MAC Address in perm_addr */
+ memcpy(net_dev->perm_addr, net_dev->dev_addr, ETH_ALEN);
+
return 1;
}
@@ -271,7 +275,8 @@ static int __devinit sis900_get_mac_addr(struct pci_dev * pci_dev, struct net_de
*
* SiS630E model, use APC CMOS RAM to store MAC address.
* APC CMOS RAM is accessed through ISA bridge.
- * MAC address is read into @net_dev->dev_addr.
+ * MAC address is read into @net_dev->dev_addr and
+ * @net_dev->perm_addr.
*/
static int __devinit sis630e_get_mac_addr(struct pci_dev * pci_dev,
@@ -296,6 +301,10 @@ static int __devinit sis630e_get_mac_addr(struct pci_dev * pci_dev,
outb(0x09 + i, 0x70);
((u8 *)(net_dev->dev_addr))[i] = inb(0x71);
}
+
+ /* Store MAC Address in perm_addr */
+ memcpy(net_dev->perm_addr, net_dev->dev_addr, ETH_ALEN);
+
pci_write_config_byte(isa_bridge, 0x48, reg & ~0x40);
pci_dev_put(isa_bridge);
@@ -310,7 +319,7 @@ static int __devinit sis630e_get_mac_addr(struct pci_dev * pci_dev,
*
* SiS635 model, set MAC Reload Bit to load Mac address from APC
* to rfdr. rfdr is accessed through rfcr. MAC address is read into
- * @net_dev->dev_addr.
+ * @net_dev->dev_addr and @net_dev->perm_addr.
*/
static int __devinit sis635_get_mac_addr(struct pci_dev * pci_dev,
@@ -334,6 +343,9 @@ static int __devinit sis635_get_mac_addr(struct pci_dev * pci_dev,
*( ((u16 *)net_dev->dev_addr) + i) = inw(ioaddr + rfdr);
}
+ /* Store MAC Address in perm_addr */
+ memcpy(net_dev->perm_addr, net_dev->dev_addr, ETH_ALEN);
+
/* enable packet filtering */
outl(rfcrSave | RFEN, rfcr + ioaddr);
@@ -353,7 +365,7 @@ static int __devinit sis635_get_mac_addr(struct pci_dev * pci_dev,
* EEDONE signal to refuse EEPROM access by LAN.
* The EEPROM map of SiS962 or SiS963 is different to SiS900.
* The signature field in SiS962 or SiS963 spec is meaningless.
- * MAC address is read into @net_dev->dev_addr.
+ * MAC address is read into @net_dev->dev_addr and @net_dev->perm_addr.
*/
static int __devinit sis96x_get_mac_addr(struct pci_dev * pci_dev,
@@ -372,6 +384,9 @@ static int __devinit sis96x_get_mac_addr(struct pci_dev * pci_dev,
for (i = 0; i < 3; i++)
((u16 *)(net_dev->dev_addr))[i] = read_eeprom(ioaddr, i+EEPROMMACAddr);
+ /* Store MAC Address in perm_addr */
+ memcpy(net_dev->perm_addr, net_dev->dev_addr, ETH_ALEN);
+
outl(EEDONE, ee_addr);
return 1;
} else {
--
1.7.4.1
^ permalink raw reply related
* Re: [RFC] iproute2: Fix meta match u32 with 0xffffffff
From: Stephen Hemminger @ 2011-04-12 15:19 UTC (permalink / raw)
To: tgraf; +Cc: netdev
In-Reply-To: <1302595009.3664.8.camel@lsx>
On Tue, 12 Apr 2011 09:56:49 +0200
Thomas Graf <tgraf@redhat.com> wrote:
> On Mon, 2011-04-11 at 11:52 -0700, Stephen Hemminger wrote:
> > The value 0xffffffff is a valid mask and bstrtoul() would return
> > ULONG_MAX which was the error value. Resolve the problem by separating
> > return value and error indication.
> >
> > -unsigned long bstrtoul(const struct bstr *b)
> > +int bstrtoul(const struct bstr *b, unsigned long *lp)
> > {
> > char *inv = NULL;
> > - unsigned long l;
> > char buf[b->len+1];
> >
> > + if (b->len == 0)
> > + return -EINVAL;
> > +
> > memcpy(buf, b->data, b->len);
> > buf[b->len] = '\0';
> >
> > - l = strtoul(buf, &inv, 0);
> > - if (l == ULONG_MAX || inv == buf)
> > - return ULONG_MAX;
> > + *lp = strtoul(buf, &inv, 0);
> > + if (inv == buf)
> > + return -EINVAL;
> > +
> > + if (*lp == ULONG_MAX || errno == ERANGE)
> > + return -ERANGE;
> >
> > - return l;
> > + return 0;
> > }
>
> This is definitely much better but we still can't parse ULONG_MAX
> as string representative. Checking glibc docs, the only way to do it is
> to ignore the return value for error checking and look errno.
>
I think the error case is ret == ULONG_MAX && errno == ERANGE
If there is no error, then strtoul doesn't set errno.
--
^ permalink raw reply
* Re: Kernel panic when using bridge
From: Jan Lübbe @ 2011-04-12 15:13 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Scot Doyle, Stephen Hemminger, Hiroaki SHIMODA, netdev
In-Reply-To: <1302619749.3233.56.camel@edumazet-laptop>
On Tue, 2011-04-12 at 16:49 +0200, Eric Dumazet wrote:
> Of course, this might be a complete shot in the dark, but a
> stackprotector fault in icmp_send() really sounds like a problem in
> ip_options_echo() [ or bad input data given to this function ]
It was my understanding that all IP options given to ip_options_echo are
either from local sources or have gone through ip_options_compile, which
seems to verify that the sum of the individual option lengths do not
exceed the ip header. So there wouldn't need to be additional checks in
ip_options_echo.
If this is not the case, we need size checks in ip_options_echo before
copying over each option.
> Other related changes (but as old as v2.6.22) :
>
> commit 11a03f78fbf15a866ba
> ([NetLabel]: core network changes)
When investigating the problem I had with timestamps, i found that most
of the lines in ip_options_echo and _compile have not been changed since
before 2.2 (some even before 2.0). The newer changes have all been
updates for changed API elsewhere in the stack.
Regards,
Jan
^ permalink raw reply
* Re: [PATCH] inetpeer: reduce stack usage
From: Eric Dumazet @ 2011-04-12 14:55 UTC (permalink / raw)
To: Hiroaki SHIMODA; +Cc: David Miller, Scot Doyle, Stephen Hemminger, netdev
In-Reply-To: <20110412235126.63841dfe.shimoda.hiroaki@gmail.com>
Le mardi 12 avril 2011 à 23:51 +0900, Hiroaki SHIMODA a écrit :
> I couldn't understand that actually cleanup_once() was called
> from inet_getpeer() and then the stack overflow was hit,
> but this patch surely reduces stack usage.
>
> Reviewed-by: Hiroaki SHIMODA <shimoda.hiroaki@gmail.com>
>
Well, I dont believe we actually hit a stack overflow in Scot Doyle
reported crashes, but it certainly is better to use a bit less stack
anyway ;)
Thanks for reviewing !
^ permalink raw reply
* Re: [PATCH] inetpeer: reduce stack usage
From: Hiroaki SHIMODA @ 2011-04-12 14:51 UTC (permalink / raw)
To: Eric Dumazet, David Miller; +Cc: Scot Doyle, Stephen Hemminger, netdev
In-Reply-To: <1302597580.3233.14.camel@edumazet-laptop>
On Tue, 12 Apr 2011 10:39:40 +0200
Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On 64bit arches, we use 752 bytes of stack when cleanup_once() is called
> from inet_getpeer().
>
> Lets share the avl stack to save ~376 bytes.
>
> Before patch :
>
> # objdump -d net/ipv4/inetpeer.o | scripts/checkstack.pl
>
> 0x000006c3 unlink_from_pool [inetpeer.o]: 376
> 0x00000721 unlink_from_pool [inetpeer.o]: 376
> 0x00000cb1 inet_getpeer [inetpeer.o]: 376
> 0x00000e6d inet_getpeer [inetpeer.o]: 376
> 0x0004 inet_initpeers [inetpeer.o]: 112
> # size net/ipv4/inetpeer.o
> text data bss dec hex filename
> 5320 432 21 5773 168d net/ipv4/inetpeer.o
>
> After patch :
>
> objdump -d net/ipv4/inetpeer.o | scripts/checkstack.pl
> 0x00000c11 inet_getpeer [inetpeer.o]: 376
> 0x00000dcd inet_getpeer [inetpeer.o]: 376
> 0x00000ab9 peer_check_expire [inetpeer.o]: 328
> 0x00000b7f peer_check_expire [inetpeer.o]: 328
> 0x0004 inet_initpeers [inetpeer.o]: 112
> # size net/ipv4/inetpeer.o
> text data bss dec hex filename
> 5163 432 21 5616 15f0 net/ipv4/inetpeer.o
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> Cc: Scot Doyle <lkml@scotdoyle.com>
> Cc: Stephen Hemminger <shemminger@vyatta.com>
> Cc: Hiroaki SHIMODA <shimoda.hiroaki@gmail.com>
I couldn't understand that actually cleanup_once() was called
from inet_getpeer() and then the stack overflow was hit,
but this patch surely reduces stack usage.
Reviewed-by: Hiroaki SHIMODA <shimoda.hiroaki@gmail.com>
Thanks.
^ permalink raw reply
* Re: [PATCH v4] net: bnx2x: convert to hw_features
From: Michał Mirosław @ 2011-04-12 14:49 UTC (permalink / raw)
To: Vladislav Zolotarov; +Cc: netdev@vger.kernel.org, Eilon Greenstein
In-Reply-To: <1302619012.6750.8.camel@lb-tlvb-vladz>
On Tue, Apr 12, 2011 at 05:36:52PM +0300, Vladislav Zolotarov wrote:
> On Tue, 2011-04-12 at 07:07 -0700, Michał Mirosław wrote:
> > On Tue, Apr 12, 2011 at 03:10:28PM +0300, Vladislav Zolotarov wrote:
> > > On Mon, 2011-04-11 at 13:26 -0700, Michał Mirosław wrote:
> > > > Since ndo_fix_features callback is postponing features change when
> > > > bp->recovery_state != BNX2X_RECOVERY_DONE, netdev_update_features()
> > > > has to be called again when this condition changes. Previously,
> > > > ethtool_ops->set_flags callback returned -EBUSY in that case
> > > > (it's not possible in the new model).
> > > ACK (with reservations). ;)
> > > Could u, pls., just add this comment I've asked for in the previous
> > > e-mail?
> > The one about vlan_features?
> Yeah, this one.
> > I'll just fix the comment in netdevice.h
> > instead, since it might be not clear enough.
> Could u still add this comment to bnx2x in your patch as well. It'll
> just make these code section more clear regardless the netdevice.h
> contents... ;)
This would be duplicating information easily found elsewhere.
I don't like it. :)
> > > The things I first thought to comment on are:
> > > - Removing TPA_ENABLED_FLAG the similar way u've removed the
> > > bp->rx_csum.
> > > - Merging the code handling 'features' in bnx2x_init_bp() with the
> > > similar code in bnx2x_init_dev().
> > >
> > > However I think it would be right if we clear our mess by ourselves and
> > > that u have already done much enough... ;)
> > >
> > > I've run our standard test suite (which in particular heavily tests the
> > > RX_CSUM and LRO flags toggling) on this patch and it passed it.
> > It might be possible to get rid of ndo_set_features, since it looks like
> > the reset/recovery handler is doing unload/load itself. This needs more
> > digging into the driver than this simple conversion.
> Hmm... I don't get u here. Although the recovery handler does
> unload/load itself if there has been an attempt to change features
> during the recovery it won't be able to get applied until the whole
> recovery process ends. So, this patch added the extra call for
> netdev_update_features() right after we know that the recovery process
> has successfully ended. So, could u, pls., explain exactly what do u
> mean here by saying "It might be possible to get rid of
> ndo_set_features"?
Hmm. I thought one, and wrote another.
Since bnx2x_parity_recover() runs with rtnl_lock(), as should
netdev_update_features(), then in case the recovery is in progress
it should be enough to not call bnx2x_reload_if_running() then
and just change the flags --- changes will be picked up on bnx2x_nic_load()
after recovery is complete. This removes the need for netdev_update_features()
calls.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: Kernel panic when using bridge
From: Eric Dumazet @ 2011-04-12 14:49 UTC (permalink / raw)
To: Jan Lübbe; +Cc: Scot Doyle, Stephen Hemminger, Hiroaki SHIMODA, netdev
In-Reply-To: <1302617968.30934.34.camel@polaris.local>
Le mardi 12 avril 2011 à 16:19 +0200, Jan Lübbe a écrit :
> Your patch should catch those forged packets before more harmful things
> can go wrong, but even before my patch, i think forged packets could
> cause trouble...
Well, this is a debugging aid and wont avoid a crash later (since we
already made an out of bounds write, generally on a stack slot)
Of course, this might be a complete shot in the dark, but a
stackprotector fault in icmp_send() really sounds like a problem in
ip_options_echo() [ or bad input data given to this function ]
Other related changes (but as old as v2.6.22) :
commit 11a03f78fbf15a866ba
([NetLabel]: core network changes)
^ permalink raw reply
* Re: [PATCH v4] net: bnx2x: convert to hw_features
From: Vladislav Zolotarov @ 2011-04-12 14:36 UTC (permalink / raw)
To: Michał Mirosław; +Cc: netdev@vger.kernel.org, Eilon Greenstein
In-Reply-To: <20110412140708.GA21835@rere.qmqm.pl>
On Tue, 2011-04-12 at 07:07 -0700, Michał Mirosław wrote:
> On Tue, Apr 12, 2011 at 03:10:28PM +0300, Vladislav Zolotarov wrote:
> > On Mon, 2011-04-11 at 13:26 -0700, Michał Mirosław wrote:
> > > Since ndo_fix_features callback is postponing features change when
> > > bp->recovery_state != BNX2X_RECOVERY_DONE, netdev_update_features()
> > > has to be called again when this condition changes. Previously,
> > > ethtool_ops->set_flags callback returned -EBUSY in that case
> > > (it's not possible in the new model).
> > ACK (with reservations). ;)
> > Could u, pls., just add this comment I've asked for in the previous
> > e-mail?
>
> The one about vlan_features?
Yeah, this one.
> I'll just fix the comment in netdevice.h
> instead, since it might be not clear enough.
Could u still add this comment to bnx2x in your patch as well. It'll
just make these code section more clear regardless the netdevice.h
contents... ;)
>
> > The things I first thought to comment on are:
> > - Removing TPA_ENABLED_FLAG the similar way u've removed the
> > bp->rx_csum.
> > - Merging the code handling 'features' in bnx2x_init_bp() with the
> > similar code in bnx2x_init_dev().
> >
> > However I think it would be right if we clear our mess by ourselves and
> > that u have already done much enough... ;)
> >
> > I've run our standard test suite (which in particular heavily tests the
> > RX_CSUM and LRO flags toggling) on this patch and it passed it.
>
> It might be possible to get rid of ndo_set_features, since it looks like
> the reset/recovery handler is doing unload/load itself. This needs more
> digging into the driver than this simple conversion.
Hmm... I don't get u here. Although the recovery handler does
unload/load itself if there has been an attempt to change features
during the recovery it won't be able to get applied until the whole
recovery process ends. So, this patch added the extra call for
netdev_update_features() right after we know that the recovery process
has successfully ended. So, could u, pls., explain exactly what do u
mean here by saying "It might be possible to get rid of
ndo_set_features"?
thanks,
vlad
>
> Best Regards,
> Michał Mirosław
>
^ permalink raw reply
* Re: Kernel panic when using bridge
From: Jan Lübbe @ 2011-04-12 14:19 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Scot Doyle, Stephen Hemminger, Hiroaki SHIMODA, netdev
In-Reply-To: <1302614145.3233.47.camel@edumazet-laptop>
On Tue, 2011-04-12 at 15:15 +0200, Eric Dumazet wrote:
> Le mardi 12 avril 2011 à 15:02 +0200, Jan Lübbe a écrit :
> > Here you check dopt->optlen, which certainly should be 40 at most. The
> > calculation of dopt->optlen wasn't changed by my patch, though.
>
> Check again the thread Jan.
>
> Scot is using a tool (IP Stack Checker's tcpsic) to forge random tcp
> packets.
> Maybe your patch is fine but requires a change in a previous function,
> to make sure we deny some crazy packet before generating an ip_options
> with more than 40 bytes, in an icmp_send() reply.
One thing which could expose a problem is that it now will timestamp the
packet in the last 'slot', too. (which it didn't before)
In general, there is not a lot of error-checking in the options stuff.
> I took a look at this ip_options stuff and must say its really hard to
> even _read_ the code. Understanding it might need several days or a new
> brain ?
It took me some days do even figure out how it is supposed to fit
together...
> I cannot Ack or Nack your patch, I must admit it. Isnt it frightening?
David Miller already declared this code as 'officially terrible'...
Your patch should catch those forged packets before more harmful things
can go wrong, but even before my patch, i think forged packets could
cause trouble...
Regards,
Jan
^ permalink raw reply
* [PATCH] net: vlan_features comment clarification
From: Michał Mirosław @ 2011-04-12 14:07 UTC (permalink / raw)
To: netdev
Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
---
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 09d2624..cb8178a 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -1035,7 +1035,7 @@ struct net_device {
u32 hw_features;
/* user-requested features */
u32 wanted_features;
- /* VLAN feature mask */
+ /* mask of features inheritable by VLAN devices */
u32 vlan_features;
/* Net device feature bits; if you change something,
--
1.7.2.5
^ permalink raw reply related
* Re: [PATCH v4] net: bnx2x: convert to hw_features
From: Michał Mirosław @ 2011-04-12 14:07 UTC (permalink / raw)
To: Vladislav Zolotarov; +Cc: netdev@vger.kernel.org, Eilon Greenstein
In-Reply-To: <1302610228.32697.298.camel@lb-tlvb-vladz>
On Tue, Apr 12, 2011 at 03:10:28PM +0300, Vladislav Zolotarov wrote:
> On Mon, 2011-04-11 at 13:26 -0700, Michał Mirosław wrote:
> > Since ndo_fix_features callback is postponing features change when
> > bp->recovery_state != BNX2X_RECOVERY_DONE, netdev_update_features()
> > has to be called again when this condition changes. Previously,
> > ethtool_ops->set_flags callback returned -EBUSY in that case
> > (it's not possible in the new model).
> ACK (with reservations). ;)
> Could u, pls., just add this comment I've asked for in the previous
> e-mail?
The one about vlan_features? I'll just fix the comment in netdevice.h
instead, since it might be not clear enough.
> The things I first thought to comment on are:
> - Removing TPA_ENABLED_FLAG the similar way u've removed the
> bp->rx_csum.
> - Merging the code handling 'features' in bnx2x_init_bp() with the
> similar code in bnx2x_init_dev().
>
> However I think it would be right if we clear our mess by ourselves and
> that u have already done much enough... ;)
>
> I've run our standard test suite (which in particular heavily tests the
> RX_CSUM and LRO flags toggling) on this patch and it passed it.
It might be possible to get rid of ndo_set_features, since it looks like
the reset/recovery handler is doing unload/load itself. This needs more
digging into the driver than this simple conversion.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: profile my proto_hook.function
From: Eric Dumazet @ 2011-04-12 14:03 UTC (permalink / raw)
To: zhou rui; +Cc: netdev
In-Reply-To: <BANLkTi==HczThr66Yr0-Rm-RkEUw44sqrg@mail.gmail.com>
Le mardi 12 avril 2011 à 22:00 +0800, zhou rui a écrit :
> hi
> I used a prot_hook(added via dev_add_pack,protocol=ETH_P_ALL) to
> process packets in kernel part.but the performance was bad
>
> ksoftirqd used almost 100% cpu on my 1G nic(intel e1000,w/o RSS) XEON
> 5400 hypertown machine.
>
> looks my packet processing is too slow. but how to profile this?
> i.e. how many CPU resources used in softirq and normal kernel protocol
> stack(tcp/ip,etc)
> and how many used in my packet processing function?
>
> and any idea to improve it?how about moving the processing function to driver?
You coud use perf tool, included in kernel sources
# cd ~your_kernel_sources
# cd tools/perf
# make
<run your workload, and record activity :>
# ./perf record -a -g sleep 10
Then later, you can take a look at results
./perf report
^ permalink raw reply
* profile my proto_hook.function
From: zhou rui @ 2011-04-12 14:00 UTC (permalink / raw)
To: netdev
hi
I used a prot_hook(added via dev_add_pack,protocol=ETH_P_ALL) to
process packets in kernel part.but the performance was bad
ksoftirqd used almost 100% cpu on my 1G nic(intel e1000,w/o RSS) XEON
5400 hypertown machine.
looks my packet processing is too slow. but how to profile this?
i.e. how many CPU resources used in softirq and normal kernel protocol
stack(tcp/ip,etc)
and how many used in my packet processing function?
and any idea to improve it?how about moving the processing function to driver?
thanks
rui
^ permalink raw reply
* Bonding/LACP on RTL8169sc/8110sc (R1869)
From: Jonathan Thibault @ 2011-04-12 13:35 UTC (permalink / raw)
To: netdev; +Cc: Francois Romieu
Greetings all,
I have a a pair of Jetway motherboards with an add-on 3Gbit LAN modules
and have been doing some testing for a linux router project. I found
that I while I can get LACP working properly using eth0 and eth1, using
the exact same setup with any of the other three lan fails. Is this a
known issue?
This is on Linux 2.6.32.10. Here is a blurb of dmesg showing the NICs
detected.
eth0: RTL8168b/8111b at 0xf8adc000, 00:30:18:ac:a6:80, XID 0c200000 IRQ 24
eth1: RTL8168b/8111b at 0xf8ae0000, 00:30:18:ac:a6:81, XID 0c200000 IRQ 25
eth2: RTL8169sc/8110sc at 0xf8ae4c00, 00:30:18:ae:34:3a, XID 18000000 IRQ 18
eth3: RTL8169sc/8110sc at 0xf8ae8800, 00:30:18:ae:34:3b, XID 18000000 IRQ 19
eth4: RTL8169sc/8110sc at 0xf8aec400, 00:30:18:ae:34:3c, XID 18000000 IRQ 16
I can probably manage this project using only eth0 and eth1 in LACP
configuration but I figured I'd give a heads up.
The interfaces seem able to detect when the link is up or down, bonding
removes and adds them to the LACP trunk but I can't get traffic through
them.
Kind regards,
Jonathan
^ permalink raw reply
* [PATCH] r8169: Be verbose when unable to load fw patch
From: Borislav Petkov @ 2011-04-12 13:32 UTC (permalink / raw)
To: Francois Romieu; +Cc: linux-kernel, Borislav Petkov, netdev
From: Borislav Petkov <borislav.petkov@amd.com>
When the driver fails loading the firmware, it doesn't say which patch
exactly it is missing. We have three different firmware images depending
on the hw revision, so mention the exact patch name it is unable to load
in the warning message.
Cc: Francois Romieu <romieu@fr.zoreil.com>
Cc: netdev@vger.kernel.org
Signed-off-by: Borislav Petkov <borislav.petkov@amd.com>
---
drivers/net/r8169.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index 493b0de..b068115 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -2248,7 +2248,7 @@ static void rtl8168d_1_hw_phy_config(struct rtl8169_private *tp)
rtl_writephy(tp, 0x05, 0x001b);
if ((rtl_readphy(tp, 0x06) != 0xbf00) ||
(rtl_apply_firmware(tp, FIRMWARE_8168D_1) < 0)) {
- netif_warn(tp, probe, tp->dev, "unable to apply firmware patch\n");
+ netif_warn(tp, probe, tp->dev, "unable to apply firmware patch %s\n", FIRMWARE_8168D_1);
}
rtl_writephy(tp, 0x1f, 0x0000);
@@ -2353,7 +2353,7 @@ static void rtl8168d_2_hw_phy_config(struct rtl8169_private *tp)
rtl_writephy(tp, 0x05, 0x001b);
if ((rtl_readphy(tp, 0x06) != 0xb300) ||
(rtl_apply_firmware(tp, FIRMWARE_8168D_2) < 0)) {
- netif_warn(tp, probe, tp->dev, "unable to apply firmware patch\n");
+ netif_warn(tp, probe, tp->dev, "unable to apply firmware patch %s\n", FIRMWARE_8168D_2);
}
rtl_writephy(tp, 0x1f, 0x0000);
@@ -2475,7 +2475,7 @@ static void rtl8105e_hw_phy_config(struct rtl8169_private *tp)
msleep(100);
if (rtl_apply_firmware(tp, FIRMWARE_8105E_1) < 0)
- netif_warn(tp, probe, tp->dev, "unable to apply firmware patch\n");
+ netif_warn(tp, probe, tp->dev, "unable to apply firmware patch %s\n", FIRMWARE_8105E_1);
rtl_writephy_batch(tp, phy_reg_init, ARRAY_SIZE(phy_reg_init));
}
--
1.7.4
^ permalink raw reply related
* Re: Kernel panic when using bridge
From: Eric Dumazet @ 2011-04-12 13:15 UTC (permalink / raw)
To: Jan Lübbe; +Cc: Scot Doyle, Stephen Hemminger, Hiroaki SHIMODA, netdev
In-Reply-To: <1302613353.30934.22.camel@polaris.local>
Le mardi 12 avril 2011 à 15:02 +0200, Jan Lübbe a écrit :
> Hi!
>
> On Tue, 2011-04-12 at 13:49 +0200, Eric Dumazet wrote:
> > Considering recent changes in ip_options_echo() I would suggest to add
> > following patch and/or revert commit 8628bd8af7c4c14f40
> > (ipv4: Fix IP timestamp option (IPOPT_TS_PRESPEC) handling in
> > ip_options_echo())
>
> I've read this thread, but I'm not sure why my patch is related to these
> kernel panics. The behavior is only changed for packets with the
> timestamp option in prespecified addresses mode. Even then, it shouldn't
> cause ip_options_build to write to unallocated memory later.
>
> > diff --git a/net/ipv4/ip_options.c b/net/ipv4/ip_options.c
> > index 28a736f..35f2bf9 100644
> > --- a/net/ipv4/ip_options.c
> > +++ b/net/ipv4/ip_options.c
> > @@ -200,6 +200,11 @@ int ip_options_echo(struct ip_options * dopt, struct sk_buff * skb)
> > *dptr++ = IPOPT_END;
> > dopt->optlen++;
> > }
> > + if (unlikely(dopt->optlen > 40)) {
> > + pr_err("ip_options_echo() fatal error optlen=%u > 40\n", dopt->optlen);
> > + print_hex_dump(KERN_ERR, "ip options: ", DUMP_PREFIX_OFFSET,
> > + 16, 1, dopt->__data, dopt->optlen, false);
> > + }
> > return 0;
> > }
>
> Here you check dopt->optlen, which certainly should be 40 at most. The
> calculation of dopt->optlen wasn't changed by my patch, though.
Check again the thread Jan.
Scot is using a tool (IP Stack Checker's tcpsic) to forge random tcp
packets.
Maybe your patch is fine but requires a change in a previous function,
to make sure we deny some crazy packet before generating an ip_options
with more than 40 bytes, in an icmp_send() reply.
I took a look at this ip_options stuff and must say its really hard to
even _read_ the code. Understanding it might need several days or a new
brain ?
I cannot Ack or Nack your patch, I must admit it. Isnt it frightening ?
^ permalink raw reply
* Re: Loopback and Nagle's algorithm
From: Eric Dumazet @ 2011-04-12 13:08 UTC (permalink / raw)
To: Jiri Kosina; +Cc: Adam McLaurin, Will Newton, linux-kernel, netdev
In-Reply-To: <alpine.LRH.2.00.1104121354010.6953@twin.jikos.cz>
Le mardi 12 avril 2011 à 13:54 +0200, Jiri Kosina a écrit :
> On Tue, 12 Apr 2011, Adam McLaurin wrote:
>
> > > It may be caused by an increase in context switch rate, as both sender
> > > and receiver are on the same machine.
> >
> > I'm not sure that's what's happening, since the box where I'm running
> > this test has 8 physical CPU's and 32 cores in total.
>
> Have you tried firing up the testcase under perf, to see what it reveals
> as the bottleneck?
>
CC netdev
This rings a bell here.
I suspect we hit mod_timer() / lock_timer_base() because of delack
timer constantly changing.
I remember raising this point last year :
http://kerneltrap.org/mailarchive/linux-netdev/2010/5/20/6277741
David answer :
http://kerneltrap.org/mailarchive/linux-netdev/2010/6/2/6278430
I am afraid no change was done...
^ permalink raw reply
* Re: Kernel panic when using bridge
From: Jan Lübbe @ 2011-04-12 13:02 UTC (permalink / raw)
To: Eric Dumazet; +Cc: Scot Doyle, Stephen Hemminger, Hiroaki SHIMODA, netdev
In-Reply-To: <1302608951.3233.33.camel@edumazet-laptop>
Hi!
On Tue, 2011-04-12 at 13:49 +0200, Eric Dumazet wrote:
> Considering recent changes in ip_options_echo() I would suggest to add
> following patch and/or revert commit 8628bd8af7c4c14f40
> (ipv4: Fix IP timestamp option (IPOPT_TS_PRESPEC) handling in
> ip_options_echo())
I've read this thread, but I'm not sure why my patch is related to these
kernel panics. The behavior is only changed for packets with the
timestamp option in prespecified addresses mode. Even then, it shouldn't
cause ip_options_build to write to unallocated memory later.
> diff --git a/net/ipv4/ip_options.c b/net/ipv4/ip_options.c
> index 28a736f..35f2bf9 100644
> --- a/net/ipv4/ip_options.c
> +++ b/net/ipv4/ip_options.c
> @@ -200,6 +200,11 @@ int ip_options_echo(struct ip_options * dopt, struct sk_buff * skb)
> *dptr++ = IPOPT_END;
> dopt->optlen++;
> }
> + if (unlikely(dopt->optlen > 40)) {
> + pr_err("ip_options_echo() fatal error optlen=%u > 40\n", dopt->optlen);
> + print_hex_dump(KERN_ERR, "ip options: ", DUMP_PREFIX_OFFSET,
> + 16, 1, dopt->__data, dopt->optlen, false);
> + }
> return 0;
> }
Here you check dopt->optlen, which certainly should be 40 at most. The
calculation of dopt->optlen wasn't changed by my patch, though.
Regards,
Jan
^ permalink raw reply
* Re: 2.6.39-rc2 boot crash
From: Patrick McHardy @ 2011-04-12 12:49 UTC (permalink / raw)
To: Evgeniy Polyakov
Cc: Eric B Munson, David Miller, dave, linux-kernel, gregkh,
ksrinivasan, NetDev
In-Reply-To: <20110411220657.GB5783@ioremap.net>
On 12.04.2011 00:06, Evgeniy Polyakov wrote:
> Hi.
>
> On Mon, Apr 11, 2011 at 05:07:47PM -0400, Eric B Munson (emunson@mgebm.net) wrote:
>>> I can't figure this out, the only thing that should have changed is the
>>> time the initial PROC_CN_MCAST_LISTEN message is received. Apparently
>>> at that point connector is not fully initialized yet. Please post your
>>> config and the full boot log. Thanks.
>>>
>>
>> I am still seeing this on Linus' tree, is there anything more I can do to help
>> track the problem?
Sorry, I had a hardware failure, I'm back working on this now.
> Patrick, do you need my assist on this bug?
Thanks, but I can meanwhile reproduce the problem, so I think I
should have a fix soon.
^ permalink raw reply
* Re: [PATCH v4] net: bnx2x: convert to hw_features
From: Vladislav Zolotarov @ 2011-04-12 12:10 UTC (permalink / raw)
To: Michał Mirosław; +Cc: netdev@vger.kernel.org, Eilon Greenstein
In-Reply-To: <20110411202630.C079D13909@rere.qmqm.pl>
On Mon, 2011-04-11 at 13:26 -0700, Michał Mirosław wrote:
> Since ndo_fix_features callback is postponing features change when
> bp->recovery_state != BNX2X_RECOVERY_DONE, netdev_update_features()
> has to be called again when this condition changes. Previously,
> ethtool_ops->set_flags callback returned -EBUSY in that case
> (it's not possible in the new model).
ACK (with reservations). ;)
Could u, pls., just add this comment I've asked for in the previous
e-mail?
The things I first thought to comment on are:
- Removing TPA_ENABLED_FLAG the similar way u've removed the
bp->rx_csum.
- Merging the code handling 'features' in bnx2x_init_bp() with the
similar code in bnx2x_init_dev().
However I think it would be right if we clear our mess by ourselves and
that u have already done much enough... ;)
I've run our standard test suite (which in particular heavily tests the
RX_CSUM and LRO flags toggling) on this patch and it passed it.
Thanks a lot, Michal.
vlad
>
> Signed-off-by: Michał Mirosław <mirq-linux@rere.qmqm.pl>
> ---
> v4: - complete bp->rx_csum -> NETIF_F_RXCSUM conversion
> - add check for failed ndo_set_features in ndo_open callback
> v3: - include NETIF_F_LRO in hw_features
> - don't call netdev_update_features() if bnx2x_nic_load() failed
> v2: - comment in ndo_fix_features callback
> ---
> drivers/net/bnx2x/bnx2x.h | 1 -
> drivers/net/bnx2x/bnx2x_cmn.c | 54 +++++++++++++++++++--
> drivers/net/bnx2x/bnx2x_cmn.h | 3 +
> drivers/net/bnx2x/bnx2x_ethtool.c | 95 -------------------------------------
> drivers/net/bnx2x/bnx2x_main.c | 41 +++++++++-------
> 5 files changed, 75 insertions(+), 119 deletions(-)
>
> diff --git a/drivers/net/bnx2x/bnx2x.h b/drivers/net/bnx2x/bnx2x.h
> index b7ff87b..fefd1d5 100644
> --- a/drivers/net/bnx2x/bnx2x.h
> +++ b/drivers/net/bnx2x/bnx2x.h
> @@ -918,7 +918,6 @@ struct bnx2x {
>
> int tx_ring_size;
>
> - u32 rx_csum;
> /* L2 header size + 2*VLANs (8 bytes) + LLC SNAP (8 bytes) */
> #define ETH_OVREHEAD (ETH_HLEN + 8 + 8)
> #define ETH_MIN_PACKET_SIZE 60
> diff --git a/drivers/net/bnx2x/bnx2x_cmn.c b/drivers/net/bnx2x/bnx2x_cmn.c
> index e83ac6d..7f49cf4 100644
> --- a/drivers/net/bnx2x/bnx2x_cmn.c
> +++ b/drivers/net/bnx2x/bnx2x_cmn.c
> @@ -640,7 +640,7 @@ reuse_rx:
>
> skb_checksum_none_assert(skb);
>
> - if (bp->rx_csum) {
> + if (bp->dev->features & NETIF_F_RXCSUM) {
> if (likely(BNX2X_RX_CSUM_OK(cqe)))
> skb->ip_summed = CHECKSUM_UNNECESSARY;
> else
> @@ -2443,11 +2443,21 @@ alloc_err:
>
> }
>
> +static int bnx2x_reload_if_running(struct net_device *dev)
> +{
> + struct bnx2x *bp = netdev_priv(dev);
> +
> + if (unlikely(!netif_running(dev)))
> + return 0;
> +
> + bnx2x_nic_unload(bp, UNLOAD_NORMAL);
> + return bnx2x_nic_load(bp, LOAD_NORMAL);
> +}
> +
> /* called with rtnl_lock */
> int bnx2x_change_mtu(struct net_device *dev, int new_mtu)
> {
> struct bnx2x *bp = netdev_priv(dev);
> - int rc = 0;
>
> if (bp->recovery_state != BNX2X_RECOVERY_DONE) {
> printk(KERN_ERR "Handling parity error recovery. Try again later\n");
> @@ -2464,12 +2474,44 @@ int bnx2x_change_mtu(struct net_device *dev, int new_mtu)
> */
> dev->mtu = new_mtu;
>
> - if (netif_running(dev)) {
> - bnx2x_nic_unload(bp, UNLOAD_NORMAL);
> - rc = bnx2x_nic_load(bp, LOAD_NORMAL);
> + return bnx2x_reload_if_running(dev);
> +}
> +
> +u32 bnx2x_fix_features(struct net_device *dev, u32 features)
> +{
> + struct bnx2x *bp = netdev_priv(dev);
> +
> + if (bp->recovery_state != BNX2X_RECOVERY_DONE) {
> + netdev_err(dev, "Handling parity error recovery. Try again later\n");
> +
> + /* Don't allow bnx2x_set_features() to be called now. */
> + return dev->features;
> + }
> +
> + /* TPA requires Rx CSUM offloading */
> + if (!(features & NETIF_F_RXCSUM) || bp->disable_tpa)
> + features &= ~NETIF_F_LRO;
> +
> + return features;
> +}
> +
> +int bnx2x_set_features(struct net_device *dev, u32 features)
> +{
> + struct bnx2x *bp = netdev_priv(dev);
> + u32 flags = bp->flags;
> +
> + if (features & NETIF_F_LRO)
> + flags |= TPA_ENABLE_FLAG;
> + else
> + flags &= ~TPA_ENABLE_FLAG;
> +
> + if (flags ^ bp->flags) {
> + bp->flags = flags;
> +
> + return bnx2x_reload_if_running(dev);
> }
>
> - return rc;
> + return 0;
> }
>
> void bnx2x_tx_timeout(struct net_device *dev)
> diff --git a/drivers/net/bnx2x/bnx2x_cmn.h b/drivers/net/bnx2x/bnx2x_cmn.h
> index 775fef0..1cdab69 100644
> --- a/drivers/net/bnx2x/bnx2x_cmn.h
> +++ b/drivers/net/bnx2x/bnx2x_cmn.h
> @@ -431,6 +431,9 @@ void bnx2x_free_mem_bp(struct bnx2x *bp);
> */
> int bnx2x_change_mtu(struct net_device *dev, int new_mtu);
>
> +u32 bnx2x_fix_features(struct net_device *dev, u32 features);
> +int bnx2x_set_features(struct net_device *dev, u32 features);
> +
> /**
> * tx timeout netdev callback
> *
> diff --git a/drivers/net/bnx2x/bnx2x_ethtool.c b/drivers/net/bnx2x/bnx2x_ethtool.c
> index 1479994..ad7d91e 100644
> --- a/drivers/net/bnx2x/bnx2x_ethtool.c
> +++ b/drivers/net/bnx2x/bnx2x_ethtool.c
> @@ -1299,91 +1299,6 @@ static int bnx2x_set_pauseparam(struct net_device *dev,
> return 0;
> }
>
> -static int bnx2x_set_flags(struct net_device *dev, u32 data)
> -{
> - struct bnx2x *bp = netdev_priv(dev);
> - int changed = 0;
> - int rc = 0;
> -
> - if (bp->recovery_state != BNX2X_RECOVERY_DONE) {
> - printk(KERN_ERR "Handling parity error recovery. Try again later\n");
> - return -EAGAIN;
> - }
> -
> - if (!(data & ETH_FLAG_RXVLAN))
> - return -EINVAL;
> -
> - if ((data & ETH_FLAG_LRO) && bp->rx_csum && bp->disable_tpa)
> - return -EINVAL;
> -
> - rc = ethtool_op_set_flags(dev, data, ETH_FLAG_LRO | ETH_FLAG_RXVLAN |
> - ETH_FLAG_TXVLAN | ETH_FLAG_RXHASH);
> - if (rc)
> - return rc;
> -
> - /* TPA requires Rx CSUM offloading */
> - if ((data & ETH_FLAG_LRO) && bp->rx_csum) {
> - if (!(bp->flags & TPA_ENABLE_FLAG)) {
> - bp->flags |= TPA_ENABLE_FLAG;
> - changed = 1;
> - }
> - } else if (bp->flags & TPA_ENABLE_FLAG) {
> - dev->features &= ~NETIF_F_LRO;
> - bp->flags &= ~TPA_ENABLE_FLAG;
> - changed = 1;
> - }
> -
> - if (changed && netif_running(dev)) {
> - bnx2x_nic_unload(bp, UNLOAD_NORMAL);
> - rc = bnx2x_nic_load(bp, LOAD_NORMAL);
> - }
> -
> - return rc;
> -}
> -
> -static u32 bnx2x_get_rx_csum(struct net_device *dev)
> -{
> - struct bnx2x *bp = netdev_priv(dev);
> -
> - return bp->rx_csum;
> -}
> -
> -static int bnx2x_set_rx_csum(struct net_device *dev, u32 data)
> -{
> - struct bnx2x *bp = netdev_priv(dev);
> - int rc = 0;
> -
> - if (bp->recovery_state != BNX2X_RECOVERY_DONE) {
> - printk(KERN_ERR "Handling parity error recovery. Try again later\n");
> - return -EAGAIN;
> - }
> -
> - bp->rx_csum = data;
> -
> - /* Disable TPA, when Rx CSUM is disabled. Otherwise all
> - TPA'ed packets will be discarded due to wrong TCP CSUM */
> - if (!data) {
> - u32 flags = ethtool_op_get_flags(dev);
> -
> - rc = bnx2x_set_flags(dev, (flags & ~ETH_FLAG_LRO));
> - }
> -
> - return rc;
> -}
> -
> -static int bnx2x_set_tso(struct net_device *dev, u32 data)
> -{
> - if (data) {
> - dev->features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
> - dev->features |= NETIF_F_TSO6;
> - } else {
> - dev->features &= ~(NETIF_F_TSO | NETIF_F_TSO_ECN);
> - dev->features &= ~NETIF_F_TSO6;
> - }
> -
> - return 0;
> -}
> -
> static const struct {
> char string[ETH_GSTRING_LEN];
> } bnx2x_tests_str_arr[BNX2X_NUM_TESTS] = {
> @@ -2207,16 +2122,6 @@ static const struct ethtool_ops bnx2x_ethtool_ops = {
> .set_ringparam = bnx2x_set_ringparam,
> .get_pauseparam = bnx2x_get_pauseparam,
> .set_pauseparam = bnx2x_set_pauseparam,
> - .get_rx_csum = bnx2x_get_rx_csum,
> - .set_rx_csum = bnx2x_set_rx_csum,
> - .get_tx_csum = ethtool_op_get_tx_csum,
> - .set_tx_csum = ethtool_op_set_tx_hw_csum,
> - .set_flags = bnx2x_set_flags,
> - .get_flags = ethtool_op_get_flags,
> - .get_sg = ethtool_op_get_sg,
> - .set_sg = ethtool_op_set_sg,
> - .get_tso = ethtool_op_get_tso,
> - .set_tso = bnx2x_set_tso,
> .self_test = bnx2x_self_test,
> .get_sset_count = bnx2x_get_sset_count,
> .get_strings = bnx2x_get_strings,
> diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c
> index f3cf889..5fd7cbb 100644
> --- a/drivers/net/bnx2x/bnx2x_main.c
> +++ b/drivers/net/bnx2x/bnx2x_main.c
> @@ -7728,6 +7728,7 @@ static void bnx2x_parity_recover(struct bnx2x *bp)
> return;
> }
>
> + netdev_update_features(bp->dev);
> return;
> }
> } else { /* non-leader */
> @@ -7755,10 +7756,12 @@ static void bnx2x_parity_recover(struct bnx2x *bp)
> * the "process kill". It's an exit
> * point for a non-leader.
> */
> - bnx2x_nic_load(bp, LOAD_NORMAL);
> + int rc = bnx2x_nic_load(bp, LOAD_NORMAL);
> bp->recovery_state =
> BNX2X_RECOVERY_DONE;
> smp_wmb();
> + if (!rc)
> + netdev_update_features(bp->dev);
> return;
> }
> }
> @@ -8904,8 +8907,6 @@ static int __devinit bnx2x_init_bp(struct bnx2x *bp)
> bp->multi_mode = multi_mode;
> bp->int_mode = int_mode;
>
> - bp->dev->features |= NETIF_F_GRO;
> -
> /* Set TPA flags */
> if (disable_tpa) {
> bp->flags &= ~TPA_ENABLE_FLAG;
> @@ -8925,8 +8926,6 @@ static int __devinit bnx2x_init_bp(struct bnx2x *bp)
>
> bp->tx_ring_size = MAX_TX_AVAIL;
>
> - bp->rx_csum = 1;
> -
> /* make sure that the numbers are in the right granularity */
> bp->tx_ticks = (50 / BNX2X_BTR) * BNX2X_BTR;
> bp->rx_ticks = (25 / BNX2X_BTR) * BNX2X_BTR;
> @@ -8954,6 +8953,7 @@ static int __devinit bnx2x_init_bp(struct bnx2x *bp)
> static int bnx2x_open(struct net_device *dev)
> {
> struct bnx2x *bp = netdev_priv(dev);
> + int rc;
>
> netif_carrier_off(dev);
>
> @@ -8993,7 +8993,14 @@ static int bnx2x_open(struct net_device *dev)
>
> bp->recovery_state = BNX2X_RECOVERY_DONE;
>
> - return bnx2x_nic_load(bp, LOAD_OPEN);
> + rc = bnx2x_nic_load(bp, LOAD_OPEN);
> + if (!rc) {
> + netdev_update_features(bp->dev);
> + if (bp->state != BNX2X_STATE_OPEN)
> + return -EBUSY;
> + }
> +
> + return rc;
> }
>
> /* called with rtnl_lock */
> @@ -9304,6 +9311,8 @@ static const struct net_device_ops bnx2x_netdev_ops = {
> .ndo_validate_addr = eth_validate_addr,
> .ndo_do_ioctl = bnx2x_ioctl,
> .ndo_change_mtu = bnx2x_change_mtu,
> + .ndo_fix_features = bnx2x_fix_features,
> + .ndo_set_features = bnx2x_set_features,
> .ndo_tx_timeout = bnx2x_tx_timeout,
> #ifdef CONFIG_NET_POLL_CONTROLLER
> .ndo_poll_controller = poll_bnx2x,
> @@ -9430,20 +9439,18 @@ static int __devinit bnx2x_init_dev(struct pci_dev *pdev,
>
> dev->netdev_ops = &bnx2x_netdev_ops;
> bnx2x_set_ethtool_ops(dev);
> - dev->features |= NETIF_F_SG;
> - dev->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
> +
> if (bp->flags & USING_DAC_FLAG)
> dev->features |= NETIF_F_HIGHDMA;
> - dev->features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
> - dev->features |= NETIF_F_TSO6;
> - dev->features |= (NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX);
>
> - dev->vlan_features |= NETIF_F_SG;
> - dev->vlan_features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM;
> - if (bp->flags & USING_DAC_FLAG)
> - dev->vlan_features |= NETIF_F_HIGHDMA;
> - dev->vlan_features |= (NETIF_F_TSO | NETIF_F_TSO_ECN);
> - dev->vlan_features |= NETIF_F_TSO6;
> + dev->hw_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> + NETIF_F_TSO | NETIF_F_TSO_ECN | NETIF_F_TSO6 |
> + NETIF_F_RXCSUM | NETIF_F_LRO | NETIF_F_HW_VLAN_TX;
> +
> + dev->features |= dev->hw_features | NETIF_F_HW_VLAN_RX;
> +
> + dev->vlan_features = NETIF_F_SG | NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM |
> + NETIF_F_TSO | NETIF_F_TSO_ECN | NETIF_F_TSO6 | NETIF_F_HIGHDMA;
>
> #ifdef BCM_DCBNL
> dev->dcbnl_ops = &bnx2x_dcbnl_ops;
^ permalink raw reply
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