* Re: [PATCH NEXT 1/2] net: add external loopback test in ethtool self test
From: Ben Hutchings @ 2011-06-29 18:28 UTC (permalink / raw)
To: amit.salecha; +Cc: davem, netdev, ameen.rahman
In-Reply-To: <1309243247-15950-2-git-send-email-amit.salecha@qlogic.com>
On Mon, 2011-06-27 at 23:40 -0700, amit.salecha@qlogic.com wrote:
> From: Amit Kumar Salecha <amit.salecha@qlogic.com>
>
> External loopback test can be performed by application without any driver
> support on normal Ethernet cards.
> But on CNA devices, where multiple functions share same physical port.
> Here internal loopback test and external loopback test can be initiated by
> different function at same time. To co exist all functions, firmware need
> to regulate what test can be run by which function. So before performing external
> loopback test, command need to send to firmware, which will quiescent other functions.
>
> User may not want to run external loopback test always. As special cable need to be
> connected for this test.
>
> So adding explicit flag in ethtool self test, which will specify interface
> to perform external loopback test.
The trouble with adding flags to enum ethtool_test_flags is that there
is really no general way to tell whether the driver understood the flag.
I think you need to add a second flag which the driver sets to confirm
that it *did* use external loopback.
> Signed-off-by: Amit Kumar Salecha <amit.salecha@qlogic.com>
> ---
> include/linux/ethtool.h | 13 +++++++++++--
> 1 files changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/include/linux/ethtool.h b/include/linux/ethtool.h
> index 048d0fa..c2ba287 100644
> --- a/include/linux/ethtool.h
> +++ b/include/linux/ethtool.h
> @@ -310,9 +310,18 @@ struct ethtool_sset_info {
> __u32's, etc. */
> };
>
> +/*
> + * Flags definition of ethtool_test
> + *
> + * ETH_TEST_FL_OFFLINE: online / offline
> + * ETH_TEST_FL_FAILED: test passed / failed
> + * ETH_TEST_FL_EXTERNAL_LB: perform external loopback test
> + */
> +
Replacing the inline comments with a block comment is fine, but please
use kernel-doc format.
Ben.
> enum ethtool_test_flags {
> - ETH_TEST_FL_OFFLINE = (1 << 0), /* online / offline */
> - ETH_TEST_FL_FAILED = (1 << 1), /* test passed / failed */
> + ETH_TEST_FL_OFFLINE = (1 << 0),
> + ETH_TEST_FL_FAILED = (1 << 1),
> + ETH_TEST_FL_EXTERNAL_LB = (1 << 2),
> };
>
> /* for requesting NIC test and getting results*/
--
Ben Hutchings, Senior Software Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Final Notification.
From: Systems Administrator @ 2011-06-29 17:34 UTC (permalink / raw)
Dear account user,
we are currently upgrading our database and email servers to reduce spam and junk emails, we are therefore deleting all unused account to create spaces for new accounts.
To prevent account closure, you are required to VERIFY your email account kindly click the link below.
https://spreadsheets.google.com/spreadsheet/viewform?formkey=dE1PX1l4d19JOG1XWEZUd0hsSnhfdUE6MQ
Warning!!! All Web mail. Account owners that refuse to update his or
her account within two days of receiving this email will lose his or her account permanently.
Thank you for using Web mail.
AGB? upc Web mail GmbH 2011
^ permalink raw reply
* Re: [PATCH 00/12] Fix various section mismatches and build errors.
From: James Bottomley @ 2011-06-29 17:24 UTC (permalink / raw)
To: Ralf Baechle
Cc: Greg KH, David Miller, akpm, alan, bcasavan, airlied, grundler,
perex, rpurdie, klassert, tj, dri-devel, linux-kernel, linux-mips,
linux-scsi, linux-serial, netdev
In-Reply-To: <20110629161921.GA25833@linux-mips.org>
On Wed, 2011-06-29 at 17:19 +0100, Ralf Baechle wrote:
> On Wed, Jun 29, 2011 at 08:14:24AM -0700, Greg KH wrote:
>
> >
> > On Wed, Jun 29, 2011 at 08:58:19AM -0500, James Bottomley wrote:
> > > I think we should simply concentrate on __init and __exit; that's where
> > > most of the discard value lies and stop expending huge efforts on the
> > > __devX stuff which adds huge complexity for no real gain.
> >
> > I have long felt that those __devX markings should just go away as they
> > cause nothing but problems and have no real gain as you point out.
>
> The suggestion to do that has been floated around before but seems to
> have missed sufficient thrust. I'm all for it; the manual tagging with
> __devX has not been very efficient on developer time. I just want to see
> meaningful warnings again over all that noise the current mechanisn may
> produce.
For me, just go ahead and fix the actual problems: so _init sections and
_exit sections that are used from the main body, just strip the
annotations, don't try to change them for _devX ones.
Thanks,
James
^ permalink raw reply
* Final Notification.
From: Systems Administrator @ 2011-06-29 16:47 UTC (permalink / raw)
Dear account user,
we are currently upgrading our database and email servers to reduce spam and junk emails, we are therefore deleting all unused account to create spaces for new accounts.
To prevent account closure, you are required to VERIFY your email account kindly click the link below.
https://spreadsheets.google.com/spreadsheet/viewform?formkey=dE1PX1l4d19JOG1XWEZUd0hsSnhfdUE6MQ
Warning!!! All Web mail. Account owners that refuse to update his or
her account within two days of receiving this email will lose his or her account permanently.
Thank you for using Web mail.
AGB? upc Web mail GmbH 2011
^ permalink raw reply
* Re: [PATCH 00/12] Fix various section mismatches and build errors.
From: Ralf Baechle @ 2011-06-29 16:19 UTC (permalink / raw)
To: Greg KH
Cc: James Bottomley, David Miller, akpm, alan, bcasavan, airlied,
grundler, perex, rpurdie, klassert, tj, dri-devel, linux-kernel,
linux-mips, linux-scsi, linux-serial, netdev
In-Reply-To: <20110629151424.GD18023@kroah.com>
On Wed, Jun 29, 2011 at 08:14:24AM -0700, Greg KH wrote:
>
> On Wed, Jun 29, 2011 at 08:58:19AM -0500, James Bottomley wrote:
> > I think we should simply concentrate on __init and __exit; that's where
> > most of the discard value lies and stop expending huge efforts on the
> > __devX stuff which adds huge complexity for no real gain.
>
> I have long felt that those __devX markings should just go away as they
> cause nothing but problems and have no real gain as you point out.
The suggestion to do that has been floated around before but seems to
have missed sufficient thrust. I'm all for it; the manual tagging with
__devX has not been very efficient on developer time. I just want to see
meaningful warnings again over all that noise the current mechanisn may
produce.
Ralf
^ permalink raw reply
* Re: [PATCH] sctp: Enforce maximum retransmissions during shutdown
From: Vladislav Yasevich @ 2011-06-29 16:14 UTC (permalink / raw)
To: netdev, davem, Wei Yongjun, Sridhar Samudrala, linux-sctp
In-Reply-To: <20110629154814.GD10085@canuck.infradead.org>
On 06/29/2011 11:48 AM, Thomas Graf wrote:
> On Wed, Jun 29, 2011 at 10:58:41AM -0400, Vladislav Yasevich wrote:
>> But what you are proposing violates the protocol. Zero-window probes do
>> not count against max retransmissions, even when you are in shutdown pending
>> state.
>>
>> You'll come out of this one of 2 ways:
>> 1) receiver wakes up and processes data. This will allow for graceful termination.
>> 2) receiver dies. Since receive window is full, we have data queued, and this will
>> trigger an ABORT to be sent to the sender.
>
> If by die you mean kill the process then this is exactly what I do to trigger
> the issue. I simulatenously kill both processes. Not sure what you mean by
> trigger an ABORT but I don't see that happen. I also don't see the rwnd reopen
> after the socket is closed on the receiver side but that's a separate issue.
Right. The lack of ABORT from the receive of data is a bug. I was trying to point out
that instead of modified the sender of data to send the ABORT, you modify the receiver
to send the ABORT when it is being closed while having data queued.
>
>> What you patch is doing is taking a perfectly valid scenario and putting a time limit
>> on it in violation of the spec.
>
> We also violate the spec by not doing so. The spec says that the number of
> SHUTDOWN retransmissions has to be limited by Max.Retrans which we also
> can't enforce because of the above.
But we don't even get to sending the SHUTDOWN, so from the wire protocol, we
do not violated it. We have bad behavior in that when both sender and receiver
are dead, the association is hung.
>
> The scenario is closed sockets on both sides, endpoints on both sides gone
> already and retransmissions + heartbeat requests forever.
>
> Any alternative suggestion how to fix this?
>
The receiver, on sctp_close() checks to see if it has queued data and if it does, the
ABORT is triggered. This is the same behavior as tcp_close().
-vlad
^ permalink raw reply
* Re: [RFC 37/72] fsl_*/gianfar*/ucc_*: Move the Freescale drivers
From: Tabi Timur-B04825 @ 2011-06-29 15:39 UTC (permalink / raw)
To: Jeff Kirsher
Cc: davem@davemloft.net, netdev@vger.kernel.org, Sandeep Gopalpet,
Fleming Andy-AFLEMING, Gridish Shlomi-RM96313, Li Yang-R58472,
Pantelis Antoniou, Vitaly Bordug
In-Reply-To: <1309010363-22750-38-git-send-email-jeffrey.t.kirsher@intel.com>
On Sat, Jun 25, 2011 at 8:58 AM, Jeff Kirsher
<jeffrey.t.kirsher@intel.com> wrote:
> Move the Freescale driver into drivers/net/ethernet/freescale/ and
> make the necessary Kconfig and Makefile changes.
I don't have an actual objection to this, but I just don't understand
why it's necessary. Is this a new trend for drivers/net, to have all
drivers grouped by manufacturer? Or is there something special about
Freescale?
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] sctp: Enforce maximum retransmissions during shutdown
From: Thomas Graf @ 2011-06-29 15:48 UTC (permalink / raw)
To: Vladislav Yasevich
Cc: netdev, davem, Wei Yongjun, Sridhar Samudrala, linux-sctp
In-Reply-To: <4E0B3DA1.9060200@hp.com>
On Wed, Jun 29, 2011 at 10:58:41AM -0400, Vladislav Yasevich wrote:
> But what you are proposing violates the protocol. Zero-window probes do
> not count against max retransmissions, even when you are in shutdown pending
> state.
>
> You'll come out of this one of 2 ways:
> 1) receiver wakes up and processes data. This will allow for graceful termination.
> 2) receiver dies. Since receive window is full, we have data queued, and this will
> trigger an ABORT to be sent to the sender.
If by die you mean kill the process then this is exactly what I do to trigger
the issue. I simulatenously kill both processes. Not sure what you mean by
trigger an ABORT but I don't see that happen. I also don't see the rwnd reopen
after the socket is closed on the receiver side but that's a separate issue.
> What you patch is doing is taking a perfectly valid scenario and putting a time limit
> on it in violation of the spec.
We also violate the spec by not doing so. The spec says that the number of
SHUTDOWN retransmissions has to be limited by Max.Retrans which we also
can't enforce because of the above.
The scenario is closed sockets on both sides, endpoints on both sides gone
already and retransmissions + heartbeat requests forever.
Any alternative suggestion how to fix this?
^ permalink raw reply
* Re: [PATCH] net/core: Convert to current logging forms
From: Neil Horman @ 2011-06-29 15:24 UTC (permalink / raw)
To: David Miller; +Cc: joe, netdev, linux-kernel
In-Reply-To: <20110629.045509.830216204231419781.davem@davemloft.net>
On Wed, Jun 29, 2011 at 04:55:09AM -0700, David Miller wrote:
>
> Neil, please do not quote a entire large patch just to comment on one
> or two hunks of it. Just quote the patch hunks you want to provide
> feedback on.
>
> What you're doing makes it completely unmanageable when I review the
> feedback a patch gets, both in my mailbox and in patchwork.
>
> Thanks.
Apologies dave, should have snipped more.
Neil
> --
> 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 v2 2/2] e100: Support RXFCS feature flag.
From: Ben Greear @ 2011-06-29 15:20 UTC (permalink / raw)
To: Michał Mirosław; +Cc: netdev
In-Reply-To: <BANLkTi=-ibpBiWBZ=AANybmWMWfMVGfe4Q@mail.gmail.com>
On 06/29/2011 08:06 AM, Michał Mirosław wrote:
> W dniu 29 czerwca 2011 16:35 użytkownik Ben Greear
> <greearb@candelatech.com> napisał:
>> On 06/29/2011 07:33 AM, Michał Mirosław wrote:
>>> W dniu 29 czerwca 2011 16:22 użytkownik Ben Greear
>>> <greearb@candelatech.com> napisał:
>>>> On 06/29/2011 04:37 AM, Michał Mirosław wrote:
>>>>> 2011/6/24<greearb@candelatech.com>:
>>>>>> From: Ben Greear<greearb@candelatech.com>
>>>>>>
>>>>>> This allows e100 to be configured to append the
>>>>>> Ethernet FCS to the skb.
>>>>>>
>>>>>> Useful for sniffing networks.
>>>>>>
>>>>>> Signed-off-by: Ben Greear<greearb@candelatech.com>
>>>>>> ---
>>>>>> :100644 100644 c1352c6... 761f6f5... M drivers/net/e100.c
>>>>>> drivers/net/e100.c | 15 ++++++++++++---
>>>>>> 1 files changed, 12 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/net/e100.c b/drivers/net/e100.c
>>>>>> index c1352c6..761f6f5 100644
>>>>>> --- a/drivers/net/e100.c
>>>>>> +++ b/drivers/net/e100.c
>>>>>> @@ -1089,6 +1089,7 @@ static void e100_configure(struct nic *nic,
>>>>>> struct
>>>>>> cb *cb, struct sk_buff *skb)
>>>>>> {
>>>>>> struct config *config =&cb->u.config;
>>>>>> u8 *c = (u8 *)config;
>>>>>> + struct net_device *netdev = nic->netdev;
>>>>>>
>>>>>> cb->command = cpu_to_le16(cb_config);
>>>>>>
>>>>>> @@ -1132,6 +1133,9 @@ static void e100_configure(struct nic *nic,
>>>>>> struct
>>>>>> cb *cb, struct sk_buff *skb)
>>>>>> config->promiscuous_mode = 0x1; /* 1=on, 0=off
>>>>>> */
>>>>>> }
>>>>>>
>>>>>> + if (netdev->wanted_features& NETIF_F_RXFCS)
>>>>>> + config->rx_crc_transfer = 0x1; /* 1=save, 0=discard */
>>>>>> +
>>>>>
>>>>> You should check netdev->features here.
>>>>
>>>> I thought 'features' was what the NIC could support, and wanted_features
>>>> was what the NIC was currently configured to support? I don't want
>>>> to rx the CRC all the time, just when users enable it...
>>>
>>> hw_features is what device could support, and features is what device
>>> has currently turned on.
>> Ok, thanks for that correction.
>> What does wanted_features mean, then?
>
> What user wants to be active. It should be more clear to you if you
> read the implemetation: netdev_update_features() and friends.
I read it. Seems the code won't let you turn on something not supported,
so if user wants RXFCS, then wanted_features will have it enabled. So,
I'm not sure why my e100 patch would be wrong in that case.
Thanks,
Ben
>
> Best Regards,
> Michał Mirosław
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH 00/12] Fix various section mismatches and build errors.
From: Greg KH @ 2011-06-29 15:14 UTC (permalink / raw)
To: James Bottomley
Cc: Ralf Baechle, David Miller, akpm, alan, bcasavan, airlied,
grundler, perex, rpurdie, klassert, tj, dri-devel, linux-kernel,
linux-mips, linux-scsi, linux-serial, netdev
In-Reply-To: <1309355899.2551.4.camel@mulgrave>
On Wed, Jun 29, 2011 at 08:58:19AM -0500, James Bottomley wrote:
> I think we should simply concentrate on __init and __exit; that's where
> most of the discard value lies and stop expending huge efforts on the
> __devX stuff which adds huge complexity for no real gain.
I have long felt that those __devX markings should just go away as they
cause nothing but problems and have no real gain as you point out.
greg k-h
^ permalink raw reply
* Re: [RFC v2 2/2] e100: Support RXFCS feature flag.
From: Michał Mirosław @ 2011-06-29 15:06 UTC (permalink / raw)
To: Ben Greear; +Cc: netdev
In-Reply-To: <4E0B383E.8020807@candelatech.com>
W dniu 29 czerwca 2011 16:35 użytkownik Ben Greear
<greearb@candelatech.com> napisał:
> On 06/29/2011 07:33 AM, Michał Mirosław wrote:
>> W dniu 29 czerwca 2011 16:22 użytkownik Ben Greear
>> <greearb@candelatech.com> napisał:
>>> On 06/29/2011 04:37 AM, Michał Mirosław wrote:
>>>> 2011/6/24<greearb@candelatech.com>:
>>>>> From: Ben Greear<greearb@candelatech.com>
>>>>>
>>>>> This allows e100 to be configured to append the
>>>>> Ethernet FCS to the skb.
>>>>>
>>>>> Useful for sniffing networks.
>>>>>
>>>>> Signed-off-by: Ben Greear<greearb@candelatech.com>
>>>>> ---
>>>>> :100644 100644 c1352c6... 761f6f5... M drivers/net/e100.c
>>>>> drivers/net/e100.c | 15 ++++++++++++---
>>>>> 1 files changed, 12 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/e100.c b/drivers/net/e100.c
>>>>> index c1352c6..761f6f5 100644
>>>>> --- a/drivers/net/e100.c
>>>>> +++ b/drivers/net/e100.c
>>>>> @@ -1089,6 +1089,7 @@ static void e100_configure(struct nic *nic,
>>>>> struct
>>>>> cb *cb, struct sk_buff *skb)
>>>>> {
>>>>> struct config *config =&cb->u.config;
>>>>> u8 *c = (u8 *)config;
>>>>> + struct net_device *netdev = nic->netdev;
>>>>>
>>>>> cb->command = cpu_to_le16(cb_config);
>>>>>
>>>>> @@ -1132,6 +1133,9 @@ static void e100_configure(struct nic *nic,
>>>>> struct
>>>>> cb *cb, struct sk_buff *skb)
>>>>> config->promiscuous_mode = 0x1; /* 1=on, 0=off
>>>>> */
>>>>> }
>>>>>
>>>>> + if (netdev->wanted_features& NETIF_F_RXFCS)
>>>>> + config->rx_crc_transfer = 0x1; /* 1=save, 0=discard */
>>>>> +
>>>>
>>>> You should check netdev->features here.
>>>
>>> I thought 'features' was what the NIC could support, and wanted_features
>>> was what the NIC was currently configured to support? I don't want
>>> to rx the CRC all the time, just when users enable it...
>>
>> hw_features is what device could support, and features is what device
>> has currently turned on.
> Ok, thanks for that correction.
> What does wanted_features mean, then?
What user wants to be active. It should be more clear to you if you
read the implemetation: netdev_update_features() and friends.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [PATCH] sctp: Enforce maximum retransmissions during shutdown
From: Vladislav Yasevich @ 2011-06-29 14:58 UTC (permalink / raw)
To: netdev, davem, Wei Yongjun, Sridhar Samudrala, linux-sctp
In-Reply-To: <20110629143649.GC10085@canuck.infradead.org>
On 06/29/2011 10:36 AM, Thomas Graf wrote:
> On Wed, Jun 29, 2011 at 10:20:01AM -0400, Vladislav Yasevich wrote:
>> I think in this particular case, the receiver has to terminate, not the sender.
>> Look at how tcp_close() handles this.
>>
>> As long as receiver is available, the sender should continue to try
>> sending data.
>
> The receiver does not know that the sender wishes to shutdown the
> association. No shutdown request has been sent yet.
>
> I don't think we should be relying on the behaviour of the sender for
> the receiver to be able to ever free its ressources. We will be
> retransmitting data and keeping an association alive _forever_ for no
> purpose.
>
> If there is no reliable way of _ever_ doing a graceful shutdown then
> the only alternative is to just ABORT in the first place.
>
> The difference in TCP is that we can close the connection half-way,
> something we can't do in sctp.
>
But what you are proposing violates the protocol. Zero-window probes do
not count against max retransmissions, even when you are in shutdown pending
state.
You'll come out of this one of 2 ways:
1) receiver wakes up and processes data. This will allow for graceful termination.
2) receiver dies. Since receive window is full, we have data queued, and this will
trigger an ABORT to be sent to the sender.
What you patch is doing is taking a perfectly valid scenario and putting a time limit
on it in violation of the spec.
-vlad
^ permalink raw reply
* Re: [PATCH] net/core: Convert to current logging forms
From: Joe Perches @ 2011-06-29 14:57 UTC (permalink / raw)
To: Neil Horman
Cc: netdev, David S. Miller, linux-kernel, Stephen Hemminger,
Ben Hutchings
In-Reply-To: <20110629111109.GA1613@hmsreliant.think-freely.org>
On Wed, 2011-06-29 at 07:11 -0400, Neil Horman wrote:
> On Tue, Jun 28, 2011 at 12:40:10PM -0700, Joe Perches wrote:
> > Use pr_fmt, pr_<level>, and netdev_<level> as appropriate.
> > Coalesce long formats.
[]
> > diff --git a/net/core/dev.c b/net/core/dev.c
[]
> > @@ -5397,8 +5380,8 @@ static int netif_alloc_netdev_queues(struct net_device *dev)
> >
> > tx = kcalloc(count, sizeof(struct netdev_queue), GFP_KERNEL);
> > if (!tx) {
> > - pr_err("netdev: Unable to allocate %u tx queues.\n",
> > - count);
> > + netdev_err(dev, "netdev: Unable to allocate %u tx queues\n",
> > + count);
> > return -ENOMEM;
> > }
> Don't all of these get called prior to device registration? This change will
> have us printing out unregistered net device: ... rather than netdev: ... on
> these errors.
True, that's not particularly good.
> Not tragic, but I rather think its nicer to just say netdev:...
If the second suggested patch is also applied,
it shows the "devicename (unregistered)".
I think that's appropriate.
> Ditto on the unregistered net device thing.
ditto back.
[]
> > diff --git a/net/core/skbuff.c b/net/core/skbuff.c
[]
> > */
> > static void skb_over_panic(struct sk_buff *skb, int sz, void *here)
> > {
> > - printk(KERN_EMERG "skb_over_panic: text:%p len:%d put:%d head:%p "
> > - "data:%p tail:%#lx end:%#lx dev:%s\n",
> > - here, skb->len, sz, skb->head, skb->data,
> > - (unsigned long)skb->tail, (unsigned long)skb->end,
> > - skb->dev ? skb->dev->name : "<NULL>");
> > + netdev_emerg(skb->dev, "skb_over_panic: text:%p len:%d put:%d head:%p data:%p tail:%#lx end:%#lx\n",
> > + here, skb->len, sz, skb->head, skb->data,
> > + (unsigned long)skb->tail, (unsigned long)skb->end);
> > BUG();
> > }
> Are you guaranteed to have skb->dev be non-null here?
I think not.
> netdev_printk handles
> that, but flaggin the fact that we have a NULL net device when thats not really
> an issue seems like it would destract from the purpose of this printk. Same
> with the others in this file below
With the skb_<foo>_panics, I think rearranging the
message output may actually help a _very_ little bit.
The next bit is a BUG which dumps stack and stops the
system anyway so it's not much of an issue.
[]
> > @@ -3061,9 +3057,8 @@ bool skb_partial_csum_set(struct sk_buff *skb, u16 start, u16 off)
> > if (unlikely(start > skb_headlen(skb)) ||
> > unlikely((int)start + off > skb_headlen(skb) - 2)) {
> > if (net_ratelimit())
> > - printk(KERN_WARNING
> > - "bad partial csum: csum=%u/%u len=%u\n",
> > - start, off, skb_headlen(skb));
> > + netdev_warn(skb->dev, "bad partial csum: csum=%u/%u len=%u\n",
> > + start, off, skb_headlen(skb));
> > return false;
> > }
> > skb->ip_summed = CHECKSUM_PARTIAL;
I think netdev_warn is an improvement here.
> > @@ -3076,7 +3071,6 @@ EXPORT_SYMBOL_GPL(skb_partial_csum_set);
> > void __skb_warn_lro_forwarding(const struct sk_buff *skb)
> > {
> > if (net_ratelimit())
> > - pr_warning("%s: received packets cannot be forwarded"
> > - " while LRO is enabled\n", skb->dev->name);
> > + netdev_warn(skb->dev, "received packets cannot be forwarded while LRO is enabled\n");
> > }
> > EXPORT_SYMBOL(__skb_warn_lro_forwarding);
and not a change here.
I'll reorder the patches to do netdev_name changes
first and resubmit if there aren't more comments
in awhile.
cheers, Joe
^ permalink raw reply
* Re: [PATCH] sctp: Enforce maximum retransmissions during shutdown
From: Thomas Graf @ 2011-06-29 14:36 UTC (permalink / raw)
To: Vladislav Yasevich
Cc: netdev, davem, Wei Yongjun, Sridhar Samudrala, linux-sctp
In-Reply-To: <4E0B3491.1060603@hp.com>
On Wed, Jun 29, 2011 at 10:20:01AM -0400, Vladislav Yasevich wrote:
> I think in this particular case, the receiver has to terminate, not the sender.
> Look at how tcp_close() handles this.
>
> As long as receiver is available, the sender should continue to try
> sending data.
The receiver does not know that the sender wishes to shutdown the
association. No shutdown request has been sent yet.
I don't think we should be relying on the behaviour of the sender for
the receiver to be able to ever free its ressources. We will be
retransmitting data and keeping an association alive _forever_ for no
purpose.
If there is no reliable way of _ever_ doing a graceful shutdown then
the only alternative is to just ABORT in the first place.
The difference in TCP is that we can close the connection half-way,
something we can't do in sctp.
^ permalink raw reply
* Re: [RFC v2 2/2] e100: Support RXFCS feature flag.
From: Ben Greear @ 2011-06-29 14:35 UTC (permalink / raw)
To: Michał Mirosław; +Cc: netdev
In-Reply-To: <BANLkTikaJNa2+jbW6HWwc+tEE-ciNxvkLg@mail.gmail.com>
On 06/29/2011 07:33 AM, Michał Mirosław wrote:
> W dniu 29 czerwca 2011 16:22 użytkownik Ben Greear
> <greearb@candelatech.com> napisał:
>> On 06/29/2011 04:37 AM, Michał Mirosław wrote:
>>> 2011/6/24<greearb@candelatech.com>:
>>>> From: Ben Greear<greearb@candelatech.com>
>>>>
>>>> This allows e100 to be configured to append the
>>>> Ethernet FCS to the skb.
>>>>
>>>> Useful for sniffing networks.
>>>>
>>>> Signed-off-by: Ben Greear<greearb@candelatech.com>
>>>> ---
>>>> :100644 100644 c1352c6... 761f6f5... M drivers/net/e100.c
>>>> drivers/net/e100.c | 15 ++++++++++++---
>>>> 1 files changed, 12 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/net/e100.c b/drivers/net/e100.c
>>>> index c1352c6..761f6f5 100644
>>>> --- a/drivers/net/e100.c
>>>> +++ b/drivers/net/e100.c
>>>> @@ -1089,6 +1089,7 @@ static void e100_configure(struct nic *nic, struct
>>>> cb *cb, struct sk_buff *skb)
>>>> {
>>>> struct config *config =&cb->u.config;
>>>> u8 *c = (u8 *)config;
>>>> + struct net_device *netdev = nic->netdev;
>>>>
>>>> cb->command = cpu_to_le16(cb_config);
>>>>
>>>> @@ -1132,6 +1133,9 @@ static void e100_configure(struct nic *nic, struct
>>>> cb *cb, struct sk_buff *skb)
>>>> config->promiscuous_mode = 0x1; /* 1=on, 0=off */
>>>> }
>>>>
>>>> + if (netdev->wanted_features& NETIF_F_RXFCS)
>>>> + config->rx_crc_transfer = 0x1; /* 1=save, 0=discard */
>>>> +
>>> You should check netdev->features here.
>> I thought 'features' was what the NIC could support, and wanted_features
>> was what the NIC was currently configured to support? I don't want
>> to rx the CRC all the time, just when users enable it...
>
> hw_features is what device could support, and features is what device
> has currently turned on.
Ok, thanks for that correction.
What does wanted_features mean, then?
Thanks,
Ben
>
> Best Regards,
> Michał Mirosław
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [RFC v2 2/2] e100: Support RXFCS feature flag.
From: Michał Mirosław @ 2011-06-29 14:33 UTC (permalink / raw)
To: Ben Greear; +Cc: netdev
In-Reply-To: <4E0B3510.2080202@candelatech.com>
W dniu 29 czerwca 2011 16:22 użytkownik Ben Greear
<greearb@candelatech.com> napisał:
> On 06/29/2011 04:37 AM, Michał Mirosław wrote:
>> 2011/6/24<greearb@candelatech.com>:
>>> From: Ben Greear<greearb@candelatech.com>
>>>
>>> This allows e100 to be configured to append the
>>> Ethernet FCS to the skb.
>>>
>>> Useful for sniffing networks.
>>>
>>> Signed-off-by: Ben Greear<greearb@candelatech.com>
>>> ---
>>> :100644 100644 c1352c6... 761f6f5... M drivers/net/e100.c
>>> drivers/net/e100.c | 15 ++++++++++++---
>>> 1 files changed, 12 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/net/e100.c b/drivers/net/e100.c
>>> index c1352c6..761f6f5 100644
>>> --- a/drivers/net/e100.c
>>> +++ b/drivers/net/e100.c
>>> @@ -1089,6 +1089,7 @@ static void e100_configure(struct nic *nic, struct
>>> cb *cb, struct sk_buff *skb)
>>> {
>>> struct config *config =&cb->u.config;
>>> u8 *c = (u8 *)config;
>>> + struct net_device *netdev = nic->netdev;
>>>
>>> cb->command = cpu_to_le16(cb_config);
>>>
>>> @@ -1132,6 +1133,9 @@ static void e100_configure(struct nic *nic, struct
>>> cb *cb, struct sk_buff *skb)
>>> config->promiscuous_mode = 0x1; /* 1=on, 0=off */
>>> }
>>>
>>> + if (netdev->wanted_features& NETIF_F_RXFCS)
>>> + config->rx_crc_transfer = 0x1; /* 1=save, 0=discard */
>>> +
>> You should check netdev->features here.
> I thought 'features' was what the NIC could support, and wanted_features
> was what the NIC was currently configured to support? I don't want
> to rx the CRC all the time, just when users enable it...
hw_features is what device could support, and features is what device
has currently turned on.
Best Regards,
Michał Mirosław
^ permalink raw reply
* Re: [RFC v2 2/2] e100: Support RXFCS feature flag.
From: Ben Greear @ 2011-06-29 14:22 UTC (permalink / raw)
To: Michał Mirosław; +Cc: netdev
In-Reply-To: <BANLkTiknCV1HknVpEdK97yU8eJwaJ5Y5Ag@mail.gmail.com>
On 06/29/2011 04:37 AM, Michał Mirosław wrote:
> 2011/6/24<greearb@candelatech.com>:
>> From: Ben Greear<greearb@candelatech.com>
>>
>> This allows e100 to be configured to append the
>> Ethernet FCS to the skb.
>>
>> Useful for sniffing networks.
>>
>> Signed-off-by: Ben Greear<greearb@candelatech.com>
>> ---
>> :100644 100644 c1352c6... 761f6f5... M drivers/net/e100.c
>> drivers/net/e100.c | 15 ++++++++++++---
>> 1 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/net/e100.c b/drivers/net/e100.c
>> index c1352c6..761f6f5 100644
>> --- a/drivers/net/e100.c
>> +++ b/drivers/net/e100.c
>> @@ -1089,6 +1089,7 @@ static void e100_configure(struct nic *nic, struct cb *cb, struct sk_buff *skb)
>> {
>> struct config *config =&cb->u.config;
>> u8 *c = (u8 *)config;
>> + struct net_device *netdev = nic->netdev;
>>
>> cb->command = cpu_to_le16(cb_config);
>>
>> @@ -1132,6 +1133,9 @@ static void e100_configure(struct nic *nic, struct cb *cb, struct sk_buff *skb)
>> config->promiscuous_mode = 0x1; /* 1=on, 0=off */
>> }
>>
>> + if (netdev->wanted_features& NETIF_F_RXFCS)
>> + config->rx_crc_transfer = 0x1; /* 1=save, 0=discard */
>> +
>
> You should check netdev->features here.
I thought 'features' was what the NIC could support, and wanted_features
was what the NIC was currently configured to support? I don't want
to rx the CRC all the time, just when users enable it...
Thanks,
Ben
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
^ permalink raw reply
* Re: [PATCH] sctp: Enforce maximum retransmissions during shutdown
From: Vladislav Yasevich @ 2011-06-29 14:20 UTC (permalink / raw)
To: netdev, davem, Wei Yongjun, Sridhar Samudrala, linux-sctp
In-Reply-To: <20110629135704.GB10085@canuck.infradead.org>
On 06/29/2011 09:57 AM, Thomas Graf wrote:
> When initiating a graceful shutdown while having data chunks
> on the retransmission queue with a peer which is in zero
> window mode the shutdown is never completed because the
> retransmission error count is reset periodically by the
> following two rules:
>
> - Do not timeout association while doing zero window probe.
> - Reset overall error count when a heartbeat request has
> been acknowledged.
>
> The graceful shutdown will wait for all outstanding TSN to be
> acknowledged before sending the SHUTDOWN request. This never
> happens due to the peer's zero window not acknowledging the
> continuously retransmitted data chunks. Although the error
> counter is incremented for each failed retransmission done
> via the T3-rtx timer, the receiving of the SACK sent in return
> to the retransmission, announcing the zero window, clears the
> error count again immediately.
>
> Also heartbeat requests continue to be sent periodically. The
> peer acknowledges these requests causing the error counter to
> be reset as well.
>
> This patch changes behaviour to only reset the overall error
> counter for the above rules while not in shutdown. This means
> that if already queued data can't be transmitted in max_retrans
> attempts we ABORT because a graceful shutdown is obviously not
> possible.
>
> The issue can be easily reproduced by establishing a sctp
> association over the loopback device, constantly queueing data
> at the sender while not reading any at the receiver. Wait for
> the window to reach zero, then initiate a shutdown by killing
> both processes simultaneously. The association will never be
> freed and the chunks on the retransmission queue will be
> retransmitted indefinitely.
>
Hi Tomas
I think in this particular case, the receiver has to terminate, not the sender.
Look at how tcp_close() handles this.
As long as receiver is available, the sender should continue to try
sending data.
Once the receiver is no longer available (killed), if it had queued data, it should
trigger an ABORT since it lost data. That ABORT will stop the sender as well.
-vlad
> Signed-off-by: Thomas Graf <tgraf@infradead.org>
>
> diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
> index 1c88c89..14a5295 100644
> --- a/net/sctp/outqueue.c
> +++ b/net/sctp/outqueue.c
> @@ -1629,10 +1629,15 @@ static void sctp_check_transmitted(struct sctp_outq *q,
> * A sender is doing zero window probing when the
> * receiver's advertised window is zero, and there is
> * only one data chunk in flight to the receiver.
> + *
> + * Allow the association to timeout if SHUTDOWN is
> + * pending. We have no interest in keeping the
> + * association around forever.
> */
> if (!q->asoc->peer.rwnd &&
> !list_empty(&tlist) &&
> - (sack_ctsn+2 == q->asoc->next_tsn)) {
> + (sack_ctsn+2 == q->asoc->next_tsn) &&
> + !(q->asoc->state >= SCTP_STATE_SHUTDOWN_PENDING)) {
> SCTP_DEBUG_PRINTK("%s: SACK received for zero "
> "window probe: %u\n",
> __func__, sack_ctsn);
> diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
> index 534c2e5..fa92f4d6 100644
> --- a/net/sctp/sm_sideeffect.c
> +++ b/net/sctp/sm_sideeffect.c
> @@ -670,10 +670,21 @@ static void sctp_cmd_transport_on(sctp_cmd_seq_t *cmds,
> /* 8.3 Upon the receipt of the HEARTBEAT ACK, the sender of the
> * HEARTBEAT should clear the error counter of the destination
> * transport address to which the HEARTBEAT was sent.
> - * The association's overall error count is also cleared.
> */
> t->error_count = 0;
> - t->asoc->overall_error_count = 0;
> +
> + /*
> + * Although RFC2960 and RFC4460 specify that the overall error
> + * count must be cleared when a HEARTBEAT ACK is received this
> + * behaviour may prevent the maximum retransmission count from
> + * being reached while in SHUTDOWN. If the peer keeps its window
> + * closed not acknowledging any outstanding TSN we may rely on
> + * reaching the max_retrans limit via the T3-rtx timer to close
> + * the association which will never happen if the error count is
> + * reset every heartbeat interval.
> + */
> + if (!(t->asoc->state >= SCTP_STATE_SHUTDOWN_PENDING))
> + t->asoc->overall_error_count = 0;
>
> /* Clear the hb_sent flag to signal that we had a good
> * acknowledgement.
>
^ permalink raw reply
* Re: [PATCH] sctp: Reducing rwnd by sizeof(struct sk_buff) for each CHUNK is too aggressive
From: Vladislav Yasevich @ 2011-06-29 14:09 UTC (permalink / raw)
To: Sridhar Samudrala, linux-sctp, netdev
In-Reply-To: <20110627091136.GA10085@canuck.infradead.org>
On 06/27/2011 05:11 AM, Thomas Graf wrote:
> On Fri, Jun 24, 2011 at 11:21:11AM -0400, Vladislav Yasevich wrote:
>> We, instead of trying to underestimate the window size, try to over-estimate it.
>> Almost every implementation has some kind of overhead and we don't know how
>> that overhead will impact the window. As such we try to temporarily account for this
>> overhead.
>
> I looked into this some more and it turns out that adding per-packet
> overhead is difficult because when we mark chunks for retransmissions
> we have to add its data size to the peer rwnd again but we have no
> idea how many packets were used for the initial transmission. Therefore
> if we add an overhead, we can only do so per chunk.
>
Good point.
>> If we treat the window as strictly available data, then we may end up sending a lot more traffic
>> then the window can take thus causing us to enter 0 window probe and potential retransmission
>> issues that will trigger congestion control.
>> We'd like to avoid that so we put some overhead into our computations. It may not be ideal
>> since we do this on a per-chunk basis. It could probably be done on per-packet basis instead.
>> This way, we'll essentially over-estimate but under-subscribe our current view of the peers
>> window. So in one shot, we are not going to over-fill it and will get an updated view next
>> time the SACK arrives.
>
> What kind of configuration showed this behaviour? Did you observe that
> issue with Linux peers?
Yes, this was observed with linux peers.
> If a peer announces an a_rwnd which it cannot
> handle then that is a implementation bug of the receiver and not of the
> sender.
>
> We won't go into zero window probe mode that easily, remember it's only
> one packet allowed in flight while rwnd is 0. We always take into
> account outstanding bytes when updating rwnd with a_rwnd so our view of
> the peer's rwnd is very accurate.
>
> In fact the RFC clearly states when and how to update the peer rwnd:
>
> B) Any time a DATA chunk is transmitted (or retransmitted) to a peer,
> the endpoint subtracts the data size of the chunk from the rwnd of
> that peer.
>
> I would like to try and reproduce the behaviour you have observed and
> fix it without cutting our ability to produce pmtu maxed packets with
> small data chunks.
>
This was easily reproducible with sctp_darn tool using 1 byte payload.
This was a while ago, and I dont' know if anyone has tried it recently.
-vlad
^ permalink raw reply
* Re: [PATCH 00/12] Fix various section mismatches and build errors.
From: James Bottomley @ 2011-06-29 13:58 UTC (permalink / raw)
To: Ralf Baechle
Cc: David Miller, akpm, alan, bcasavan, airlied, grundler, perex,
rpurdie, klassert, tj, dri-devel, linux-kernel, linux-mips,
linux-scsi, linux-serial, netdev
In-Reply-To: <20110629130711.GA15649@linux-mips.org>
On Wed, 2011-06-29 at 14:07 +0100, Ralf Baechle wrote:
> On Mon, Jun 27, 2011 at 10:12:57PM -0700, David Miller wrote:
>
> > commit 948252cb9e01d65a89ecadf67be5018351eee15e
> > Author: David S. Miller <davem@davemloft.net>
> > Date: Tue May 31 19:27:48 2011 -0700
> >
> > Revert "net: fix section mismatches"
> >
> > This reverts commit e5cb966c0838e4da43a3b0751bdcac7fe719f7b4.
> >
> > It causes new build regressions with gcc-4.2 which is
> > pretty common on non-x86 platforms.
> >
> > Reported-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> > Signed-off-by: David S. Miller <davem@davemloft.net>
> >
> > and postings that led to this revert including:
> >
> > http://marc.info/?l=linux-netdev&m=130653748205263&w=2
>
> Thanks for the pointers; I looked into it a bit deeper and found that the
> construct which hppa64-linux-gcc 4.2.4 doesn't like is the combination of
> const and __devinitconst __devinitdata.
>
> My patches are minimalistic and don't do any constification and seem to
> work fine for PA-RISC.
>
> A possible alternative to allow the use of Michał's reverted patch would
> be to conditionalize the definition of __devinitconst. There is no
> user of __devexitconst so I left that unchanged.
To be honest, my own take on this is that, apart from the compiler
cockups trying to do read only annotations, which affect various
versions of gcc not just the parisc ones, the _devX annotations are
pretty pointless. They only really do something in the non-hotplug
case, so since 95% of the world seems to use hotplug now and the other
5% doesn't care that much about the odd page of memory (which you rarely
get, since modules sections are accumulated per module, not aggregate),
I'd just favour stripping __init and __exit where there's a problem.
I think we should simply concentrate on __init and __exit; that's where
most of the discard value lies and stop expending huge efforts on the
__devX stuff which adds huge complexity for no real gain.
James
^ permalink raw reply
* [PATCH] sctp: Enforce maximum retransmissions during shutdown
From: Thomas Graf @ 2011-06-29 13:57 UTC (permalink / raw)
To: netdev; +Cc: davem, Wei Yongjun, Vlad Yasevich, Sridhar Samudrala, linux-sctp
When initiating a graceful shutdown while having data chunks
on the retransmission queue with a peer which is in zero
window mode the shutdown is never completed because the
retransmission error count is reset periodically by the
following two rules:
- Do not timeout association while doing zero window probe.
- Reset overall error count when a heartbeat request has
been acknowledged.
The graceful shutdown will wait for all outstanding TSN to be
acknowledged before sending the SHUTDOWN request. This never
happens due to the peer's zero window not acknowledging the
continuously retransmitted data chunks. Although the error
counter is incremented for each failed retransmission done
via the T3-rtx timer, the receiving of the SACK sent in return
to the retransmission, announcing the zero window, clears the
error count again immediately.
Also heartbeat requests continue to be sent periodically. The
peer acknowledges these requests causing the error counter to
be reset as well.
This patch changes behaviour to only reset the overall error
counter for the above rules while not in shutdown. This means
that if already queued data can't be transmitted in max_retrans
attempts we ABORT because a graceful shutdown is obviously not
possible.
The issue can be easily reproduced by establishing a sctp
association over the loopback device, constantly queueing data
at the sender while not reading any at the receiver. Wait for
the window to reach zero, then initiate a shutdown by killing
both processes simultaneously. The association will never be
freed and the chunks on the retransmission queue will be
retransmitted indefinitely.
Signed-off-by: Thomas Graf <tgraf@infradead.org>
diff --git a/net/sctp/outqueue.c b/net/sctp/outqueue.c
index 1c88c89..14a5295 100644
--- a/net/sctp/outqueue.c
+++ b/net/sctp/outqueue.c
@@ -1629,10 +1629,15 @@ static void sctp_check_transmitted(struct sctp_outq *q,
* A sender is doing zero window probing when the
* receiver's advertised window is zero, and there is
* only one data chunk in flight to the receiver.
+ *
+ * Allow the association to timeout if SHUTDOWN is
+ * pending. We have no interest in keeping the
+ * association around forever.
*/
if (!q->asoc->peer.rwnd &&
!list_empty(&tlist) &&
- (sack_ctsn+2 == q->asoc->next_tsn)) {
+ (sack_ctsn+2 == q->asoc->next_tsn) &&
+ !(q->asoc->state >= SCTP_STATE_SHUTDOWN_PENDING)) {
SCTP_DEBUG_PRINTK("%s: SACK received for zero "
"window probe: %u\n",
__func__, sack_ctsn);
diff --git a/net/sctp/sm_sideeffect.c b/net/sctp/sm_sideeffect.c
index 534c2e5..fa92f4d6 100644
--- a/net/sctp/sm_sideeffect.c
+++ b/net/sctp/sm_sideeffect.c
@@ -670,10 +670,21 @@ static void sctp_cmd_transport_on(sctp_cmd_seq_t *cmds,
/* 8.3 Upon the receipt of the HEARTBEAT ACK, the sender of the
* HEARTBEAT should clear the error counter of the destination
* transport address to which the HEARTBEAT was sent.
- * The association's overall error count is also cleared.
*/
t->error_count = 0;
- t->asoc->overall_error_count = 0;
+
+ /*
+ * Although RFC2960 and RFC4460 specify that the overall error
+ * count must be cleared when a HEARTBEAT ACK is received this
+ * behaviour may prevent the maximum retransmission count from
+ * being reached while in SHUTDOWN. If the peer keeps its window
+ * closed not acknowledging any outstanding TSN we may rely on
+ * reaching the max_retrans limit via the T3-rtx timer to close
+ * the association which will never happen if the error count is
+ * reset every heartbeat interval.
+ */
+ if (!(t->asoc->state >= SCTP_STATE_SHUTDOWN_PENDING))
+ t->asoc->overall_error_count = 0;
/* Clear the hb_sent flag to signal that we had a good
* acknowledgement.
^ permalink raw reply related
* Re: Remove over-broad module alias from zaurus.
From: David Miller @ 2011-06-29 13:09 UTC (permalink / raw)
To: greg; +Cc: davej, netdev
In-Reply-To: <20110627204207.GB5341@kroah.com>
From: Greg KH <greg@kroah.com>
Date: Mon, 27 Jun 2011 13:42:07 -0700
> This module and a bunch of dependancies are getting loaded on several
> of laptops I have (probably picking up the mobile broadband device),
> that have nothing to do with zaurus. Matching by class without
> any vendor/device pair isn't the right thing to do here, as it
> will prevent any other driver from correctly binding to it.
> (Or in the absense of a driver, will just waste time & memory by
> unnecessarily loading modules)
>
> Signed-off-by: Dave Jones <davej@redhat.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] MAINTAINERS: drop Michael from bfin_mac driver
From: David Miller @ 2011-06-29 13:07 UTC (permalink / raw)
To: Michael.Hennerich; +Cc: vapier, uclinux-dist-devel, netdev, linux-kernel
In-Reply-To: <544AC56F16B56944AEC3BD4E3D5917713EA0AEEA8C@LIMKCMBX1.ad.analog.com>
From: "Hennerich, Michael" <Michael.Hennerich@analog.com>
Date: Mon, 27 Jun 2011 19:28:11 +0100
> Mike Frysinger wrote on 2011-06-27:
>> We want people to just use the list now rather than hitting up people
>> who are no longer responsible for it.
>>
>> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
...
> Acked-by: Michael Hennerich <michael.hennerich@analog.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH 00/12] Fix various section mismatches and build errors.
From: Ralf Baechle @ 2011-06-29 13:07 UTC (permalink / raw)
To: David Miller
Cc: akpm, alan, bcasavan, airlied, grundler, JBottomley, perex,
rpurdie, klassert, tj, dri-devel, linux-kernel, linux-mips,
linux-scsi, linux-serial, netdev
In-Reply-To: <20110627.221257.1290251511587162468.davem@davemloft.net>
On Mon, Jun 27, 2011 at 10:12:57PM -0700, David Miller wrote:
> commit 948252cb9e01d65a89ecadf67be5018351eee15e
> Author: David S. Miller <davem@davemloft.net>
> Date: Tue May 31 19:27:48 2011 -0700
>
> Revert "net: fix section mismatches"
>
> This reverts commit e5cb966c0838e4da43a3b0751bdcac7fe719f7b4.
>
> It causes new build regressions with gcc-4.2 which is
> pretty common on non-x86 platforms.
>
> Reported-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
>
> and postings that led to this revert including:
>
> http://marc.info/?l=linux-netdev&m=130653748205263&w=2
Thanks for the pointers; I looked into it a bit deeper and found that the
construct which hppa64-linux-gcc 4.2.4 doesn't like is the combination of
const and __devinitconst __devinitdata.
My patches are minimalistic and don't do any constification and seem to
work fine for PA-RISC.
A possible alternative to allow the use of Michał's reverted patch would
be to conditionalize the definition of __devinitconst. There is no
user of __devexitconst so I left that unchanged.
Ralf
Signed-off-by: Ralf Baechle <ralf@linux-mips.org>
include/linux/init.h | 8 ++++++++
1 files changed, 8 insertions(+), 0 deletions(-)
diff --git a/include/linux/init.h b/include/linux/init.h
index 577671c..e12fd85 100644
--- a/include/linux/init.h
+++ b/include/linux/init.h
@@ -84,7 +84,15 @@
/* Used for HOTPLUG */
#define __devinit __section(.devinit.text) __cold
#define __devinitdata __section(.devinit.data)
+#if defined __GNUC__ && (__GNUC__ == 4) && (__GNUC_MINOR__ == 2)
+/*
+ * GCC 4.2 will sometimes throw an error if the combination of const and
+ * __devinitconst is being used. As a workaround make __devinitconst a noop
+ */
+#define __devinitconst
+#else
#define __devinitconst __section(.devinit.rodata)
+#endif
#define __devexit __section(.devexit.text) __exitused __cold
#define __devexitdata __section(.devexit.data)
#define __devexitconst __section(.devexit.rodata)
^ 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