* Re: [PATCH] bridge: Forward reserved group addresses if !STP
From: Stephen Hemminger @ 2010-10-19 3:28 UTC (permalink / raw)
To: Benjamin Poirier
Cc: David S. Miller, Herbert Xu, Eric Dumazet, Jiri Pirko, bridge,
netdev, linux-kernel
In-Reply-To: <1287454175-22903-1-git-send-email-benjamin.poirier@polymtl.ca>
On Mon, 18 Oct 2010 22:09:35 -0400
Benjamin Poirier <benjamin.poirier@polymtl.ca> wrote:
> Make all frames sent to reserved group MAC addresses (01:80:c2:00:00:00 to
> 01:80:c2:00:00:0f) be forwarded if STP is disabled. This enables
> forwarding EAPOL frames, among other things.
>
> Signed-off-by: Benjamin Poirier <benjamin.poirier@polymtl.ca>
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
^ permalink raw reply
* Re: [RFC PATCH 1/7] ebtables: Allow filtering of hardware accelerated vlan frames.
From: Jesse Gross @ 2010-10-19 3:14 UTC (permalink / raw)
To: Ben Hutchings; +Cc: davem, netdev
In-Reply-To: <1287431892.2252.575.camel@achroite.uk.solarflarecom.com>
On Mon, Oct 18, 2010 at 12:58 PM, Ben Hutchings
<bhutchings@solarflare.com> wrote:
> On Wed, 2010-10-13 at 13:02 -0700, Jesse Gross wrote:
>> An upcoming commit will allow packets with hardware vlan acceleration
>> information to be passed though more parts of the network stack, including
>> packets trunked through the bridge. This adds support for matching and
>> filtering those packets through ebtables.
>>
>> Signed-off-by: Jesse Gross <jesse@nicira.com>
>> ---
>> net/bridge/br_netfilter.c | 16 +++++++++-------
>> net/bridge/netfilter/ebt_vlan.c | 38 +++++++++++++++++++++++---------------
>> net/bridge/netfilter/ebtables.c | 15 +++++++++++----
>> 3 files changed, 43 insertions(+), 26 deletions(-)
>>
>> diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c
>> index 7f9ce96..d6a4fec 100644
>> --- a/net/bridge/br_netfilter.c
>> +++ b/net/bridge/br_netfilter.c
>> @@ -64,22 +64,24 @@ static int brnf_filter_pppoe_tagged __read_mostly = 0;
>>
>> static inline __be16 vlan_proto(const struct sk_buff *skb)
>> {
>> - return vlan_eth_hdr(skb)->h_vlan_encapsulated_proto;
>> + if (skb->protocol == htons(ETH_P_8021Q))
>> + return vlan_eth_hdr(skb)->h_vlan_encapsulated_proto;
>> + else if (vlan_tx_tag_present(skb))
>> + return skb->protocol;
>
> If there are two levels of VLAN-encapsulation, this will return either
> the inner or outer tag depending on whether VLAN acceleration is being
> used. It should behave consistently.
Thanks, you're right. I fixed it to always use the outer tag (same as
previous behavior).
^ permalink raw reply
* Re: [Ksummit-2010-discuss] [v2] Remaining BKL users, what to do
From: Dave Airlie @ 2010-10-19 2:45 UTC (permalink / raw)
To: Greg KH
Cc: Arnd Bergmann, codalist, ksummit-2010-discuss, autofs, Jan Harkes,
Samuel Ortiz, Jan Kara, Arnaldo Carvalho de Melo, netdev,
Anders Larsen, linux-kernel, dri-devel, Bryan Schumaker,
Christoph Hellwig, Petr Vandrovec, Mikulas Patocka, linux-fsdevel,
Evgeniy Dushistov, Ingo Molnar, Andrew Hendry, linux-media
In-Reply-To: <20101019022413.GB30307@kroah.com>
On Tue, Oct 19, 2010 at 12:24 PM, Greg KH <greg@kroah.com> wrote:
> On Tue, Oct 19, 2010 at 10:57:43AM +1000, Dave Airlie wrote:
>> On Tue, Oct 19, 2010 at 10:40 AM, Greg KH <greg@kroah.com> wrote:
>> > On Tue, Oct 19, 2010 at 09:00:09AM +1000, Dave Airlie wrote:
>> >> On Tue, Oct 19, 2010 at 4:43 AM, Greg KH <greg@kroah.com> wrote:
>> >> > On Mon, Oct 18, 2010 at 05:42:06PM +0200, Arnd Bergmann wrote:
>> >> >>
>> >> >> Out of the remaining modules, I guess i810/i830, adfs, hpfs and ufs might end
>> >> >> up not getting fixed at all, we can either mark them non-SMP or move them
>> >> >> to drivers/staging once all the others are done.
>> >> >
>> >> > I recommend moving them to staging, and then retire them from there if
>> >> > no one steps up to maintain them.
>> >>
>> >> I think this sets a bad precedent, these drivers work fine. Removing
>> >> BKL from them is hard, and involves finding and booting hw that
>> >> developers don't have much time/interest in at the moment. Anyone who
>> >> has access to the i810 hw and has time to work out the locking has
>> >> more important things to be doing with modern hw, however it doesn't
>> >> mean we should just drop support for old drivers because they don't
>> >> have active maintainers. Removing the BKL from the kernel is a great
>> >> goal, but breaking userspace ABI by removing drivers isn't.
>> >
>> > Should we just restrict such drivers to only be able to build on UP
>> > machines with preempt disabled so that the BKL could be safely removed
>> > from them?
>> >
>> > Or what other idea do you have as to what could be done here?
>> >
>> > I do have access to this hardware, but its on an old single processor
>> > laptop, so any work that it would take to help do this development,
>> > really wouldn't be able to be tested to be valid at all.
>>
>> There is only very rare case where the i830 driver might get used with
>> SMP and really I think that case is in the don't care place, since if
>> you have that hw you probably should be using i915 on it anyways.
>
> So, there is no need for the i830 driver? Can it just be removed
> because i915 works instead?
No because it provides a different userspace ABI to the i915 driver to
a different userspace X driver etc.
like I'm sure the intersection of this driver and reality are getting
quite limited, but its still a userspace ABI change and needs to be
treated as such. Xorg 6.7 and XFree86 4.3 were the last users of the
old driver/API.
>> So it really only leaves the problem case of what do distros do if we
>> mark things as BROKEN_ON_SMP, since no distro builds UP kernels and
>> when you boot the SMP kernels on UP they don't run as SMP so not
>> having the driver load on those is a problem. Maybe we just need some
>> sort of warn on smp if a smp unfriendly driver is loaded and we
>> transition to SMP mode. Though this sounds like either (a) something
>> we do now and I don't about it, (b) work.
>
> So you are saying that just because distros will never build such a
> thing, we should keep it building for SMP mode? Why not prevent it from
> being built and if a distro really cares, then they will pony up the
> development to fix the driver up?
Distros build the driver now even it it didn't work on SMP it wouldn't
matter to the 99% of people who have this hw since it can't suppport
SMP except in some corner cases. So not building for SMP is the same
as just throwing it out of the kernel since most people don't run
kernel.org kernels, and shouldn't have to just to get a driver for
some piece of hardware that worked fine up until now.
Look at this from a user who has this hardware pov, it works for them
now with a distro kernel, us breaking it isn't going to help that user
or make any distro care, its just going to screw over the people who
are actually using it.
> In other words, if someone really cares, then they will do the work,
> otherwise why worry? Especially as it seems that no one here is going
> to do it, right?
Well the thing is doing the work right is a non-trivial task and just
dropping support only screws the people using the hardware,
it doesn't place any burden on the distro developers to fix it up. If
people are really serious about making the BKL go away completely, I
think the onus should be on them to fix the drivers not on the users
who are using it, like I'm guessing if this gets broken the bug will
end up in Novell or RH bugzilla in a year and nobody will ever see it.
Dave.
^ permalink raw reply
* Re: [Ksummit-2010-discuss] [v2] Remaining BKL users, what to do
From: Greg KH @ 2010-10-19 2:24 UTC (permalink / raw)
To: Dave Airlie
Cc: Arnd Bergmann, codalist, ksummit-2010-discuss, autofs, Jan Harkes,
Samuel Ortiz, Jan Kara, Arnaldo Carvalho de Melo, netdev,
Anders Larsen, linux-kernel, dri-devel, Bryan Schumaker,
Christoph Hellwig, Petr Vandrovec, Mikulas Patocka, linux-fsdevel,
Evgeniy Dushistov, Ingo Molnar, Andrew Hendry, linux-media
In-Reply-To: <AANLkTi=ffaihP5-yNYFKAbAbX+XbRgWRXXfCZd4J3KwQ@mail.gmail.com>
On Tue, Oct 19, 2010 at 10:57:43AM +1000, Dave Airlie wrote:
> On Tue, Oct 19, 2010 at 10:40 AM, Greg KH <greg@kroah.com> wrote:
> > On Tue, Oct 19, 2010 at 09:00:09AM +1000, Dave Airlie wrote:
> >> On Tue, Oct 19, 2010 at 4:43 AM, Greg KH <greg@kroah.com> wrote:
> >> > On Mon, Oct 18, 2010 at 05:42:06PM +0200, Arnd Bergmann wrote:
> >> >>
> >> >> Out of the remaining modules, I guess i810/i830, adfs, hpfs and ufs might end
> >> >> up not getting fixed at all, we can either mark them non-SMP or move them
> >> >> to drivers/staging once all the others are done.
> >> >
> >> > I recommend moving them to staging, and then retire them from there if
> >> > no one steps up to maintain them.
> >>
> >> I think this sets a bad precedent, these drivers work fine. Removing
> >> BKL from them is hard, and involves finding and booting hw that
> >> developers don't have much time/interest in at the moment. Anyone who
> >> has access to the i810 hw and has time to work out the locking has
> >> more important things to be doing with modern hw, however it doesn't
> >> mean we should just drop support for old drivers because they don't
> >> have active maintainers. Removing the BKL from the kernel is a great
> >> goal, but breaking userspace ABI by removing drivers isn't.
> >
> > Should we just restrict such drivers to only be able to build on UP
> > machines with preempt disabled so that the BKL could be safely removed
> > from them?
> >
> > Or what other idea do you have as to what could be done here?
> >
> > I do have access to this hardware, but its on an old single processor
> > laptop, so any work that it would take to help do this development,
> > really wouldn't be able to be tested to be valid at all.
>
> There is only very rare case where the i830 driver might get used with
> SMP and really I think that case is in the don't care place, since if
> you have that hw you probably should be using i915 on it anyways.
So, there is no need for the i830 driver? Can it just be removed
because i915 works instead?
> So it really only leaves the problem case of what do distros do if we
> mark things as BROKEN_ON_SMP, since no distro builds UP kernels and
> when you boot the SMP kernels on UP they don't run as SMP so not
> having the driver load on those is a problem. Maybe we just need some
> sort of warn on smp if a smp unfriendly driver is loaded and we
> transition to SMP mode. Though this sounds like either (a) something
> we do now and I don't about it, (b) work.
So you are saying that just because distros will never build such a
thing, we should keep it building for SMP mode? Why not prevent it from
being built and if a distro really cares, then they will pony up the
development to fix the driver up?
In other words, if someone really cares, then they will do the work,
otherwise why worry? Especially as it seems that no one here is going
to do it, right?
thanks,
greg k-h
^ permalink raw reply
* Re: Linux 2.6.35/TIPC 2.0 ABI breaking changes
From: Leandro Lucarella @ 2010-10-19 2:16 UTC (permalink / raw)
To: Neil Horman
Cc: Paul Gortmaker, jon.maloy, netdev, linux-kernel, tipc-discussion,
David Miller
In-Reply-To: <20101018234547.GA5703@hmsreliant.think-freely.org>
Neil Horman, el 18 de octubre a las 19:45 me escribiste:
> > What I think has happened here (and I'll double check this
> > tomorrow, since it is before I started assisting with tipc)
> > is that a backwards incompatible change *did* inadvertently
> > creep in via these two (related) commits:
> >
> > --------------
> > commit d88dca79d3852a3623f606f781e013d61486828a
> > Author: Neil Horman <nhorman@tuxdriver.com>
> > Date: Mon Mar 8 12:20:58 2010 -0800
> >
> > tipc: fix endianness on tipc subscriber messages
> > --------------
> >
> > and
> >
> > ---------------
> > commit c6537d6742985da1fbf12ae26cde6a096fd35b5c
> > Author: Jon Paul Maloy <jon.maloy@ericsson.com>
> > Date: Tue Apr 6 11:40:52 2010 +0000
> >
> > TIPC: Updated topology subscription protocol according to latest spec
> > ---------------
> >
> > Based on Leandro's info, I think it comes down to userspace
> > not knowing exactly where to find these bits anymore:
> >
> > #define TIPC_SUB_SERVICE 0x00 /* Filter for service availability */
> > #define TIPC_SUB_PORTS 0x01 /* Filter for port availability */
> > #define TIPC_SUB_CANCEL 0x04 /* Cancel a subscription */
> >
> That shouldn't be the case. Prior to the above changes the tipc implementation
> tracked the endianess of the hosts to which it was connected and swapped data
> that it sent to those hosts accordingly. With these changes the kernel client
> simply swaps the data to network byte order on send and swaps it back to local
> order on receive universally. That second commit added a bit from the reserved
> pool of one of the connection establishment messages to indicate that a peer was
> using this new protocol. If some non-local byte order information is making it
> into user space, thats a bug that needs fixing.
>
> What may be happening is some old client that doesn't know about the new bit
> might be communicating with an new client that does. IIRC the spec called for
> clients that set bits in the reserved field to drop frames from that client, so
> that condition shouldn't occur, but TIPC may just be ignoring reserved bits. I
> wouldn't be suprised.
>
> Its also possible that the payload data between applications using tipc follow
> the same broken byte swapping method that the protocol itself did, but if that
> were the case I would expect the application to continue running normally,
> unless user space had direct access to the protocol header in its entirety, and
> read it directly, in which case I think I would just cry.
I think there is some misunderstanding here. The compatibility was
broken only for subscriptions messages. The subscriptions messages are
not sent between tipc clients (or maybe they are, but that's not how
tipc developers normally use them AFAIK). You send a subscription
message to your host tipc stack and the stack reply you with event
notifications. Even when they are message sent through a socket, they
are used as an API.
So, this has nothing to do with payload data transmitted by applications
using tipc. We are talking about the tipc API, which is "masked" into
a socket.
Here is a small example (~150 SLOC with comments). Using TIPC 2.0 API:
http://tipc.cslab.ericsson.net/cgi-bin/gitweb.cgi?p=people/allan/tipcutils.git;a=blob;h=efdfa3802e51d9a2a9091b3d97625de9e686b72e;hb=tipcutils2.0;f=demos/topology_subscr_demo/client_tipc.c
Using the "old" TIPC 1.6 API:
http://tipc.cslab.ericsson.net/cgi-bin/gitweb.cgi?p=people/allan/tipcutils.git;a=blob;h=ac5dfc5004b482372abb7905c90fe3073fc9165d;hb=15f57f7572898959e0aaa66293895a8255d77021;f=demos/topology_subscr_demo/subscriptions.c
> > ...because it doesn't know if there is the old auto endian
> > swap thing being done or not being done.
> >
> > Assuming it is possible to do so in some non-kludgy way,
> > it sounds like we want to be looking into an in-kernel change
> > that ensures the older user space binaries get their
> > functionality restored then?
> >
> Lets try figure out exactly what data is getting mis-read first. Maybe we can
> fix it without having to go back to making a sending host figure out a receiving
> hosts byte order. That would be nice. Can you describe the problem in more
> detail?
The problem is not between the tipc stacks in different hosts, is
between the tipc stack and the applications using it (well, maybe there
is a problem somewhere else too).
This was a deliberate API change, not a subtle bug...
--
Leandro Lucarella (AKA luca) http://llucax.com.ar/
----------------------------------------------------------------------
GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145 104C 949E BFB6 5F5A 8D05)
----------------------------------------------------------------------
For me to ask a woman out, I've got to get into a mental state like the karate
guys before they break the bricks.
-- George Constanza
^ permalink raw reply
* [PATCH] bridge: Forward reserved group addresses if !STP
From: Benjamin Poirier @ 2010-10-19 2:09 UTC (permalink / raw)
To: Stephen Hemminger
Cc: David S. Miller, Herbert Xu, Eric Dumazet, Jiri Pirko, bridge,
netdev, linux-kernel
In-Reply-To: <20101018093837.26bb149a@nehalam>
Make all frames sent to reserved group MAC addresses (01:80:c2:00:00:00 to
01:80:c2:00:00:0f) be forwarded if STP is disabled. This enables
forwarding EAPOL frames, among other things.
Signed-off-by: Benjamin Poirier <benjamin.poirier@polymtl.ca>
---
net/bridge/br_input.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/net/bridge/br_input.c b/net/bridge/br_input.c
index 826cd52..436488c 100644
--- a/net/bridge/br_input.c
+++ b/net/bridge/br_input.c
@@ -159,7 +159,7 @@ struct sk_buff *br_handle_frame(struct sk_buff *skb)
goto drop;
/* If STP is turned off, then forward */
- if (p->br->stp_enabled == BR_NO_STP && dest[5] == 0)
+ if (p->br->stp_enabled == BR_NO_STP)
goto forward;
if (NF_HOOK(NFPROTO_BRIDGE, NF_BR_LOCAL_IN, skb, skb->dev,
--
1.7.1
^ permalink raw reply related
* [PATCH net-next] bnx2: Increase max rx ring size from 1K to 2K
From: Michael Chan @ 2010-10-19 0:30 UTC (permalink / raw)
To: davem; +Cc: andy, jfeeney, netdev
A number of customers are reporting packet loss under certain workloads
(e.g. heavy bursts of small packets) with flow control disabled. A larger
rx ring helps to prevent these losses.
No change in default rx ring size and memory consumption.
Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
Acked-by: John Feeney <jfeeney@redhat.com>
Signed-off-by: Michael Chan <mchan@broadcom.com>
---
drivers/net/bnx2.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/bnx2.h b/drivers/net/bnx2.h
index efdfbc2..62ac83e 100644
--- a/drivers/net/bnx2.h
+++ b/drivers/net/bnx2.h
@@ -6502,8 +6502,8 @@ struct l2_fhdr {
#define TX_DESC_CNT (BCM_PAGE_SIZE / sizeof(struct tx_bd))
#define MAX_TX_DESC_CNT (TX_DESC_CNT - 1)
-#define MAX_RX_RINGS 4
-#define MAX_RX_PG_RINGS 16
+#define MAX_RX_RINGS 8
+#define MAX_RX_PG_RINGS 32
#define RX_DESC_CNT (BCM_PAGE_SIZE / sizeof(struct rx_bd))
#define MAX_RX_DESC_CNT (RX_DESC_CNT - 1)
#define MAX_TOTAL_RX_DESC_CNT (MAX_RX_DESC_CNT * MAX_RX_RINGS)
--
1.6.4.GIT
^ permalink raw reply related
* Re: [PATCH 3/3] net: allocate tx queues in register_netdevice
From: John Fastabend @ 2010-10-19 1:19 UTC (permalink / raw)
To: Tom Herbert
Cc: Ben Hutchings, davem@davemloft.net, netdev@vger.kernel.org,
eric.dumazet@gmail.com
In-Reply-To: <AANLkTinX+2aW_yDa0o=oiaxt6BJGNWjiifGCS7auz0O9@mail.gmail.com>
On 10/18/2010 5:17 PM, Tom Herbert wrote:
> On Mon, Oct 18, 2010 at 5:12 PM, John Fastabend
> <john.r.fastabend@intel.com> wrote:
>> On 10/18/2010 2:41 PM, Ben Hutchings wrote:
>>> On Mon, 2010-10-18 at 11:02 -0700, Tom Herbert wrote:
>>>> This patch introduces netif_alloc_netdev_queues which is called from
>>>> register_device instead of alloc_netdev_mq. This makes TX queue
>>>> allocation symmetric with RX allocation similarly allows drivers to
>>>> change dev->num_tx_queues after allocating netdev and before
>>>> registering it.
>>>
>>> Changing num_tx_queues is probably *not* desirable, same as for
>>> num_rx_queues.
>>>
>
> Okay, so both netif_set_real_num_rx_queues and
> netif_set_real_num_tx_queues should return -EINVAL if queue count >
> num_[tr]x_queues?
>
Yes and also if txq|rxq < 1 but it looks like this case is covered.
>>
>> Right, this will break ixgbe and other drivers that may call netif_set_real_num_{rx|tx}_queues() to increase the number of
>> queues. Returning an error code seems like a good idea though.
>>
>> John.
>>
>>> [...]
>>>> --- a/net/core/dev.c
>>>> +++ b/net/core/dev.c
>>>> @@ -1553,18 +1553,24 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
>>>> * Routine to help set real_num_tx_queues. To avoid skbs mapped to queues
>>>> * greater then real_num_tx_queues stale skbs on the qdisc must be flushed.
>>>> */
>>>> -void netif_set_real_num_tx_queues(struct net_device *dev, unsigned int txq)
>>>> +int netif_set_real_num_tx_queues(struct net_device *dev, unsigned int txq)
>>>> {
>>>> - unsigned int real_num = dev->real_num_tx_queues;
>>>> + if (txq < 1)
>>>> + return -EINVAL;
>>>>
>>>> - if (unlikely(txq > dev->num_tx_queues))
>>>> - ;
>>>> - else if (txq > real_num)
>>>> - dev->real_num_tx_queues = txq;
>>>> - else if (txq < real_num) {
>>>> - dev->real_num_tx_queues = txq;
>>>> - qdisc_reset_all_tx_gt(dev, txq);
>>>> - }
>>>> + if (dev->reg_state == NETREG_REGISTERED) {
>>>> + ASSERT_RTNL();
>>>> +
>>>> + if (txq > dev->num_tx_queues)
>>>> + return -EINVAL;
>>>> +
>>>> + if (txq < dev->real_num_tx_queues)
>>>> + qdisc_reset_all_tx_gt(dev, txq);
>>>> + } else
>>>> + dev->num_tx_queues = txq;
>>> [...]
>>>
>>> The kernel-doc comment should be updated to reflect the locking
>>> requirement and the possibility of failure when called after
>>> registration.
>>>
>>> Ben.
>>>
>>
>>
> --
> 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: [PATCH 1/3] cxgb3: function namespace cleanup
From: Divy Le Ray @ 2010-10-19 1:14 UTC (permalink / raw)
To: Stephen Hemminger
Cc: David S. Miller, Casey Leedom, Dimitrios Michailidis, netdev
In-Reply-To: <20101015224523.536739529@vyatta.com>
On 10/15/2010 03:43 PM, Stephen Hemminger wrote:
> Make local functions static. Remove functions that are
> defined and never used. Compile tested only.
>
> Signed-off-by: Stephen Hemminger<shemminger@vyatta.com>
Acked-by: Divy Le Ray <divy@chelsio.com>
> ---
> drivers/net/cxgb3/adapter.h | 3
> drivers/net/cxgb3/common.h | 18 ---
> drivers/net/cxgb3/cxgb3_defs.h | 3
> drivers/net/cxgb3/cxgb3_offload.c | 9 +
> drivers/net/cxgb3/mc5.c | 38 -------
> drivers/net/cxgb3/sge.c | 39 -------
> drivers/net/cxgb3/t3_hw.c | 197 ++------------------------------------
> 7 files changed, 18 insertions(+), 289 deletions(-)
>
> --- a/drivers/net/cxgb3/common.h 2010-10-15 11:13:45.087851624 -0700
> +++ b/drivers/net/cxgb3/common.h 2010-10-15 11:24:41.425577576 -0700
> @@ -673,7 +673,6 @@ void t3_xgm_intr_enable(struct adapter *
> void t3_xgm_intr_disable(struct adapter *adapter, int idx);
> void t3_port_intr_enable(struct adapter *adapter, int idx);
> void t3_port_intr_disable(struct adapter *adapter, int idx);
> -void t3_port_intr_clear(struct adapter *adapter, int idx);
> int t3_slow_intr_handler(struct adapter *adapter);
> int t3_phy_intr_handler(struct adapter *adapter);
>
> @@ -689,14 +688,10 @@ int t3_check_tpsram_version(struct adapt
> int t3_check_tpsram(struct adapter *adapter, const u8 *tp_ram,
> unsigned int size);
> int t3_set_proto_sram(struct adapter *adap, const u8 *data);
> -int t3_read_flash(struct adapter *adapter, unsigned int addr,
> - unsigned int nwords, u32 *data, int byte_oriented);
> int t3_load_fw(struct adapter *adapter, const u8 * fw_data, unsigned int size);
> int t3_get_fw_version(struct adapter *adapter, u32 *vers);
> int t3_check_fw_version(struct adapter *adapter);
> int t3_init_hw(struct adapter *adapter, u32 fw_params);
> -void mac_prep(struct cmac *mac, struct adapter *adapter, int index);
> -void early_hw_init(struct adapter *adapter, const struct adapter_info *ai);
> int t3_reset_adapter(struct adapter *adapter);
> int t3_prep_adapter(struct adapter *adapter, const struct adapter_info *ai,
> int reset);
> @@ -706,8 +701,6 @@ void t3_fatal_err(struct adapter *adapte
> void t3_set_vlan_accel(struct adapter *adapter, unsigned int ports, int on);
> void t3_config_rss(struct adapter *adapter, unsigned int rss_config,
> const u8 * cpus, const u16 *rspq);
> -int t3_read_rss(struct adapter *adapter, u8 * lkup, u16 *map);
> -int t3_mps_set_active_ports(struct adapter *adap, unsigned int port_mask);
> int t3_cim_ctl_blk_read(struct adapter *adap, unsigned int addr,
> unsigned int n, unsigned int *valp);
> int t3_mc7_bd_read(struct mc7 *mc7, unsigned int start, unsigned int n,
> @@ -731,19 +724,12 @@ void t3_mc5_prep(struct adapter *adapter
> int t3_mc5_init(struct mc5 *mc5, unsigned int nservers, unsigned int nfilters,
> unsigned int nroutes);
> void t3_mc5_intr_handler(struct mc5 *mc5);
> -int t3_read_mc5_range(const struct mc5 *mc5, unsigned int start, unsigned int n,
> - u32 *buf);
>
> -int t3_tp_set_coalescing_size(struct adapter *adap, unsigned int size, int psh);
> -void t3_tp_set_max_rxsize(struct adapter *adap, unsigned int size);
> void t3_tp_set_offload_mode(struct adapter *adap, int enable);
> void t3_tp_get_mib_stats(struct adapter *adap, struct tp_mib_stats *tps);
> void t3_load_mtus(struct adapter *adap, unsigned short mtus[NMTUS],
> unsigned short alpha[NCCTRL_WIN],
> unsigned short beta[NCCTRL_WIN], unsigned short mtu_cap);
> -void t3_read_hw_mtus(struct adapter *adap, unsigned short mtus[NMTUS]);
> -void t3_get_cong_cntl_tab(struct adapter *adap,
> - unsigned short incr[NMTUS][NCCTRL_WIN]);
> void t3_config_trace_filter(struct adapter *adapter,
> const struct trace_params *tp, int filter_index,
> int invert, int enable);
> @@ -769,10 +755,6 @@ int t3_sge_enable_ecntxt(struct adapter
> int t3_sge_disable_fl(struct adapter *adapter, unsigned int id);
> int t3_sge_disable_rspcntxt(struct adapter *adapter, unsigned int id);
> int t3_sge_disable_cqcntxt(struct adapter *adapter, unsigned int id);
> -int t3_sge_read_ecntxt(struct adapter *adapter, unsigned int id, u32 data[4]);
> -int t3_sge_read_fl(struct adapter *adapter, unsigned int id, u32 data[4]);
> -int t3_sge_read_cq(struct adapter *adapter, unsigned int id, u32 data[4]);
> -int t3_sge_read_rspq(struct adapter *adapter, unsigned int id, u32 data[4]);
> int t3_sge_cqcntxt_op(struct adapter *adapter, unsigned int id, unsigned int op,
> unsigned int credits);
>
> --- a/drivers/net/cxgb3/cxgb3_defs.h 2010-10-15 11:17:35.345242098 -0700
> +++ b/drivers/net/cxgb3/cxgb3_defs.h 2010-10-15 11:28:15.233294035 -0700
> @@ -43,8 +43,6 @@
>
> void *cxgb_alloc_mem(unsigned long size);
> void cxgb_free_mem(void *addr);
> -void cxgb_neigh_update(struct neighbour *neigh);
> -void cxgb_redirect(struct dst_entry *old, struct dst_entry *new);
>
> /*
> * Map an ATID or STID to their entries in the corresponding TID tables.
> @@ -111,7 +109,6 @@ static inline struct t3c_tid_entry *look
> return&e->t3c_tid;
> }
>
> -int process_rx(struct t3cdev *dev, struct sk_buff **skbs, int n);
> int attach_t3cdev(struct t3cdev *dev);
> void detach_t3cdev(struct t3cdev *dev);
> #endif
> --- a/drivers/net/cxgb3/cxgb3_offload.c 2010-10-15 11:12:57.413912920 -0700
> +++ b/drivers/net/cxgb3/cxgb3_offload.c 2010-10-15 11:28:04.412900334 -0700
> @@ -60,6 +60,9 @@ static LIST_HEAD(adapter_list);
> static const unsigned int MAX_ATIDS = 64 * 1024;
> static const unsigned int ATID_BASE = 0x10000;
>
> +static void cxgb_neigh_update(struct neighbour *neigh);
> +static void cxgb_redirect(struct dst_entry *old, struct dst_entry *new);
> +
> static inline int offload_activated(struct t3cdev *tdev)
> {
> const struct adapter *adapter = tdev2adap(tdev);
> @@ -1015,7 +1018,7 @@ EXPORT_SYMBOL(t3_register_cpl_handler);
> /*
> * T3CDEV's receive method.
> */
> -int process_rx(struct t3cdev *dev, struct sk_buff **skbs, int n)
> +static int process_rx(struct t3cdev *dev, struct sk_buff **skbs, int n)
> {
> while (n--) {
> struct sk_buff *skb = *skbs++;
> @@ -1070,7 +1073,7 @@ static int is_offloading(struct net_devi
> return 0;
> }
>
> -void cxgb_neigh_update(struct neighbour *neigh)
> +static void cxgb_neigh_update(struct neighbour *neigh)
> {
> struct net_device *dev = neigh->dev;
>
> @@ -1104,7 +1107,7 @@ static void set_l2t_ix(struct t3cdev *td
> tdev->send(tdev, skb);
> }
>
> -void cxgb_redirect(struct dst_entry *old, struct dst_entry *new)
> +static void cxgb_redirect(struct dst_entry *old, struct dst_entry *new)
> {
> struct net_device *olddev, *newdev;
> struct tid_info *ti;
> --- a/drivers/net/cxgb3/mc5.c 2010-10-15 11:12:49.041572658 -0700
> +++ b/drivers/net/cxgb3/mc5.c 2010-10-15 11:15:00.746932380 -0700
> @@ -374,44 +374,6 @@ int t3_mc5_init(struct mc5 *mc5, unsigne
> return err;
> }
>
> -/*
> - * read_mc5_range - dump a part of the memory managed by MC5
> - * @mc5: the MC5 handle
> - * @start: the start address for the dump
> - * @n: number of 72-bit words to read
> - * @buf: result buffer
> - *
> - * Read n 72-bit words from MC5 memory from the given start location.
> - */
> -int t3_read_mc5_range(const struct mc5 *mc5, unsigned int start,
> - unsigned int n, u32 *buf)
> -{
> - u32 read_cmd;
> - int err = 0;
> - struct adapter *adap = mc5->adapter;
> -
> - if (mc5->part_type == IDT75P52100)
> - read_cmd = IDT_CMD_READ;
> - else if (mc5->part_type == IDT75N43102)
> - read_cmd = IDT4_CMD_READ;
> - else
> - return -EINVAL;
> -
> - mc5_dbgi_mode_enable(mc5);
> -
> - while (n--) {
> - t3_write_reg(adap, A_MC5_DB_DBGI_REQ_ADDR0, start++);
> - if (mc5_cmd_write(adap, read_cmd)) {
> - err = -EIO;
> - break;
> - }
> - dbgi_rd_rsp3(adap, buf + 2, buf + 1, buf);
> - buf += 3;
> - }
> -
> - mc5_dbgi_mode_disable(mc5);
> - return 0;
> -}
>
> #define MC5_INT_FATAL (F_PARITYERR | F_REQQPARERR | F_DISPQPARERR)
>
> --- a/drivers/net/cxgb3/t3_hw.c 2010-10-15 11:13:07.590326587 -0700
> +++ b/drivers/net/cxgb3/t3_hw.c 2010-10-15 11:26:01.636456578 -0700
> @@ -34,6 +34,8 @@
> #include "sge_defs.h"
> #include "firmware_exports.h"
>
> +static void t3_port_intr_clear(struct adapter *adapter, int idx);
> +
> /**
> * t3_wait_op_done_val - wait until an operation is completed
> * @adapter: the adapter performing the operation
> @@ -840,8 +842,8 @@ static int flash_wait_op(struct adapter
> * (i.e., big-endian), otherwise as 32-bit words in the platform's
> * natural endianess.
> */
> -int t3_read_flash(struct adapter *adapter, unsigned int addr,
> - unsigned int nwords, u32 *data, int byte_oriented)
> +static int t3_read_flash(struct adapter *adapter, unsigned int addr,
> + unsigned int nwords, u32 *data, int byte_oriented)
> {
> int ret;
>
> @@ -2111,7 +2113,7 @@ void t3_port_intr_disable(struct adapter
> * Clear port-specific (i.e., MAC and PHY) interrupts for the given
> * adapter port.
> */
> -void t3_port_intr_clear(struct adapter *adapter, int idx)
> +static void t3_port_intr_clear(struct adapter *adapter, int idx)
> {
> struct cphy *phy =&adap2pinfo(adapter, idx)->phy;
>
> @@ -2484,98 +2486,6 @@ int t3_sge_cqcntxt_op(struct adapter *ad
> }
>
> /**
> - * t3_sge_read_context - read an SGE context
> - * @type: the context type
> - * @adapter: the adapter
> - * @id: the context id
> - * @data: holds the retrieved context
> - *
> - * Read an SGE egress context. The caller is responsible for ensuring
> - * only one context operation occurs at a time.
> - */
> -static int t3_sge_read_context(unsigned int type, struct adapter *adapter,
> - unsigned int id, u32 data[4])
> -{
> - if (t3_read_reg(adapter, A_SG_CONTEXT_CMD)& F_CONTEXT_CMD_BUSY)
> - return -EBUSY;
> -
> - t3_write_reg(adapter, A_SG_CONTEXT_CMD,
> - V_CONTEXT_CMD_OPCODE(0) | type | V_CONTEXT(id));
> - if (t3_wait_op_done(adapter, A_SG_CONTEXT_CMD, F_CONTEXT_CMD_BUSY, 0,
> - SG_CONTEXT_CMD_ATTEMPTS, 1))
> - return -EIO;
> - data[0] = t3_read_reg(adapter, A_SG_CONTEXT_DATA0);
> - data[1] = t3_read_reg(adapter, A_SG_CONTEXT_DATA1);
> - data[2] = t3_read_reg(adapter, A_SG_CONTEXT_DATA2);
> - data[3] = t3_read_reg(adapter, A_SG_CONTEXT_DATA3);
> - return 0;
> -}
> -
> -/**
> - * t3_sge_read_ecntxt - read an SGE egress context
> - * @adapter: the adapter
> - * @id: the context id
> - * @data: holds the retrieved context
> - *
> - * Read an SGE egress context. The caller is responsible for ensuring
> - * only one context operation occurs at a time.
> - */
> -int t3_sge_read_ecntxt(struct adapter *adapter, unsigned int id, u32 data[4])
> -{
> - if (id>= 65536)
> - return -EINVAL;
> - return t3_sge_read_context(F_EGRESS, adapter, id, data);
> -}
> -
> -/**
> - * t3_sge_read_cq - read an SGE CQ context
> - * @adapter: the adapter
> - * @id: the context id
> - * @data: holds the retrieved context
> - *
> - * Read an SGE CQ context. The caller is responsible for ensuring
> - * only one context operation occurs at a time.
> - */
> -int t3_sge_read_cq(struct adapter *adapter, unsigned int id, u32 data[4])
> -{
> - if (id>= 65536)
> - return -EINVAL;
> - return t3_sge_read_context(F_CQ, adapter, id, data);
> -}
> -
> -/**
> - * t3_sge_read_fl - read an SGE free-list context
> - * @adapter: the adapter
> - * @id: the context id
> - * @data: holds the retrieved context
> - *
> - * Read an SGE free-list context. The caller is responsible for ensuring
> - * only one context operation occurs at a time.
> - */
> -int t3_sge_read_fl(struct adapter *adapter, unsigned int id, u32 data[4])
> -{
> - if (id>= SGE_QSETS * 2)
> - return -EINVAL;
> - return t3_sge_read_context(F_FREELIST, adapter, id, data);
> -}
> -
> -/**
> - * t3_sge_read_rspq - read an SGE response queue context
> - * @adapter: the adapter
> - * @id: the context id
> - * @data: holds the retrieved context
> - *
> - * Read an SGE response queue context. The caller is responsible for
> - * ensuring only one context operation occurs at a time.
> - */
> -int t3_sge_read_rspq(struct adapter *adapter, unsigned int id, u32 data[4])
> -{
> - if (id>= SGE_QSETS)
> - return -EINVAL;
> - return t3_sge_read_context(F_RESPONSEQ, adapter, id, data);
> -}
> -
> -/**
> * t3_config_rss - configure Rx packet steering
> * @adapter: the adapter
> * @rss_config: RSS settings (written to TP_RSS_CONFIG)
> @@ -2616,42 +2526,6 @@ void t3_config_rss(struct adapter *adapt
> }
>
> /**
> - * t3_read_rss - read the contents of the RSS tables
> - * @adapter: the adapter
> - * @lkup: holds the contents of the RSS lookup table
> - * @map: holds the contents of the RSS map table
> - *
> - * Reads the contents of the receive packet steering tables.
> - */
> -int t3_read_rss(struct adapter *adapter, u8 * lkup, u16 *map)
> -{
> - int i;
> - u32 val;
> -
> - if (lkup)
> - for (i = 0; i< RSS_TABLE_SIZE; ++i) {
> - t3_write_reg(adapter, A_TP_RSS_LKP_TABLE,
> - 0xffff0000 | i);
> - val = t3_read_reg(adapter, A_TP_RSS_LKP_TABLE);
> - if (!(val& 0x80000000))
> - return -EAGAIN;
> - *lkup++ = val;
> - *lkup++ = (val>> 8);
> - }
> -
> - if (map)
> - for (i = 0; i< RSS_TABLE_SIZE; ++i) {
> - t3_write_reg(adapter, A_TP_RSS_MAP_TABLE,
> - 0xffff0000 | i);
> - val = t3_read_reg(adapter, A_TP_RSS_MAP_TABLE);
> - if (!(val& 0x80000000))
> - return -EAGAIN;
> - *map++ = val;
> - }
> - return 0;
> -}
> -
> -/**
> * t3_tp_set_offload_mode - put TP in NIC/offload mode
> * @adap: the adapter
> * @enable: 1 to select offload mode, 0 for regular NIC
> @@ -2868,7 +2742,8 @@ static void tp_set_timers(struct adapter
> *
> * Set the receive coalescing size and PSH bit handling.
> */
> -int t3_tp_set_coalescing_size(struct adapter *adap, unsigned int size, int psh)
> +static int t3_tp_set_coalescing_size(struct adapter *adap,
> + unsigned int size, int psh)
> {
> u32 val;
>
> @@ -2898,7 +2773,7 @@ int t3_tp_set_coalescing_size(struct ada
> * Set TP's max receive size. This is the limit that applies when
> * receive coalescing is disabled.
> */
> -void t3_tp_set_max_rxsize(struct adapter *adap, unsigned int size)
> +static void t3_tp_set_max_rxsize(struct adapter *adap, unsigned int size)
> {
> t3_write_reg(adap, A_TP_PARA_REG7,
> V_PMMAXXFERLEN0(size) | V_PMMAXXFERLEN1(size));
> @@ -3018,48 +2893,6 @@ void t3_load_mtus(struct adapter *adap,
> }
>
> /**
> - * t3_read_hw_mtus - returns the values in the HW MTU table
> - * @adap: the adapter
> - * @mtus: where to store the HW MTU values
> - *
> - * Reads the HW MTU table.
> - */
> -void t3_read_hw_mtus(struct adapter *adap, unsigned short mtus[NMTUS])
> -{
> - int i;
> -
> - for (i = 0; i< NMTUS; ++i) {
> - unsigned int val;
> -
> - t3_write_reg(adap, A_TP_MTU_TABLE, 0xff000000 | i);
> - val = t3_read_reg(adap, A_TP_MTU_TABLE);
> - mtus[i] = val& 0x3fff;
> - }
> -}
> -
> -/**
> - * t3_get_cong_cntl_tab - reads the congestion control table
> - * @adap: the adapter
> - * @incr: where to store the alpha values
> - *
> - * Reads the additive increments programmed into the HW congestion
> - * control table.
> - */
> -void t3_get_cong_cntl_tab(struct adapter *adap,
> - unsigned short incr[NMTUS][NCCTRL_WIN])
> -{
> - unsigned int mtu, w;
> -
> - for (mtu = 0; mtu< NMTUS; ++mtu)
> - for (w = 0; w< NCCTRL_WIN; ++w) {
> - t3_write_reg(adap, A_TP_CCTRL_TABLE,
> - 0xffff0000 | (mtu<< 5) | w);
> - incr[mtu][w] = t3_read_reg(adap, A_TP_CCTRL_TABLE)&
> - 0x1fff;
> - }
> -}
> -
> -/**
> * t3_tp_get_mib_stats - read TP's MIB counters
> * @adap: the adapter
> * @tps: holds the returned counter values
> @@ -3223,15 +3056,6 @@ static int tp_init(struct adapter *adap,
> return busy;
> }
>
> -int t3_mps_set_active_ports(struct adapter *adap, unsigned int port_mask)
> -{
> - if (port_mask& ~((1<< adap->params.nports) - 1))
> - return -EINVAL;
> - t3_set_reg_field(adap, A_MPS_CFG, F_PORT1ACTIVE | F_PORT0ACTIVE,
> - port_mask<< S_PORT0ACTIVE);
> - return 0;
> -}
> -
> /*
> * Perform the bits of HW initialization that are dependent on the Tx
> * channels being used.
> @@ -3687,7 +3511,7 @@ static void mc7_prep(struct adapter *ada
> mc7->width = G_WIDTH(cfg);
> }
>
> -void mac_prep(struct cmac *mac, struct adapter *adapter, int index)
> +static void mac_prep(struct cmac *mac, struct adapter *adapter, int index)
> {
> u16 devid;
>
> @@ -3707,7 +3531,8 @@ void mac_prep(struct cmac *mac, struct a
> }
> }
>
> -void early_hw_init(struct adapter *adapter, const struct adapter_info *ai)
> +static void early_hw_init(struct adapter *adapter,
> + const struct adapter_info *ai)
> {
> u32 val = V_PORTSPEED(is_10G(adapter) ? 3 : 2);
>
> --- a/drivers/net/cxgb3/adapter.h 2010-10-15 11:29:31.428075569 -0700
> +++ b/drivers/net/cxgb3/adapter.h 2010-10-15 11:29:46.280619610 -0700
> @@ -336,9 +336,6 @@ int t3_sge_alloc_qset(struct adapter *ad
> int irq_vec_idx, const struct qset_params *p,
> int ntxq, struct net_device *dev,
> struct netdev_queue *netdevq);
> -int t3_get_desc(const struct sge_qset *qs, unsigned int qnum, unsigned int idx,
> - unsigned char *data);
> -irqreturn_t t3_sge_intr_msix(int irq, void *cookie);
> extern struct workqueue_struct *cxgb3_wq;
>
> int t3_get_edc_fw(struct cphy *phy, int edc_idx, int size);
> --- a/drivers/net/cxgb3/sge.c 2010-10-15 11:13:02.710128196 -0700
> +++ b/drivers/net/cxgb3/sge.c 2010-10-15 11:30:17.141751919 -0700
> @@ -2554,7 +2554,7 @@ static inline int handle_responses(struc
> * The MSI-X interrupt handler for an SGE response queue for the non-NAPI case
> * (i.e., response queue serviced in hard interrupt).
> */
> -irqreturn_t t3_sge_intr_msix(int irq, void *cookie)
> +static irqreturn_t t3_sge_intr_msix(int irq, void *cookie)
> {
> struct sge_qset *qs = cookie;
> struct adapter *adap = qs->adap;
> @@ -3320,40 +3320,3 @@ void t3_sge_prep(struct adapter *adap, s
>
> spin_lock_init(&adap->sge.reg_lock);
> }
> -
> -/**
> - * t3_get_desc - dump an SGE descriptor for debugging purposes
> - * @qs: the queue set
> - * @qnum: identifies the specific queue (0..2: Tx, 3:response, 4..5: Rx)
> - * @idx: the descriptor index in the queue
> - * @data: where to dump the descriptor contents
> - *
> - * Dumps the contents of a HW descriptor of an SGE queue. Returns the
> - * size of the descriptor.
> - */
> -int t3_get_desc(const struct sge_qset *qs, unsigned int qnum, unsigned int idx,
> - unsigned char *data)
> -{
> - if (qnum>= 6)
> - return -EINVAL;
> -
> - if (qnum< 3) {
> - if (!qs->txq[qnum].desc || idx>= qs->txq[qnum].size)
> - return -EINVAL;
> - memcpy(data,&qs->txq[qnum].desc[idx], sizeof(struct tx_desc));
> - return sizeof(struct tx_desc);
> - }
> -
> - if (qnum == 3) {
> - if (!qs->rspq.desc || idx>= qs->rspq.size)
> - return -EINVAL;
> - memcpy(data,&qs->rspq.desc[idx], sizeof(struct rsp_desc));
> - return sizeof(struct rsp_desc);
> - }
> -
> - qnum -= 4;
> - if (!qs->fl[qnum].desc || idx>= qs->fl[qnum].size)
> - return -EINVAL;
> - memcpy(data,&qs->fl[qnum].desc[idx], sizeof(struct rx_desc));
> - return sizeof(struct rx_desc);
> -}
>
>
>
^ permalink raw reply
* Re: [Ksummit-2010-discuss] [v2] Remaining BKL users, what to do
From: Dave Airlie @ 2010-10-19 0:57 UTC (permalink / raw)
To: Greg KH
Cc: Arnd Bergmann, codalist, ksummit-2010-discuss, autofs, Jan Harkes,
Samuel Ortiz, Jan Kara, Arnaldo Carvalho de Melo, netdev,
Anders Larsen, linux-kernel, dri-devel, Bryan Schumaker,
Christoph Hellwig, Petr Vandrovec, Mikulas Patocka, linux-fsdevel,
Evgeniy Dushistov, Ingo Molnar, Andrew Hendry, linux-media
In-Reply-To: <20101019004004.GB28380@kroah.com>
On Tue, Oct 19, 2010 at 10:40 AM, Greg KH <greg@kroah.com> wrote:
> On Tue, Oct 19, 2010 at 09:00:09AM +1000, Dave Airlie wrote:
>> On Tue, Oct 19, 2010 at 4:43 AM, Greg KH <greg@kroah.com> wrote:
>> > On Mon, Oct 18, 2010 at 05:42:06PM +0200, Arnd Bergmann wrote:
>> >>
>> >> Out of the remaining modules, I guess i810/i830, adfs, hpfs and ufs might end
>> >> up not getting fixed at all, we can either mark them non-SMP or move them
>> >> to drivers/staging once all the others are done.
>> >
>> > I recommend moving them to staging, and then retire them from there if
>> > no one steps up to maintain them.
>>
>> I think this sets a bad precedent, these drivers work fine. Removing
>> BKL from them is hard, and involves finding and booting hw that
>> developers don't have much time/interest in at the moment. Anyone who
>> has access to the i810 hw and has time to work out the locking has
>> more important things to be doing with modern hw, however it doesn't
>> mean we should just drop support for old drivers because they don't
>> have active maintainers. Removing the BKL from the kernel is a great
>> goal, but breaking userspace ABI by removing drivers isn't.
>
> Should we just restrict such drivers to only be able to build on UP
> machines with preempt disabled so that the BKL could be safely removed
> from them?
>
> Or what other idea do you have as to what could be done here?
>
> I do have access to this hardware, but its on an old single processor
> laptop, so any work that it would take to help do this development,
> really wouldn't be able to be tested to be valid at all.
There is only very rare case where the i830 driver might get used with
SMP and really I think that case is in the don't care place, since if
you have that hw you probably should be using i915 on it anyways.
So it really only leaves the problem case of what do distros do if we
mark things as BROKEN_ON_SMP, since no distro builds UP kernels and
when you boot the SMP kernels on UP they don't run as SMP so not
having the driver load on those is a problem. Maybe we just need some
sort of warn on smp if a smp unfriendly driver is loaded and we
transition to SMP mode. Though this sounds like either (a) something
we do now and I don't about it, (b) work.
Dave.
>
> thanks,
>
> greg k-h
>
^ permalink raw reply
* Re: [Ksummit-2010-discuss] [v2] Remaining BKL users, what to do
From: Greg KH @ 2010-10-19 0:40 UTC (permalink / raw)
To: Dave Airlie
Cc: Arnd Bergmann, codalist, ksummit-2010-discuss, autofs, Jan Harkes,
Samuel Ortiz, Jan Kara, Arnaldo Carvalho de Melo, netdev,
Anders Larsen, linux-kernel, dri-devel, Bryan Schumaker,
Christoph Hellwig, Petr Vandrovec, Mikulas Patocka, linux-fsdevel,
Evgeniy Dushistov, Ingo Molnar, Andrew Hendry, linux-media
In-Reply-To: <AANLkTin2KPNNXvwcWphhM-5qexB14FS7M7ezkCCYCZ2H@mail.gmail.com>
On Tue, Oct 19, 2010 at 09:00:09AM +1000, Dave Airlie wrote:
> On Tue, Oct 19, 2010 at 4:43 AM, Greg KH <greg@kroah.com> wrote:
> > On Mon, Oct 18, 2010 at 05:42:06PM +0200, Arnd Bergmann wrote:
> >>
> >> Out of the remaining modules, I guess i810/i830, adfs, hpfs and ufs might end
> >> up not getting fixed at all, we can either mark them non-SMP or move them
> >> to drivers/staging once all the others are done.
> >
> > I recommend moving them to staging, and then retire them from there if
> > no one steps up to maintain them.
>
> I think this sets a bad precedent, these drivers work fine. Removing
> BKL from them is hard, and involves finding and booting hw that
> developers don't have much time/interest in at the moment. Anyone who
> has access to the i810 hw and has time to work out the locking has
> more important things to be doing with modern hw, however it doesn't
> mean we should just drop support for old drivers because they don't
> have active maintainers. Removing the BKL from the kernel is a great
> goal, but breaking userspace ABI by removing drivers isn't.
Should we just restrict such drivers to only be able to build on UP
machines with preempt disabled so that the BKL could be safely removed
from them?
Or what other idea do you have as to what could be done here?
I do have access to this hardware, but its on an old single processor
laptop, so any work that it would take to help do this development,
really wouldn't be able to be tested to be valid at all.
thanks,
greg k-h
^ permalink raw reply
* [PATCH net-next] socket: localize functions
From: Stephen Hemminger @ 2010-10-19 0:27 UTC (permalink / raw)
To: David Miller; +Cc: netdev
A couple of functions in socket.c are only used there and
should be localized.
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/include/linux/socket.h 2010-10-18 17:06:18.455644277 -0700
+++ b/include/linux/socket.h 2010-10-18 17:06:26.763083909 -0700
@@ -326,7 +326,6 @@ extern long verify_iovec(struct msghdr *
extern int memcpy_toiovec(struct iovec *v, unsigned char *kdata, int len);
extern int memcpy_toiovecend(const struct iovec *v, unsigned char *kdata,
int offset, int len);
-extern int move_addr_to_user(struct sockaddr *kaddr, int klen, void __user *uaddr, int __user *ulen);
extern int move_addr_to_kernel(void __user *uaddr, int ulen, struct sockaddr *kaddr);
extern int put_cmsg(struct msghdr*, int level, int type, int len, void *data);
--- a/net/socket.c 2010-10-18 17:06:18.475642930 -0700
+++ b/net/socket.c 2010-10-18 17:12:53.001522923 -0700
@@ -209,8 +209,8 @@ int move_addr_to_kernel(void __user *uad
* specified. Zero is returned for a success.
*/
-int move_addr_to_user(struct sockaddr *kaddr, int klen, void __user *uaddr,
- int __user *ulen)
+static int move_addr_to_user(struct sockaddr *kaddr, int klen,
+ void __user *uaddr, int __user *ulen)
{
int err;
int len;
@@ -661,7 +661,8 @@ void __sock_recv_timestamp(struct msghdr
}
EXPORT_SYMBOL_GPL(__sock_recv_timestamp);
-inline void sock_recv_drops(struct msghdr *msg, struct sock *sk, struct sk_buff *skb)
+static inline void sock_recv_drops(struct msghdr *msg, struct sock *sk,
+ struct sk_buff *skb)
{
if (sock_flag(sk, SOCK_RXQ_OVFL) && skb && skb->dropcount)
put_cmsg(msg, SOL_SOCKET, SO_RXQ_OVFL,
^ permalink raw reply
* [PATCH net-next-2.6 3/5] jme: Prevent possible read re-order error
From: Guo-Fu Tseng @ 2010-10-19 0:10 UTC (permalink / raw)
To: David Miller; +Cc: Guo-Fu Tseng, linux-netdev
In-Reply-To: <1287447044-24471-1-git-send-email-cooldavid@cooldavid.org>
From: Guo-Fu Tseng <cooldavid@cooldavid.org>
Adding read memory barrier in between flag reading and data reading of
receive descriptors. This prevents the data being read before hardware
complete writing informations.
Reported-by: Stefan Hajnoczi <stefanha@gmail.com>
Signed-off-by: Guo-Fu Tseng <cooldavid@cooldavid.org>
---
drivers/net/jme.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/jme.c b/drivers/net/jme.c
index 0ea0da3..095f899 100644
--- a/drivers/net/jme.c
+++ b/drivers/net/jme.c
@@ -989,6 +989,7 @@ jme_process_receive(struct jme_adapter *jme, int limit)
goto out;
--limit;
+ rmb();
desccnt = rxdesc->descwb.desccnt & RXWBDCNT_DCNT;
if (unlikely(desccnt > 1 ||
--
1.7.2.2
^ permalink raw reply related
* [PATCH net-next-2.6 4/5] jme: Adding mii-tool support
From: Guo-Fu Tseng @ 2010-10-19 0:10 UTC (permalink / raw)
To: David Miller; +Cc: Guo-Fu Tseng, linux-netdev
In-Reply-To: <1287447044-24471-1-git-send-email-cooldavid@cooldavid.org>
From: Guo-Fu Tseng <cooldavid@cooldavid.org>
Adding mii-tool support for some distribution only have mii-tool
installed by default.
Signed-off-by: Guo-Fu Tseng <cooldavid@cooldavid.org>
---
drivers/net/jme.c | 34 +++++++++++++++++++++++++++++++++-
1 files changed, 33 insertions(+), 1 deletions(-)
diff --git a/drivers/net/jme.c b/drivers/net/jme.c
index 095f899..c34c70f 100644
--- a/drivers/net/jme.c
+++ b/drivers/net/jme.c
@@ -2411,8 +2411,37 @@ jme_set_settings(struct net_device *netdev,
if (!rc) {
if (fdc)
jme_reset_link(jme);
- set_bit(JME_FLAG_SSET, &jme->flags);
jme->old_ecmd = *ecmd;
+ set_bit(JME_FLAG_SSET, &jme->flags);
+ }
+
+ return rc;
+}
+
+static int
+jme_ioctl(struct net_device *netdev, struct ifreq *rq, int cmd)
+{
+ int rc;
+ struct jme_adapter *jme = netdev_priv(netdev);
+ struct mii_ioctl_data *mii_data = if_mii(rq);
+ unsigned int duplex_chg;
+
+ if (cmd == SIOCSMIIREG) {
+ u16 val = mii_data->val_in;
+ if (!(val & (BMCR_RESET|BMCR_ANENABLE)) &&
+ (val & BMCR_SPEED1000))
+ return -EINVAL;
+ }
+
+ spin_lock_bh(&jme->phy_lock);
+ rc = generic_mii_ioctl(&jme->mii_if, mii_data, cmd, &duplex_chg);
+ spin_unlock_bh(&jme->phy_lock);
+
+ if (!rc && (cmd == SIOCSMIIREG)) {
+ if (duplex_chg)
+ jme_reset_link(jme);
+ jme_get_settings(netdev, &jme->old_ecmd);
+ set_bit(JME_FLAG_SSET, &jme->flags);
}
return rc;
@@ -2692,6 +2721,7 @@ static const struct net_device_ops jme_netdev_ops = {
.ndo_open = jme_open,
.ndo_stop = jme_close,
.ndo_validate_addr = eth_validate_addr,
+ .ndo_do_ioctl = jme_ioctl,
.ndo_start_xmit = jme_start_xmit,
.ndo_set_mac_address = jme_set_macaddr,
.ndo_set_multicast_list = jme_set_multi,
@@ -2883,6 +2913,8 @@ jme_init_one(struct pci_dev *pdev,
jme->mii_if.supports_gmii = true;
else
jme->mii_if.supports_gmii = false;
+ jme->mii_if.phy_id_mask = 0x1F;
+ jme->mii_if.reg_num_mask = 0x1F;
jme->mii_if.mdio_read = jme_mdio_read;
jme->mii_if.mdio_write = jme_mdio_write;
--
1.7.2.2
^ permalink raw reply related
* [PATCH net-next-2.6 2/5] jme: Add comment in jme_set_settings
From: Guo-Fu Tseng @ 2010-10-19 0:10 UTC (permalink / raw)
To: David Miller; +Cc: Guo-Fu Tseng, linux-netdev
In-Reply-To: <1287447044-24471-1-git-send-email-cooldavid@cooldavid.org>
From: Guo-Fu Tseng <cooldavid@cooldavid.org>
Explains what `fdc` variable is for.
Signed-off-by: Guo-Fu Tseng <cooldavid@cooldavid.org>
---
drivers/net/jme.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/jme.c b/drivers/net/jme.c
index e04f180..0ea0da3 100644
--- a/drivers/net/jme.c
+++ b/drivers/net/jme.c
@@ -2394,6 +2394,10 @@ jme_set_settings(struct net_device *netdev,
if (ecmd->speed == SPEED_1000 && ecmd->autoneg != AUTONEG_ENABLE)
return -EINVAL;
+ /*
+ * Check If user changed duplex only while force_media.
+ * Hardware would not generate link change interrupt.
+ */
if (jme->mii_if.force_media &&
ecmd->autoneg != AUTONEG_ENABLE &&
(jme->mii_if.full_duplex != ecmd->duplex))
@@ -2403,10 +2407,9 @@ jme_set_settings(struct net_device *netdev,
rc = mii_ethtool_sset(&(jme->mii_if), ecmd);
spin_unlock_bh(&jme->phy_lock);
- if (!rc && fdc)
- jme_reset_link(jme);
-
if (!rc) {
+ if (fdc)
+ jme_reset_link(jme);
set_bit(JME_FLAG_SSET, &jme->flags);
jme->old_ecmd = *ecmd;
}
--
1.7.2.2
^ permalink raw reply related
* [PATCH net-next-2.6 5/5] jme: Advance version number
From: Guo-Fu Tseng @ 2010-10-19 0:10 UTC (permalink / raw)
To: David Miller; +Cc: Guo-Fu Tseng, linux-netdev
In-Reply-To: <1287447044-24471-1-git-send-email-cooldavid@cooldavid.org>
From: Guo-Fu Tseng <cooldavid@cooldavid.org>
Advance version number and update copyright info
Signed-off-by: Guo-Fu Tseng <cooldavid@cooldavid.org>
---
drivers/net/jme.c | 1 +
drivers/net/jme.h | 3 ++-
2 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/drivers/net/jme.c b/drivers/net/jme.c
index c34c70f..d7a975e 100644
--- a/drivers/net/jme.c
+++ b/drivers/net/jme.c
@@ -3,6 +3,7 @@
*
* Copyright 2008 JMicron Technology Corporation
* http://www.jmicron.com/
+ * Copyright (c) 2009 - 2010 Guo-Fu Tseng <cooldavid@cooldavid.org>
*
* Author: Guo-Fu Tseng <cooldavid@cooldavid.org>
*
diff --git a/drivers/net/jme.h b/drivers/net/jme.h
index 1360f68..eac0926 100644
--- a/drivers/net/jme.h
+++ b/drivers/net/jme.h
@@ -3,6 +3,7 @@
*
* Copyright 2008 JMicron Technology Corporation
* http://www.jmicron.com/
+ * Copyright (c) 2009 - 2010 Guo-Fu Tseng <cooldavid@cooldavid.org>
*
* Author: Guo-Fu Tseng <cooldavid@cooldavid.org>
*
@@ -25,7 +26,7 @@
#define __JME_H_INCLUDED__
#define DRV_NAME "jme"
-#define DRV_VERSION "1.0.6"
+#define DRV_VERSION "1.0.7"
#define PFX DRV_NAME ": "
#define PCI_DEVICE_ID_JMICRON_JMC250 0x0250
--
1.7.2.2
^ permalink raw reply related
* [PATCH net-next-2.6 1/5] jme: Fix PHY power-off error
From: Guo-Fu Tseng @ 2010-10-19 0:10 UTC (permalink / raw)
To: David Miller; +Cc: Guo-Fu Tseng, linux-netdev, stable
From: Guo-Fu Tseng <cooldavid@cooldavid.org>
Adding phy_on in opposition to phy_off.
Signed-off-by: Guo-Fu Tseng <cooldavid@cooldavid.org>
Cc: <stable@kernel.org>
---
drivers/net/jme.c | 22 ++++++++++++++++++----
1 files changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/net/jme.c b/drivers/net/jme.c
index c04c096..e04f180 100644
--- a/drivers/net/jme.c
+++ b/drivers/net/jme.c
@@ -1574,6 +1574,16 @@ jme_free_irq(struct jme_adapter *jme)
}
}
+static inline void
+jme_phy_on(struct jme_adapter *jme)
+{
+ u32 bmcr;
+
+ bmcr = jme_mdio_read(jme->dev, jme->mii_if.phy_id, MII_BMCR);
+ bmcr &= ~BMCR_PDOWN;
+ jme_mdio_write(jme->dev, jme->mii_if.phy_id, MII_BMCR, bmcr);
+}
+
static int
jme_open(struct net_device *netdev)
{
@@ -1594,10 +1604,12 @@ jme_open(struct net_device *netdev)
jme_start_irq(jme);
- if (test_bit(JME_FLAG_SSET, &jme->flags))
+ if (test_bit(JME_FLAG_SSET, &jme->flags)) {
+ jme_phy_on(jme);
jme_set_settings(netdev, &jme->old_ecmd);
- else
+ } else {
jme_reset_phy_processor(jme);
+ }
jme_reset_link(jme);
@@ -3005,10 +3017,12 @@ jme_resume(struct pci_dev *pdev)
jme_clear_pm(jme);
pci_restore_state(pdev);
- if (test_bit(JME_FLAG_SSET, &jme->flags))
+ if (test_bit(JME_FLAG_SSET, &jme->flags)) {
+ jme_phy_on(jme);
jme_set_settings(netdev, &jme->old_ecmd);
- else
+ } else {
jme_reset_phy_processor(jme);
+ }
jme_start_irq(jme);
netif_device_attach(netdev);
--
1.7.2.2
^ permalink raw reply related
* Re: [PATCH 3/3] net: allocate tx queues in register_netdevice
From: Tom Herbert @ 2010-10-19 0:17 UTC (permalink / raw)
To: John Fastabend
Cc: Ben Hutchings, davem@davemloft.net, netdev@vger.kernel.org,
eric.dumazet@gmail.com
In-Reply-To: <4CBCE26E.4020405@intel.com>
On Mon, Oct 18, 2010 at 5:12 PM, John Fastabend
<john.r.fastabend@intel.com> wrote:
> On 10/18/2010 2:41 PM, Ben Hutchings wrote:
>> On Mon, 2010-10-18 at 11:02 -0700, Tom Herbert wrote:
>>> This patch introduces netif_alloc_netdev_queues which is called from
>>> register_device instead of alloc_netdev_mq. This makes TX queue
>>> allocation symmetric with RX allocation similarly allows drivers to
>>> change dev->num_tx_queues after allocating netdev and before
>>> registering it.
>>
>> Changing num_tx_queues is probably *not* desirable, same as for
>> num_rx_queues.
>>
Okay, so both netif_set_real_num_rx_queues and
netif_set_real_num_tx_queues should return -EINVAL if queue count >
num_[tr]x_queues?
>
> Right, this will break ixgbe and other drivers that may call netif_set_real_num_{rx|tx}_queues() to increase the number of
> queues. Returning an error code seems like a good idea though.
>
> John.
>
>> [...]
>>> --- a/net/core/dev.c
>>> +++ b/net/core/dev.c
>>> @@ -1553,18 +1553,24 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
>>> * Routine to help set real_num_tx_queues. To avoid skbs mapped to queues
>>> * greater then real_num_tx_queues stale skbs on the qdisc must be flushed.
>>> */
>>> -void netif_set_real_num_tx_queues(struct net_device *dev, unsigned int txq)
>>> +int netif_set_real_num_tx_queues(struct net_device *dev, unsigned int txq)
>>> {
>>> - unsigned int real_num = dev->real_num_tx_queues;
>>> + if (txq < 1)
>>> + return -EINVAL;
>>>
>>> - if (unlikely(txq > dev->num_tx_queues))
>>> - ;
>>> - else if (txq > real_num)
>>> - dev->real_num_tx_queues = txq;
>>> - else if (txq < real_num) {
>>> - dev->real_num_tx_queues = txq;
>>> - qdisc_reset_all_tx_gt(dev, txq);
>>> - }
>>> + if (dev->reg_state == NETREG_REGISTERED) {
>>> + ASSERT_RTNL();
>>> +
>>> + if (txq > dev->num_tx_queues)
>>> + return -EINVAL;
>>> +
>>> + if (txq < dev->real_num_tx_queues)
>>> + qdisc_reset_all_tx_gt(dev, txq);
>>> + } else
>>> + dev->num_tx_queues = txq;
>> [...]
>>
>> The kernel-doc comment should be updated to reflect the locking
>> requirement and the possibility of failure when called after
>> registration.
>>
>> Ben.
>>
>
>
^ permalink raw reply
* Re: [PATCH 3/3] net: allocate tx queues in register_netdevice
From: John Fastabend @ 2010-10-19 0:12 UTC (permalink / raw)
To: Ben Hutchings
Cc: Tom Herbert, davem@davemloft.net, netdev@vger.kernel.org,
eric.dumazet@gmail.com
In-Reply-To: <1287438080.2252.792.camel@achroite.uk.solarflarecom.com>
On 10/18/2010 2:41 PM, Ben Hutchings wrote:
> On Mon, 2010-10-18 at 11:02 -0700, Tom Herbert wrote:
>> This patch introduces netif_alloc_netdev_queues which is called from
>> register_device instead of alloc_netdev_mq. This makes TX queue
>> allocation symmetric with RX allocation similarly allows drivers to
>> change dev->num_tx_queues after allocating netdev and before
>> registering it.
>
> Changing num_tx_queues is probably *not* desirable, same as for
> num_rx_queues.
>
Right, this will break ixgbe and other drivers that may call netif_set_real_num_{rx|tx}_queues() to increase the number of
queues. Returning an error code seems like a good idea though.
John.
> [...]
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -1553,18 +1553,24 @@ static void dev_queue_xmit_nit(struct sk_buff *skb, struct net_device *dev)
>> * Routine to help set real_num_tx_queues. To avoid skbs mapped to queues
>> * greater then real_num_tx_queues stale skbs on the qdisc must be flushed.
>> */
>> -void netif_set_real_num_tx_queues(struct net_device *dev, unsigned int txq)
>> +int netif_set_real_num_tx_queues(struct net_device *dev, unsigned int txq)
>> {
>> - unsigned int real_num = dev->real_num_tx_queues;
>> + if (txq < 1)
>> + return -EINVAL;
>>
>> - if (unlikely(txq > dev->num_tx_queues))
>> - ;
>> - else if (txq > real_num)
>> - dev->real_num_tx_queues = txq;
>> - else if (txq < real_num) {
>> - dev->real_num_tx_queues = txq;
>> - qdisc_reset_all_tx_gt(dev, txq);
>> - }
>> + if (dev->reg_state == NETREG_REGISTERED) {
>> + ASSERT_RTNL();
>> +
>> + if (txq > dev->num_tx_queues)
>> + return -EINVAL;
>> +
>> + if (txq < dev->real_num_tx_queues)
>> + qdisc_reset_all_tx_gt(dev, txq);
>> + } else
>> + dev->num_tx_queues = txq;
> [...]
>
> The kernel-doc comment should be updated to reflect the locking
> requirement and the possibility of failure when called after
> registration.
>
> Ben.
>
^ permalink raw reply
* Re: [MeeGo-Dev] can: How to set bitrate
From: Masayuki Ohtake @ 2010-10-19 0:09 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: andrew.chih.howe.khor-ral2JQCrhuEAvxtiuMwx3w, Samuel Ortiz,
Foster, Margie, ML netdev, ML linux-kernel,
socketcan-core-0fE9KPoRgkgATYTw5x5z8w, Wang, Yong Y,
Ewe, Kok Howg, Intel OTC, Tomoya MORINAGA, David S. Miller,
Christian Pellegrin, Wang, Qi
In-Reply-To: <4CBC29E6.3070601@grandegger.com>
On Monday, October 18, 2010 8:05 PM, Wolfgang Grandegger wrote:
> ~$ rpm -qf /usr/include/db_185.h
> db4-devel-4.7.25-13.fc12.i686
>
> Which means you need db4 devel to fully build iproute2.
Thank you for your information.
Using the following command,
"yum install db4*"
I could confirm building iproute2 is success and
set bitrate.
Thanks, Ohtake(OKISemi)
^ permalink raw reply
* [PATCH] bridge: make br_parse_ip_options static
From: Stephen Hemminger @ 2010-10-19 0:03 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Signed-off-by: Stephen Hemminger <shemminger@vyatta.com>
--- a/net/bridge/br_netfilter.c 2010-10-18 17:01:36.903364885 -0700
+++ b/net/bridge/br_netfilter.c 2010-10-18 17:01:48.106569141 -0700
@@ -213,7 +213,7 @@ static inline void nf_bridge_update_prot
* expected format
*/
-int br_parse_ip_options(struct sk_buff *skb)
+static int br_parse_ip_options(struct sk_buff *skb)
{
struct ip_options *opt;
struct iphdr *iph;
^ permalink raw reply
* Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic
From: Neil Horman @ 2010-10-18 23:59 UTC (permalink / raw)
To: Paul Gortmaker; +Cc: davem, netdev, allan.stephens
In-Reply-To: <20101018214356.GA27204@windriver.com>
On Mon, Oct 18, 2010 at 05:43:56PM -0400, Paul Gortmaker wrote:
> [Re: [PATCH net-next 3/5] tipc: Optimizations to bearer enabling logic] On 18/10/2010 (Mon 06:50) Neil Horman wrote:
>
> > On Fri, Oct 15, 2010 at 05:31:16PM -0400, Paul Gortmaker wrote:
> > > On 10-10-15 07:00 AM, Neil Horman wrote:
> > >
> > > [...]
> > >
> > > > This definately looks more concise, but I don't see why its necessecary to drop
> > > > the tipc_net_lock around the call to enable_bearer. The only caler of
> > > > tipc_register_media sets the enable_bearer pointer to the enable_bearer
> > > > function, and I' don't see any path through that function which would
> > > > potentially retake that lock. In fact it seems dropping it might be dangerous,
> > > > if other paths (like from cfg_named_msg_event), tried to enable a bearer in
> > > > parallel with a user space directive from the netlink socket). With out the
> > > > protection of tipc_net_lock, you could use the same eth_bearer twice, and
> > > > corrupt that array.
> > >
> > > I think it would be protected by config_lock. but in the end if it is
> > > just an academic optimization that really isn't in a hot code path
> > > anyway, and if it just adds more confusion than real world value, then
> > > I'm fine with just dropping this on the floor (and deleting the extra
> > > memset in a cleanup patch) -- it won't be the 1st time doing this with
> > > these carry forward patches, and I'm sure it won't be the last.
> > >
> >
> > Copy that.
>
> And here is all that is left once I drop all the above. Not much. :)
>
> Thanks again,
> Paul.
>
> From 35b078621c4ca6e6f5a5aed80c34594e00f08c8e Mon Sep 17 00:00:00 2001
> From: Allan Stephens <allan.stephens@windriver.com>
> Date: Thu, 14 Oct 2010 16:09:23 -0400
> Subject: [PATCH] tipc: delete needless memset from bearer enabling.
>
> Eliminates zeroing out of the new bearer structure at the start of
> activation, since it is already in that state.
>
> Signed-off-by: Allan Stephens <allan.stephens@windriver.com>
> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
> ---
> net/tipc/bearer.c | 2 --
> 1 files changed, 0 insertions(+), 2 deletions(-)
>
> diff --git a/net/tipc/bearer.c b/net/tipc/bearer.c
> index fd9c06c..9927d1d 100644
> --- a/net/tipc/bearer.c
> +++ b/net/tipc/bearer.c
> @@ -556,8 +556,6 @@ restart:
> }
>
> b_ptr = &tipc_bearers[bearer_id];
> - memset(b_ptr, 0, sizeof(struct bearer));
> -
> strcpy(b_ptr->publ.name, name);
> res = m_ptr->enable_bearer(&b_ptr->publ);
> if (res) {
> --
> 1.7.2.1
>
> --
> 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
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply
* Re: Linux 2.6.35/TIPC 2.0 ABI breaking changes
From: Alan Cox @ 2010-10-18 23:58 UTC (permalink / raw)
To: David Miller
Cc: paul.gortmaker, luca, jon.maloy, tipc-discussion, linux-kernel,
netdev
In-Reply-To: <20101018.151708.193712688.davem@davemloft.net>
On Mon, 18 Oct 2010 15:17:08 -0700 (PDT)
David Miller <davem@davemloft.net> wrote:
> From: Paul Gortmaker <paul.gortmaker@windriver.com>
> Date: Mon, 18 Oct 2010 16:42:33 -0400
>
> > If you have access to the user space code in question, you can just
> > switch behaviour semantics based on the results of a uname call, knowing
> > that this change was included in versions since approx last Feb. There
> > is also /proc/version which can be parsed manually if you prefer.
>
> Requiring userspace to check kernel versioning information in order
> to user an exported userspace API correctly is _ALWAYS_ _WRONG_.
>
> You cannot and must not make backwards incompatible changes to
> userspace interfaces.
>
> Really, you can't.
In which case given that most distros are shipping older stuff and care
about ABI stability presumably the bug should get fixed for 2.6.36 ?
^ permalink raw reply
* Re: Linux 2.6.35/TIPC 2.0 ABI breaking changes
From: Neil Horman @ 2010-10-18 23:45 UTC (permalink / raw)
To: Paul Gortmaker
Cc: jon.maloy, netdev, linux-kernel, luca, tipc-discussion,
David Miller
In-Reply-To: <4CBCD429.8090306@windriver.com>
On Mon, Oct 18, 2010 at 07:11:37PM -0400, Paul Gortmaker wrote:
> On 10-10-18 06:17 PM, David Miller wrote:
> > From: Paul Gortmaker<paul.gortmaker@windriver.com>
> > Date: Mon, 18 Oct 2010 16:42:33 -0400
> >
> >> If you have access to the user space code in question, you can just
> >> switch behaviour semantics based on the results of a uname call, knowing
> >> that this change was included in versions since approx last Feb. There
> >> is also /proc/version which can be parsed manually if you prefer.
> >
> > Requiring userspace to check kernel versioning information in order
> > to user an exported userspace API correctly is _ALWAYS_ _WRONG_.
> >
> > You cannot and must not make backwards incompatible changes to
> > userspace interfaces.
>
> What I think has happened here (and I'll double check this
> tomorrow, since it is before I started assisting with tipc)
> is that a backwards incompatible change *did* inadvertently
> creep in via these two (related) commits:
>
> --------------
> commit d88dca79d3852a3623f606f781e013d61486828a
> Author: Neil Horman <nhorman@tuxdriver.com>
> Date: Mon Mar 8 12:20:58 2010 -0800
>
> tipc: fix endianness on tipc subscriber messages
> --------------
>
> and
>
> ---------------
> commit c6537d6742985da1fbf12ae26cde6a096fd35b5c
> Author: Jon Paul Maloy <jon.maloy@ericsson.com>
> Date: Tue Apr 6 11:40:52 2010 +0000
>
> TIPC: Updated topology subscription protocol according to latest spec
> ---------------
>
> Based on Leandro's info, I think it comes down to userspace
> not knowing exactly where to find these bits anymore:
>
> #define TIPC_SUB_SERVICE 0x00 /* Filter for service availability */
> #define TIPC_SUB_PORTS 0x01 /* Filter for port availability */
> #define TIPC_SUB_CANCEL 0x04 /* Cancel a subscription */
>
That shouldn't be the case. Prior to the above changes the tipc implementation
tracked the endianess of the hosts to which it was connected and swapped data
that it sent to those hosts accordingly. With these changes the kernel client
simply swaps the data to network byte order on send and swaps it back to local
order on receive universally. That second commit added a bit from the reserved
pool of one of the connection establishment messages to indicate that a peer was
using this new protocol. If some non-local byte order information is making it
into user space, thats a bug that needs fixing.
What may be happening is some old client that doesn't know about the new bit
might be communicating with an new client that does. IIRC the spec called for
clients that set bits in the reserved field to drop frames from that client, so
that condition shouldn't occur, but TIPC may just be ignoring reserved bits. I
wouldn't be suprised.
Its also possible that the payload data between applications using tipc follow
the same broken byte swapping method that the protocol itself did, but if that
were the case I would expect the application to continue running normally,
unless user space had direct access to the protocol header in its entirety, and
read it directly, in which case I think I would just cry.
> ...because it doesn't know if there is the old auto endian
> swap thing being done or not being done.
>
> Assuming it is possible to do so in some non-kludgy way,
> it sounds like we want to be looking into an in-kernel change
> that ensures the older user space binaries get their
> functionality restored then?
>
Lets try figure out exactly what data is getting mis-read first. Maybe we can
fix it without having to go back to making a sending host figure out a receiving
hosts byte order. That would be nice. Can you describe the problem in more
detail?
Neil
> Thanks,
> Paul.
>
------------------------------------------------------------------------------
Download new Adobe(R) Flash(R) Builder(TM) 4
The new Adobe(R) Flex(R) 4 and Flash(R) Builder(TM) 4 (formerly
Flex(R) Builder(TM)) enable the development of rich applications that run
across multiple browsers and platforms. Download your free trials today!
http://p.sf.net/sfu/adobe-dev2dev
^ permalink raw reply
* Re: Linux 2.6.35/TIPC 2.0 ABI breaking changes
From: Leandro Lucarella @ 2010-10-18 23:38 UTC (permalink / raw)
To: Paul Gortmaker
Cc: jon.maloy, Neil Horman, netdev, linux-kernel, tipc-discussion,
David Miller
In-Reply-To: <4CBCD429.8090306@windriver.com>
Paul Gortmaker, el 18 de octubre a las 19:11 me escribiste:
> On 10-10-18 06:17 PM, David Miller wrote:
> > From: Paul Gortmaker<paul.gortmaker@windriver.com>
> > Date: Mon, 18 Oct 2010 16:42:33 -0400
> >
> >> If you have access to the user space code in question, you can just
> >> switch behaviour semantics based on the results of a uname call, knowing
> >> that this change was included in versions since approx last Feb. There
> >> is also /proc/version which can be parsed manually if you prefer.
> >
> > Requiring userspace to check kernel versioning information in order
> > to user an exported userspace API correctly is _ALWAYS_ _WRONG_.
> >
> > You cannot and must not make backwards incompatible changes to
> > userspace interfaces.
>
> What I think has happened here (and I'll double check this
> tomorrow, since it is before I started assisting with tipc)
> is that a backwards incompatible change *did* inadvertently
> creep in via these two (related) commits:
>
> --------------
> commit d88dca79d3852a3623f606f781e013d61486828a
> Author: Neil Horman <nhorman@tuxdriver.com>
> Date: Mon Mar 8 12:20:58 2010 -0800
>
> tipc: fix endianness on tipc subscriber messages
> --------------
>
> and
>
> ---------------
> commit c6537d6742985da1fbf12ae26cde6a096fd35b5c
> Author: Jon Paul Maloy <jon.maloy@ericsson.com>
> Date: Tue Apr 6 11:40:52 2010 +0000
>
> TIPC: Updated topology subscription protocol according to latest spec
> ---------------
Exactly. The damage is already done, the thing now is how to proceed.
What I'm doing right now is exactly that, use uname(2) to see what
kernel version is running and act according. Act according means, check
how with which tipc.h header the program was compiled (pre or post
2.6.35), compare against the current running kernel version, fix the BO
of subscriptions and fix the filter flags using my own constants
TIPC_SUB_SERVICE_V1 and TIPC_SUB_SERVICE_V2. Is not trivial, is not
nice.
And worse, as I said, for other languages bindings, specially those that
can't directly include a .h, there is no easy solution at all.
> Based on Leandro's info, I think it comes down to userspace
> not knowing exactly where to find these bits anymore:
>
> #define TIPC_SUB_SERVICE 0x00 /* Filter for service availability */
> #define TIPC_SUB_PORTS 0x01 /* Filter for port availability */
> #define TIPC_SUB_CANCEL 0x04 /* Cancel a subscription */
>
> ...because it doesn't know if there is the old auto endian
> swap thing being done or not being done.
There are, at least, 2 problems here. One is the auto endian swap (which
I knew it existed just because of this issue). But the auto endian swap
is not fully backward compatible either AFAIK, at least I tried Gentoo's
kernel 2.6.22 r5 and the topology services doesn't work with NBO (I
don't know if is a bug introduced by Gentoo, or a bug in the kernel or
what else). So just changing the BO and claiming backward compatibility
(even when is only at source code level) is not entirely true (unless is
a Gentoo issue of course, which I should probably check).
The other problem is TIPC_SUB_SERVICE changed it value from 2 to 0. In
TIPC 1.x it was a flag set in the filter member of a subscription. In
TIPC 2.0 is the absence of TIPC_SUB_PORTS. The change seems reasonable,
as TIPC_SUB_SERVICE and TIPC_SUB_PORTS were mutually exclusive and you
had to use one always, but the result is an API and *ABI* change. You
can't use an old TIPC application without changing the code and
recompiling using a tipc.h header from 2.6.35 or newer. And then, if you
need to reboot with an older kernel, the new compiled application won't
work with the old kernel.
A third problem, much less critical but which I find annoying, is that
an inconsistency was introduced. In kernels older than 2.6.35 (TIPC 1.6)
you used HBO for all the TIPC data structures, addresses, port names,
port sequences, port ids, subscriptions, events. Now, subscriptions and
events go in NBO (which contains port sequences and port ids), but when
binding a port name/sequence, you have to use HBO. Is true that
a subscription is supposed to be a network message and the port name you
bind to is not, but is at least inconsistent with AF_INET, where all you
do uses NBO.
> Assuming it is possible to do so in some non-kludgy way,
> it sounds like we want to be looking into an in-kernel change
> that ensures the older user space binaries get their
> functionality restored then?
I think the most reasonable way to do this was to use a different port
name for the topology service for TIPC 2.0, and keep the old TIPC 1.x
topology service at the same port name it was, and using the same
interface as it did. Adding new constants TIPC_TOP_SRV2 and
TIPC_SUB_SERVICE2 (for example) would have been enough to keep backward
compatibility and allow a new clean interface. The old topology service
could be deprecated and completely removed some time in a distant
future. You could even add some #ifdef TIPC_1_COMPATIBILITY to make sure
people using the old interface is aware of that and update to the new
one.
--
Leandro Lucarella (AKA luca) http://llucax.com.ar/
----------------------------------------------------------------------
GPG Key: 5F5A8D05 (F8CD F9A7 BF00 5431 4145 104C 949E BFB6 5F5A 8D05)
----------------------------------------------------------------------
If you do not change your beliefs
Your life will always be like this
------------------------------------------------------------------------------
Download new Adobe(R) Flash(R) Builder(TM) 4
The new Adobe(R) Flex(R) 4 and Flash(R) Builder(TM) 4 (formerly
Flex(R) Builder(TM)) enable the development of rich applications that run
across multiple browsers and platforms. Download your free trials today!
http://p.sf.net/sfu/adobe-dev2dev
^ 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