Netdev List
 help / color / mirror / Atom feed
* Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
From: Robin Holt @ 2011-08-08 16:08 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA, U Bhaskar-B22300,
	Marc Kleine-Budde
In-Reply-To: <4E4008BA.6030303-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>

On Mon, Aug 08, 2011 at 06:03:06PM +0200, Wolfgang Grandegger wrote:
> On 08/08/2011 05:55 PM, Robin Holt wrote:
> > On Mon, Aug 08, 2011 at 05:33:53PM +0200, Wolfgang Grandegger wrote:
> >> On 08/08/2011 05:14 PM, Marc Kleine-Budde wrote:
> > ...
> > 
> >>> On 08/08/2011 04:59 PM, Wolfgang Grandegger wrote:
> >>>> On 08/08/2011 04:44 PM, Robin Holt wrote:
> >>>>> On Mon, Aug 08, 2011 at 04:37:44PM +0200, Wolfgang Grandegger wrote:
> >>>>>> On 08/08/2011 04:21 PM, Robin Holt wrote:
> >>>>>>> On Mon, Aug 08, 2011 at 04:16:27PM +0200, Wolfgang Grandegger wrote:
> >>>>>>>> On 08/08/2011 03:56 PM, Robin Holt wrote:
> >>>>>>>>>> commit 65bb8b060a873fa4f5188f2951081f6011259614
> >>>>>>>>>> Author: Bhaskar Upadhaya <Bhaskar.Upadhaya-KZfg59tc24xl57MIdRCFDg@public.gmane.org>
> >>>>>>>>>> Date:   Fri Mar 4 20:27:58 2011 +0530
> >>>>>>>>>
> >>>>>>>>> On a side note, that commit fixes up "fsl,flexcan-v1.0"
> >>>>>>>>> ...
> >>>>>>>>> +       do_fixup_by_compat_u32(blob, "fsl,flexcan-v1.0",
> >>>>>>>>> +                       "clock_freq", gd->bus_clk, 1);
> >>>>>>>>>
> >>>>>>>>> Should I go back to flexcan-v1.0 in my patches?
> >>>>>>>>
> >>>>>>>> Well, no. Let's wait. I don't think we need it. Also, it sets
> >>>>>>>> "clock_freq" while
> >>>>>>>>
> >>>>>>>>  http://lxr.linux.no/#linux+v3.0.1/Documentation/devicetree/bindings/net/can/fsl-flexcan.txt
> >>>>>>>>
> >>>>>>>> documents "clock-frequencies"... :-(.
> >>>>>>>
> >>>>>>> You answered a different question that I was asking.  I was asking if
> >>>>>>> I should change fsl,flexcan back to fsl,flexcan-v1.0 as documented on
> >>>>>>> line 5.  The clock_freq looks like a uboot change will need to be made
> >>>>>>> as well.
> >>>>>>
> >>>>>> Well, I wrote above: "Well, no. Let's wait. I don't think we need it."
> >>>>>>
> >>>>>> For the P1010 we can sinmply derive the clock frequency from
> >>>>>> "fsl_get_sys_freq()", which is fine for the time being. No extra
> >>>>>> properties, etc. The clk implemetation might go into
> >>>>>>
> >>>>>>  http://lxr.linux.no/#linux+v3.0.1/arch/powerpc/platforms/85xx/clock.c
> >>>>>>
> >>>>>> or
> >>>>>>
> >>>>>>  http://lxr.linux.no/#linux+v3.0.1/arch/powerpc/sysdev/fsl_soc.c
> >>>>>>
> >>>>>> And may depend on HAVE_CAN_FLEXCAN
> >>>>>>
> >>>>>> BTW, I have not found HAVE_CAN_FLEXCAN in your patch. What kernel are
> >>>>>> you using?
> >>>>>
> >>>>> I am starting with the v3.0 kernel, apply one patch from the freescale BSP
> >>>>> we receive under NDA which introduces the P1010RDB board into the QorIQ
> >>>>> platform, and then work from there for the flexcan stuff.  That patch
> >>>>> introduces the HAVE_CAN_FLEXCAN.  I do not like how freescale structured
> >>>>> that Kconfig bit, so I have tweaked it to be selected automatically
> >>>>> when P1010RDB, NET, and CAN are selected.  That allows the CAN_FLEXCAN
> >>>>> selection to determine is we are going to build the flexcan.c file.
> >>>>
> >>>> ARM boards select HAVE_CAN_FLEXCAN and I do not see a good reason why
> >>>> we should do it differently for PowerPC. 
> >>>>
> >>>> For mainline inclusion, you should provide your patches against the
> >>>> David Millers "net-next-2.6" tree, which already seems to have support
> >>>> for the P1010RDB:
> >>>>
> >>>>   config P1010_RDB
> >>>>         bool "Freescale P1010RDB"
> >>>>         select DEFAULT_UIMAGE
> >>>>         help
> >>>>           This option enables support for the MPC85xx RDB (P1010 RDB) board
> >>>>
> >>>>           P1010RDB contains P1010Si, which provides CPU performance up to 800
> >>>>           MHz and 1600 DMIPS, additional functionality and faster interfaces
> >>>>           (DDR3/3L, SATA II, and PCI  Express).
> >>>>
> >>>>
> >>>>> Our contact with Freescale would prefer that I not post that patch until
> >>>>> we get the OK from freescale to do so since we received it under NDA.
> >>>>
> >>>> I don't think we currently need it. I prefer dropping and cleaning up
> >>>> the device tree stuff as it is not needed for the P1010 anyway. If a
> >>>> new processor shows up with enhanced capabilities requiring
> >>>> configuration via device tree, we or somebody else can provide a patch.
> >>>> Marc, what do you think?
> >>>
> >>> ACK - The device tree bindings as in mainline's Documentation is a mess.
> >>> If the powerpc guys are happy with a clock interfaces based approach
> >>> somewhere in arch/ppc, I'm more than happy to remove:
> >>> - fsl,flexcan-clock-source (not implemented, even in the fsl driver)
> >>>
> >>> - fsl,flexcan-clock-divider \__ replace with code in arch/ppc, or
> >>> - clock-frequency           /   a single clock-frequency attribute
> >>
> >> In the "net-next-2.6" tree there is also:
> >>
> >>  $ grep flexcan arch/powerpc/boots/dts/*.dts
> >>   p1010rdb.dts:			fsl,flexcan-clock-source = "platform";
> >>   p1010rdb.dts:			fsl,flexcan-clock-source = "platform";
> >>   p1010si.dtsi:			compatible = "fsl,flexcan-v1.0";
> >>   p1010si.dtsi:			fsl,flexcan-clock-divider = <2>;
> >>   p1010si.dtsi:			compatible = "fsl,flexcan-v1.0";
> >>   p1010si.dtsi:			fsl,flexcan-clock-divider = <2>;
> >>
> >> Especially the fsl,flexcan-clock-divider = <2>; might make people think,
> >> that they could set something else.
> > 
> > I am currently lost on the direction.  I think I need something like:
> > 
> > 1) Patch 1/5 removing the "#include <mach/clock.h>" stays.
> 
> OK.

Is that an Acked-by: or not?

> 
> > 2) Patch 2/5 abstracting readl/writel stays.
> 
> OK.

Is that an Acked-by: or not?

> 
> > 3) Patch 3/5 of_match for ppc and the match string is "fsl,flexcan" stays.
> 
> Yep.

Done.

> 
> > 4) Patch 4/5 I have not been given clear direction to not do it but have
> >    not gotten a favorable response.
> 
> Please drop this one for mainline.

Done.

> 
> > 5) Patch 5/5 goes from being a powerpc patch back to being a flexcan.c
> 
> No, I just would prefer a more general place, e.g.:
> 
>  http://lxr.linux.no/#linux+v3.0.1/arch/powerpc/platforms/85xx/clock.c
> 
> Furthermore you need patches to cleanup some DTS and platform files and
> the Documentation.

So we would stay with the clk_* functions.  I assume clk_get() would
return NULL, clk_get_rate() would just return fsl_get_sys_freq() and
the other functions would do nothing.  Doesn't this really polute what
clk_* functions are supposed to do?  Aren't we making flexcan dictate
a different behavior for powerpc than for the arm (and possibly other)
architectures?

Thanks,
Robin

^ permalink raw reply

* Re: Bonding problem
From: Andy Gospodarek @ 2011-08-08 16:26 UTC (permalink / raw)
  To: Eduard Sinelnikov; +Cc: majordomo, netdev, Jay Vosburgh, Andy Gospodarek
In-Reply-To: <CANMAZFUibmkyfWP2TyjwV6wtox6LNry2Gh3AtXB2KHnhGF80rQ@mail.gmail.com>

On Sun, Aug 07, 2011 at 03:00:30PM +0300, Eduard Sinelnikov wrote:
> Hi,
> 
> In the kernel 2.6.39.3 ( /drivers/net/bond/bond_main.c).
> In the function  ‘bond_xmit_roundrobin’
> The code check if the bond is active via
> ‘bond_is_active_slave(slave)’ Function call.
> Which actually checks if the slave is backup or active
> What is the meaning of slave being  backup in round robin mode?
> Correct me if I wrong but in round robin every slave should send a
> packet, regardless of being active or backup.
> 
> Thank you,
>            Eduard

There probably is not a compelling reason to continue to have it.  There
may be a reason historically, but I'm not aware what that might be at
this point.  For modes other than active-backup, the value of
slave->link and slave->backup should always contain a value that
indicates the slave is up and available for transmit.


^ permalink raw reply

* [PATCH] drivers/net/can/sja1000/plx_pci.c: eliminate double free
From: Julia Lawall @ 2011-08-08 16:28 UTC (permalink / raw)
  To: Wolfgang Grandegger; +Cc: kernel-janitors, Joe Perches, netdev, linux-kernel

From: Julia Lawall <julia@diku.dk>

In this code, the failure_cleanup label calls the function
plx_pci_del_card, which frees everything in the card->net_dev array.  dev
is placed in this array immediately after allocation, so the two subsequent
jumps to failure_cleanup should not also call free_sja1000dev, but the
second one does.

If plx_pci_check_sja1000 fails, then free_sja1000dev is also called on
dev.  Because dev is already in the card->net_dev array, this implies that
when plx_pci_del_card is later called, it may get freed again.  So that
entry is reset to NULL after the free.

Finally, if there is a problem with one channel, there will be a hole in the
array.  card->channels counts the number of channels that have succeeded,
and does not keep track of the index of the largest element in the array
that is valid.  So the loop in plx_pci_del_card is changed to go up to
PLX_PCI_MAX_CHAN, which is only 2.

Signed-off-by: Julia Lawall <julia@diku.dk>

---
Compiled but not tested.  I'm not sure the fix is sufficient to take into
account possible failures.  In particular, is it safe to call
unregister_sja1000dev without previously having (successfully) called
register_sja1000dev?

 drivers/net/can/sja1000/plx_pci.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/can/sja1000/plx_pci.c b/drivers/net/can/sja1000/plx_pci.c
index 231385b..c7f3d4e 100644
--- a/drivers/net/can/sja1000/plx_pci.c
+++ b/drivers/net/can/sja1000/plx_pci.c
@@ -408,7 +408,7 @@ static void plx_pci_del_card(struct pci_dev *pdev)
 	struct sja1000_priv *priv;
 	int i = 0;
 
-	for (i = 0; i < card->channels; i++) {
+	for (i = 0; i < PLX_PCI_MAX_CHAN; i++) {
 		dev = card->net_dev[i];
 		if (!dev)
 			continue;
@@ -536,7 +536,6 @@ static int __devinit plx_pci_add_card(struct pci_dev *pdev,
 			if (err) {
 				dev_err(&pdev->dev, "Registering device failed "
 					"(err=%d)\n", err);
-				free_sja1000dev(dev);
 				goto failure_cleanup;
 			}
 
@@ -549,6 +548,7 @@ static int __devinit plx_pci_add_card(struct pci_dev *pdev,
 			dev_err(&pdev->dev, "Channel #%d not detected\n",
 				i + 1);
 			free_sja1000dev(dev);
+			card->net_dev[i] = NULL;
 		}
 	}
 

^ permalink raw reply related

* Re: 802.3ad bonding brain damaged?
From: Jay Vosburgh @ 2011-08-08 16:44 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Lamparter, Phillip Susi, netdev
In-Reply-To: <1312819168.2531.3.camel@edumazet-laptop>

Eric Dumazet <eric.dumazet@gmail.com> wrote:

>Le lundi 08 août 2011 à 09:57 +0200, David Lamparter a écrit :
>> Am Sonntag, den 07.08.2011, 15:52 -0400 schrieb Phillip Susi:
>> > - From Documentation/networking/bonding.txt:
>> > 
>> > 	Additionally, the linux bonding 802.3ad implementation
>> > 	distributes traffic by peer (using an XOR of MAC addresses),
>> > 
>> > This is counter to the entire point of 802.3ad. Distributing traffic by
>> > hash of the destination address is poor mans load balancing for
>> > systems not supporting 802.3ad. 
>> 
>> No, it isn't. 802.3ad/.1AX explicitly requires that no packet
>> re-ordering may ever occur, which can only be guaranteed by enqueueing
>> packets for one host on one TX interface. This behaviour is mandated by
>> 802.1AX-2008 page 15 which reads:
>> 
>>   This standard does not mandate any particular distribution
>>   algorithm(s); however, any distribution algorithm shall ensure that,
>>   when frames are received by a Frame Collector as specified in 5.2.3,
>>   the algorithm shall not cause
>>   a) Misordering of frames that are part of any given conversation, or
>>   b) Duplication of frames.
>> | The above requirement to maintain frame ordering is met by ensuring
>> | that all frames that compose a given conversation are transmitted on a
>> | single link in the order that they are generated by the MAC Client;
>>   hence, this requirement does not involve the addition (or
>>   modification) of any information to the MAC frame, nor any buffering
>>   or processing on the part of the corresponding Frame Collector in
>>   order to reorder frames. This approach to the operation of the
>>   distribution function permits a wide variety of distribution and load
>>   balancing algorithms to be used, while also ensuring interoperability
>>   between devices that adopt differing algorithms.
>> 
>
>It all depends on the definition of 'conversation'

	The definition from 802.1AX is:

3.8 conversation: A set of frames transmitted from one end station to
another, where all of the frames form an ordered sequence, and where the
communicating end stations require the ordering to be maintained among
the set of frames exchanged. (See IEEE Std 802.1AX, Clause 5.)

	So, basically, a TCP connection or a sequence of UDP datagrams
from one IP.port to another and optionally the reverse.

>Phillip assumed two (or more) TCP flows from machine A to machine B
>could use two different links, while you assert they MUST use a single
>link.

	The standard permits us to place separate conversations on
different ports, even if they are going to the same MAC destination.  

	802.1AX 5.2.1:

f) Frame ordering must be maintained for certain sequences of frame
exchanges between MAC Clients (known as conversations, see Clause
3). The Distributor ensures that all frames of a given conversation are
passed to a single port. For any given port, the Collector is required
to pass frames to the MAC Client in the order that they are received
from that port. The Collector is otherwise free to select frames
received from the aggregated ports in any order. Since there are no
means for frames to be misordered on a single link, this guarantees that
frame ordering is maintained for any conversation.

g) Conversations may be moved among ports within an aggregation, both
for load balancing and to maintain availability in the event of link
failures.

	The standard requires ordering for frames within any one
conversation, but does not require ordering of frames between
conversations.

	The layer2 (MAC) and layer3 (MAC + IP) hashes in bonding are
compliant to this.  The layer3+4 (IP + TCP/UDP port) is not, because
fragmented datagrams will hash differently than unfragmented datagrams.
I've not heard that this noncompliance has been a problem in actual
practice.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* Re: [RFC PATCH] common receive API + r8169 use
From: Eric Dumazet @ 2011-08-08 16:47 UTC (permalink / raw)
  To: Michał Mirosław; +Cc: Stephen Hemminger, netdev
In-Reply-To: <20110802214349.GA17411@rere.qmqm.pl>

Le mardi 02 août 2011 à 23:43 +0200, Michał Mirosław a écrit :

> I don't have fast enough transmitter yet, so have no real data. Eric's
> testing showed dramatic reduction in CPU usage after changing igb to use
> build_skb().  Inlined version of this patch should give similar results.
> 
> Eric: can you share the igb changes? I have no hardware for it, but could
> merge our changes for you to test.
> 

Hi Michal

I am just coming back from one vacation period, I'll send patches before
another one, maybe tomorrow, stay tuned ;)






^ permalink raw reply

* Re: Bonding problem
From: Jay Vosburgh @ 2011-08-08 17:06 UTC (permalink / raw)
  To: Andy Gospodarek; +Cc: Eduard Sinelnikov, netdev
In-Reply-To: <20110808162645.GT21309@gospo.rdu.redhat.com>


Andy Gospodarek <andy@greyhouse.net> wrote:

>On Sun, Aug 07, 2011 at 03:00:30PM +0300, Eduard Sinelnikov wrote:
>> Hi,
>> 
>> In the kernel 2.6.39.3 ( /drivers/net/bond/bond_main.c).
>> In the function  ‘bond_xmit_roundrobin’
>> The code check if the bond is active via
>> ‘bond_is_active_slave(slave)’ Function call.
>> Which actually checks if the slave is backup or active
>> What is the meaning of slave being  backup in round robin mode?
>> Correct me if I wrong but in round robin every slave should send a
>> packet, regardless of being active or backup.
>> 
>> Thank you,
>>            Eduard
>
>There probably is not a compelling reason to continue to have it.  There
>may be a reason historically, but I'm not aware what that might be at
>this point.  For modes other than active-backup, the value of
>slave->link and slave->backup should always contain a value that
>indicates the slave is up and available for transmit.

	If you read Eduard's other posts regarding this, the actual
issue is that when changing from another mode into round-robin,
occasionally slaves will still be marked as "backup" and won't be used:

>Date: 	Mon, 8 Aug 2011 11:16:39 +0300
>Subject: On line Bonding configuration change fails
>From: Eduard Sinelnikov <eduard.sinelnikov@gmail.com>
>To: netdev@vger.kernel.org
>Sender: netdev-owner@vger.kernel.org
>
>Hi,
>
>My configuration is a follows:
>
>             | eth0 -------------->
>Ububntu | eth1 -------------->    Swith ------------> Other computer
>
>Scenario:
>• change the bond mode to active/backup
>• unplug some of the cable
>• plug-in the unplugged cable
>• change bond mode to round robin
>
>I can see that only one eth1 is sending data. When I unplug it the ping stops.
>
>Is it a bug or some mis-configuration?
>
>In the kernel ( /drivers/net/bond/bond_main.c).
>In the function  ‘bond_xmit_roundrobin
>’
>The code check if the bond is active via
>‘bond_is_active_slave(slave)’ Function call.
>Which actually checks if the slave is backup or active
>What is the meaning of backup in round robin?
>Correct me if I wrong but in round robin every slave should send a
>packet, regardless of being active or backup.

	So from looking at the code, it seems that the actual problem is
that when transitioning to round-robin mode, one or more slaves can
remain marked as "backup," and in round-robin mode, that won't ever
change.  We could probably work around that by removing the "is_active"
test (essentially declaring that "is_active" is only valid in
active-backup mode).  That might produce a few odd messages here and
there (when removing a slave or during a link failure, for example).

	From inspection, the bond_xmit_xor function likely has this same
problem.

	-J

---
	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com

^ permalink raw reply

* Re: Bonding problem
From: Andy Gospodarek @ 2011-08-08 17:31 UTC (permalink / raw)
  To: Jay Vosburgh; +Cc: Andy Gospodarek, Eduard Sinelnikov, netdev
In-Reply-To: <26478.1312823165@death>

On Mon, Aug 08, 2011 at 10:06:05AM -0700, Jay Vosburgh wrote:
> 
> Andy Gospodarek <andy@greyhouse.net> wrote:
> 
> >On Sun, Aug 07, 2011 at 03:00:30PM +0300, Eduard Sinelnikov wrote:
> >> Hi,
> >> 
> >> In the kernel 2.6.39.3 ( /drivers/net/bond/bond_main.c).
> >> In the function  ‘bond_xmit_roundrobin’
> >> The code check if the bond is active via
> >> ‘bond_is_active_slave(slave)’ Function call.
> >> Which actually checks if the slave is backup or active
> >> What is the meaning of slave being  backup in round robin mode?
> >> Correct me if I wrong but in round robin every slave should send a
> >> packet, regardless of being active or backup.
> >> 
> >> Thank you,
> >>            Eduard
> >
> >There probably is not a compelling reason to continue to have it.  There
> >may be a reason historically, but I'm not aware what that might be at
> >this point.  For modes other than active-backup, the value of
> >slave->link and slave->backup should always contain a value that
> >indicates the slave is up and available for transmit.
> 
> 	If you read Eduard's other posts regarding this, the actual
> issue is that when changing from another mode into round-robin,
> occasionally slaves will still be marked as "backup" and won't be used:
> 

I did notice that one after I sent this first response.

> >Date: 	Mon, 8 Aug 2011 11:16:39 +0300
> >Subject: On line Bonding configuration change fails
> >From: Eduard Sinelnikov <eduard.sinelnikov@gmail.com>
> >To: netdev@vger.kernel.org
> >Sender: netdev-owner@vger.kernel.org
> >
> >Hi,
> >
> >My configuration is a follows:
> >
> >             | eth0 -------------->
> >Ububntu | eth1 -------------->    Swith ------------> Other computer
> >
> >Scenario:
> >• change the bond mode to active/backup
> >• unplug some of the cable
> >• plug-in the unplugged cable
> >• change bond mode to round robin
> >
> >I can see that only one eth1 is sending data. When I unplug it the ping stops.
> >
> >Is it a bug or some mis-configuration?
> >
> >In the kernel ( /drivers/net/bond/bond_main.c).
> >In the function  ‘bond_xmit_roundrobin
> >’
> >The code check if the bond is active via
> >‘bond_is_active_slave(slave)’ Function call.
> >Which actually checks if the slave is backup or active
> >What is the meaning of backup in round robin?
> >Correct me if I wrong but in round robin every slave should send a
> >packet, regardless of being active or backup.
> 
> 	So from looking at the code, it seems that the actual problem is
> that when transitioning to round-robin mode, one or more slaves can
> remain marked as "backup," and in round-robin mode, that won't ever
> change.  We could probably work around that by removing the "is_active"
> test (essentially declaring that "is_active" is only valid in
> active-backup mode).  That might produce a few odd messages here and
> there (when removing a slave or during a link failure, for example).
> 
> 	From inspection, the bond_xmit_xor function likely has this same
> problem.
> 

Agreed.

> 	-J
> 
> ---
> 	-Jay Vosburgh, IBM Linux Technology Center, fubar@us.ibm.com
> --
> 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

* [BUG] 3.0-rc1 Bridge not forwarding unicast packages
From: Michael Guntsche @ 2011-08-08 17:42 UTC (permalink / raw)
  To: netdev, linux-kernel

Hi list,

I just upgraded my router/bridge combo to 3.1-rc1 from 3.0 for
testing. On a first look everything seemed to work fine, but when I
tried to connect via openvpn to my internal network (tap0 being bridged
with the internal network) I noticed that I was not able to access the
server on my internal network. I could access the bridge (which is
acting as the openvpn server as well) just fine though. 
To debug this I ran tcpdump on the openvpn client and started a ping to the
internal network. I could see the ARP requests being answered.

19:23:49.247846 ARP, Request who-has 192.168.42.127 tell 192.168.42.96,
length 28
19:23:49.287752 ARP, Reply 192.168.42.127 is-at 00:13:d4:4f:a2:dc,
length 46

in this case .127 is the server on the internal net and .96 the openvpn
client, but the icmp request did not arrive on the server. 
The strange thing I noticed was that I could see broadcasts packages
from the server on the client

19:23:28.135185 IP 192.168.42.127.631 > 192.168.42.255.631: UDP, length
187
19:23:29.470975 IP 192.168.42.96.5353 > 224.0.0.251.5353: .......

but no icmp packages arrived on the server side.


brctl showmacs lan
port no mac addr                is local?       ageing timer
  1     00:0c:42:28:de:4e       yes                0.00
  2     00:0c:42:61:7f:f2       yes                0.00
  1     00:13:d4:4f:a2:dc       no                 0.00 <---- server on the lan side
  3     8e:22:41:d9:95:23       yes                0.00
  3     b6:e1:e3:06:c9:1a       no                 5.00 <---- client connected via tap0

Reverting to 3.0 solves the problem for me. I tried just reverting the bridge code on the server to the 3.0 version to make sure that it is really Bridge related, but there are too many changes outside the bridge tree so compilation fails for me.

If you need more information, please to not hesitate to conact me.

Kind regards,
Michael Guntsche

^ permalink raw reply

* Re: [BUG] 3.0-rc1 Bridge not forwarding unicast packages
From: Stephen Hemminger @ 2011-08-08 17:48 UTC (permalink / raw)
  To: Michael Guntsche; +Cc: netdev, linux-kernel
In-Reply-To: <20110808192646@it-loops.com>

On Mon, 8 Aug 2011 19:42:19 +0200
Michael Guntsche <mike@it-loops.com> wrote:

> Hi list,
> 
> I just upgraded my router/bridge combo to 3.1-rc1 from 3.0 for
> testing. On a first look everything seemed to work fine, but when I
> tried to connect via openvpn to my internal network (tap0 being bridged
> with the internal network) I noticed that I was not able to access the
> server on my internal network. I could access the bridge (which is
> acting as the openvpn server as well) just fine though. 
> To debug this I ran tcpdump on the openvpn client and started a ping to the
> internal network. I could see the ARP requests being answered.
> 
> 19:23:49.247846 ARP, Request who-has 192.168.42.127 tell 192.168.42.96,
> length 28
> 19:23:49.287752 ARP, Reply 192.168.42.127 is-at 00:13:d4:4f:a2:dc,
> length 46
> 
> in this case .127 is the server on the internal net and .96 the openvpn
> client, but the icmp request did not arrive on the server. 
> The strange thing I noticed was that I could see broadcasts packages
> from the server on the client
> 
> 19:23:28.135185 IP 192.168.42.127.631 > 192.168.42.255.631: UDP, length
> 187
> 19:23:29.470975 IP 192.168.42.96.5353 > 224.0.0.251.5353: .......
> 
> but no icmp packages arrived on the server side.
> 
> 
> brctl showmacs lan
> port no mac addr                is local?       ageing timer
>   1     00:0c:42:28:de:4e       yes                0.00
>   2     00:0c:42:61:7f:f2       yes                0.00
>   1     00:13:d4:4f:a2:dc       no                 0.00 <---- server on the lan side
>   3     8e:22:41:d9:95:23       yes                0.00
>   3     b6:e1:e3:06:c9:1a       no                 5.00 <---- client connected via tap0
> 
> Reverting to 3.0 solves the problem for me. I tried just reverting the bridge code on the server to the 3.0 version to make sure that it is really Bridge related, but there are too many changes outside the bridge tree so compilation fails for me.
> 
> If you need more information, please to not hesitate to conact me.
> 
> Kind regards,
> Michael Guntsche

Do you have spanning tree enabled?
If  so you may have a packet loop and now it is being detected.

^ permalink raw reply

* Re: [RFC PATCH v2 0/9] bql: Byte Queue Limits
From: Tom Herbert @ 2011-08-08 17:51 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: davem, netdev
In-Reply-To: <20110808084016.063a0699@nehalam.ftrdhcpuser.net>

On Mon, Aug 8, 2011 at 8:40 AM, Stephen Hemminger <shemminger@vyatta.com> wrote:
> On Sun, 7 Aug 2011 21:43:13 -0700 (PDT)
> Tom Herbert <therbert@google.com> wrote:
>
>>     netdev_tx_completed_queue: Called at end of transmit completion
>>       to inform stack of number of bytes and packets processed.
>>     netdev_tx_sent_queue: Called to inform stack when packets are
>>       queued.
>
> Couldn't these be done for the device in the existing qdisc infra
> structure (or dev_start_xmit). Alternatively, rename ndo_start_xmit
> to something else and make all the callers use the wrapper.
>
> Changing all the drivers for something that the driver has no real
> need to care about seems like incorrect object design.
>
The netdev_tx_completed_queue is needed to inform the stack of number
of packets and bytes completed in an execution of transmit completion
(epic).  I don't see a way to get that information outside of the
driver.

Tom

^ permalink raw reply

* Re: [RFC PATCH v2 0/9] bql: Byte Queue Limits
From: Stephen Hemminger @ 2011-08-08 17:55 UTC (permalink / raw)
  To: Tom Herbert; +Cc: davem, netdev
In-Reply-To: <CA+mtBx_vgsH+uHLi6p80E8CEhU7SHXY=Fw1Z-W2OFWy44LP-ig@mail.gmail.com>

On Mon, 8 Aug 2011 10:51:06 -0700
Tom Herbert <therbert@google.com> wrote:

> On Mon, Aug 8, 2011 at 8:40 AM, Stephen Hemminger <shemminger@vyatta.com> wrote:
> > On Sun, 7 Aug 2011 21:43:13 -0700 (PDT)
> > Tom Herbert <therbert@google.com> wrote:
> >
> >>     netdev_tx_completed_queue: Called at end of transmit completion
> >>       to inform stack of number of bytes and packets processed.
> >>     netdev_tx_sent_queue: Called to inform stack when packets are
> >>       queued.
> >
> > Couldn't these be done for the device in the existing qdisc infra
> > structure (or dev_start_xmit). Alternatively, rename ndo_start_xmit
> > to something else and make all the callers use the wrapper.
> >
> > Changing all the drivers for something that the driver has no real
> > need to care about seems like incorrect object design.
> >
> The netdev_tx_completed_queue is needed to inform the stack of number
> of packets and bytes completed in an execution of transmit completion
> (epic).  I don't see a way to get that information outside of the
> driver.
> 
> Tom

Since transmit completion means calling dev_kfree_skb() why not account
there? You could add some info to netdev if necessary to get compile
the statistics.

I just hate driver api complexity growth.
 

^ permalink raw reply

* Re: [RFC PATCH v2 0/9] bql: Byte Queue Limits
From: Tom Herbert @ 2011-08-08 17:56 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: davem, netdev
In-Reply-To: <20110808105529.4c8c52e1@nehalam.ftrdhcpuser.net>

On Mon, Aug 8, 2011 at 10:55 AM, Stephen Hemminger
<shemminger@vyatta.com> wrote:
> On Mon, 8 Aug 2011 10:51:06 -0700
> Tom Herbert <therbert@google.com> wrote:
>
>> On Mon, Aug 8, 2011 at 8:40 AM, Stephen Hemminger <shemminger@vyatta.com> wrote:
>> > On Sun, 7 Aug 2011 21:43:13 -0700 (PDT)
>> > Tom Herbert <therbert@google.com> wrote:
>> >
>> >>     netdev_tx_completed_queue: Called at end of transmit completion
>> >>       to inform stack of number of bytes and packets processed.
>> >>     netdev_tx_sent_queue: Called to inform stack when packets are
>> >>       queued.
>> >
>> > Couldn't these be done for the device in the existing qdisc infra
>> > structure (or dev_start_xmit). Alternatively, rename ndo_start_xmit
>> > to something else and make all the callers use the wrapper.
>> >
>> > Changing all the drivers for something that the driver has no real
>> > need to care about seems like incorrect object design.
>> >
>> The netdev_tx_completed_queue is needed to inform the stack of number
>> of packets and bytes completed in an execution of transmit completion
>> (epic).  I don't see a way to get that information outside of the
>> driver.
>>
>> Tom
>
> Since transmit completion means calling dev_kfree_skb() why not account
> there? You could add some info to netdev if necessary to get compile
> the statistics.
>
> I just hate driver api complexity growth.
>
>

^ permalink raw reply

* Re: [RFC PATCH v2 0/9] bql: Byte Queue Limits
From: Tom Herbert @ 2011-08-08 18:01 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: davem, netdev
In-Reply-To: <20110808105529.4c8c52e1@nehalam.ftrdhcpuser.net>

> Since transmit completion means calling dev_kfree_skb() why not account
> there? You could add some info to netdev if necessary to get compile
> the statistics.
>
The algorithm depends on knowing the total number of packets competed
in a single execution of transmit completion (epic based).  We only
want to recalculate the limits once per completion, which happens when
the completion function is called.

> I just hate driver api complexity growth.
>
>

^ permalink raw reply

* Re: [BUG] 3.0-rc1 Bridge not forwarding unicast packages
From: Michael Guntsche @ 2011-08-08 18:02 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev, linux-kernel
In-Reply-To: <20110808104803.2762dbf7@nehalam.ftrdhcpuser.net>

On 08 Aug 11 10:48, Stephen Hemminger wrote:
> On Mon, 8 Aug 2011 19:42:19 +0200
> Michael Guntsche <mike@it-loops.com> wrote:
> 
> > Hi list,
> > 
> > I just upgraded my router/bridge combo to 3.1-rc1 from 3.0 for
> > testing. On a first look everything seemed to work fine, but when I
> > tried to connect via openvpn to my internal network (tap0 being bridged
> > with the internal network) I noticed that I was not able to access the
> > server on my internal network. I could access the bridge (which is
> > acting as the openvpn server as well) just fine though. 
> > To debug this I ran tcpdump on the openvpn client and started a ping to the
> > internal network. I could see the ARP requests being answered.
> > 
> > 19:23:49.247846 ARP, Request who-has 192.168.42.127 tell 192.168.42.96,
> > length 28
> > 19:23:49.287752 ARP, Reply 192.168.42.127 is-at 00:13:d4:4f:a2:dc,
> > length 46
> > 
> > in this case .127 is the server on the internal net and .96 the openvpn
> > client, but the icmp request did not arrive on the server. 
> > The strange thing I noticed was that I could see broadcasts packages
> > from the server on the client
> > 
> > 19:23:28.135185 IP 192.168.42.127.631 > 192.168.42.255.631: UDP, length
> > 187
> > 19:23:29.470975 IP 192.168.42.96.5353 > 224.0.0.251.5353: .......
> > 
> > but no icmp packages arrived on the server side.
> > 
> > 
> > brctl showmacs lan
> > port no mac addr                is local?       ageing timer
> >   1     00:0c:42:28:de:4e       yes                0.00
> >   2     00:0c:42:61:7f:f2       yes                0.00
> >   1     00:13:d4:4f:a2:dc       no                 0.00 <---- server on the lan side
> >   3     8e:22:41:d9:95:23       yes                0.00
> >   3     b6:e1:e3:06:c9:1a       no                 5.00 <---- client connected via tap0
> > 
> > Reverting to 3.0 solves the problem for me. I tried just reverting the bridge code on the server to the 3.0 version to make sure that it is really Bridge related, but there are too many changes outside the bridge tree so compilation fails for me.
> > 
> > If you need more information, please to not hesitate to conact me.
> > 
> > Kind regards,
> > Michael Guntsche
> 
> Do you have spanning tree enabled?
> If  so you may have a packet loop and now it is being detected.
No STP is not enabled

brctl show lan
bridge name     bridge id               STP enabled     interfaces
lan             8000.000c4228de4e       no              lan_wire
                                                        tap0
                                                        wlan0

No ebtables rules either just checked that.
Is there a way to revert just the bridge code to the 3.0 version so i can make sure it is really the bridge code, and not something else?

Kind regards,
Mike

PS: And of COURSE the subject line should be 3.1-rc1!!!! Sorry

^ permalink raw reply

* Re: [RFC PATCH v2 0/9] bql: Byte Queue Limits
From: Stephen Hemminger @ 2011-08-08 18:19 UTC (permalink / raw)
  To: Tom Herbert; +Cc: davem, netdev
In-Reply-To: <CA+mtBx_bdCGvR+xBMq9px0vOV6a6kFn0wYyJH26+CdSPLtvgfw@mail.gmail.com>

On Mon, 8 Aug 2011 11:01:57 -0700
Tom Herbert <therbert@google.com> wrote:

> > Since transmit completion means calling dev_kfree_skb() why not account
> > there? You could add some info to netdev if necessary to get compile
> > the statistics.
> >
> The algorithm depends on knowing the total number of packets competed
> in a single execution of transmit completion (epic based).  We only
> want to recalculate the limits once per completion, which happens when
> the completion function is called.

So just add some stats to netdev and count the number of dev_kfree_skb
calls and do your work at napi complete.

^ permalink raw reply

* Re: [BUG] 3.0-rc1 Bridge not forwarding unicast packages
From: Stephen Hemminger @ 2011-08-08 18:20 UTC (permalink / raw)
  To: Michael Guntsche; +Cc: netdev, linux-kernel
In-Reply-To: <20110808195930@it-loops.com>

On Mon, 8 Aug 2011 20:02:52 +0200
Michael Guntsche <mike@it-loops.com> wrote:

> On 08 Aug 11 10:48, Stephen Hemminger wrote:
> > On Mon, 8 Aug 2011 19:42:19 +0200
> > Michael Guntsche <mike@it-loops.com> wrote:
> > 
> > > Hi list,
> > > 
> > > I just upgraded my router/bridge combo to 3.1-rc1 from 3.0 for
> > > testing. On a first look everything seemed to work fine, but when I
> > > tried to connect via openvpn to my internal network (tap0 being bridged
> > > with the internal network) I noticed that I was not able to access the
> > > server on my internal network. I could access the bridge (which is
> > > acting as the openvpn server as well) just fine though. 
> > > To debug this I ran tcpdump on the openvpn client and started a ping to the
> > > internal network. I could see the ARP requests being answered.
> > > 
> > > 19:23:49.247846 ARP, Request who-has 192.168.42.127 tell 192.168.42.96,
> > > length 28
> > > 19:23:49.287752 ARP, Reply 192.168.42.127 is-at 00:13:d4:4f:a2:dc,
> > > length 46
> > > 
> > > in this case .127 is the server on the internal net and .96 the openvpn
> > > client, but the icmp request did not arrive on the server. 
> > > The strange thing I noticed was that I could see broadcasts packages
> > > from the server on the client
> > > 
> > > 19:23:28.135185 IP 192.168.42.127.631 > 192.168.42.255.631: UDP, length
> > > 187
> > > 19:23:29.470975 IP 192.168.42.96.5353 > 224.0.0.251.5353: .......
> > > 
> > > but no icmp packages arrived on the server side.
> > > 
> > > 
> > > brctl showmacs lan
> > > port no mac addr                is local?       ageing timer
> > >   1     00:0c:42:28:de:4e       yes                0.00
> > >   2     00:0c:42:61:7f:f2       yes                0.00
> > >   1     00:13:d4:4f:a2:dc       no                 0.00 <---- server on the lan side
> > >   3     8e:22:41:d9:95:23       yes                0.00
> > >   3     b6:e1:e3:06:c9:1a       no                 5.00 <---- client connected via tap0
> > > 
> > > Reverting to 3.0 solves the problem for me. I tried just reverting the bridge code on the server to the 3.0 version to make sure that it is really Bridge related, but there are too many changes outside the bridge tree so compilation fails for me.
> > > 
> > > If you need more information, please to not hesitate to conact me.
> > > 
> > > Kind regards,
> > > Michael Guntsche
> > 
> > Do you have spanning tree enabled?
> > If  so you may have a packet loop and now it is being detected.
> No STP is not enabled
> 
> brctl show lan
> bridge name     bridge id               STP enabled     interfaces
> lan             8000.000c4228de4e       no              lan_wire
>                                                         tap0
>                                                         wlan0
> 
> No ebtables rules either just checked that.
> Is there a way to revert just the bridge code to the 3.0 version so i can make sure it is really the bridge code, and not something else?
> 

You need to use git bisect, it is not necessarily the bridge code.
Could be vpn, tap, wlan or lots of other things.

^ permalink raw reply

* Re: [PATCH 11/12] headers, scc: Add missing #include to <linux/scc.h>
From: Ben Hutchings @ 2011-08-08 18:20 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Joerg Reuter, Klaus Kudielka, linux-hams
In-Reply-To: <1312809869.2591.1151.camel@deadeye>

On Mon, Aug 08, 2011 at 02:24:29PM +0100, Ben Hutchings wrote:
> <linux/scc.h> uses SIOCDEVPRIVATE, defined in <linux/sockios.h>.
 
Unfortunately SIOCDEVPRIVATE is also defined elsewhere by glibc,
so including <linux/sockios.h> can result in duplicate definitions.
So I don't think we can make this change.

Ben.

> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
> This file isn't listed in MAINTAINERS but appears to be associated with
> one of the hamradio drivers; please could one of the hams claim it?
> 
> Ben.
> 
>  include/linux/scc.h |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/include/linux/scc.h b/include/linux/scc.h
> index 3495bd9..d5916e5 100644
> --- a/include/linux/scc.h
> +++ b/include/linux/scc.h
> @@ -3,6 +3,7 @@
>  #ifndef	_SCC_H
>  #define	_SCC_H
>  
> +#include <linux/sockios.h>
>  
>  /* selection of hardware types */
>  
> -- 
> 1.7.5.4
> 
> 
> 

-- 
Ben Hutchings
We get into the habit of living before acquiring the habit of thinking.
                                                              - Albert Camus

^ permalink raw reply

* Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
From: Wolfgang Grandegger @ 2011-08-08 18:37 UTC (permalink / raw)
  To: Robin Holt
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA, Marc Kleine-Budde,
	U Bhaskar-B22300
In-Reply-To: <20110808160810.GF4926-sJ/iWh9BUns@public.gmane.org>

On 08/08/2011 06:08 PM, Robin Holt wrote:
> On Mon, Aug 08, 2011 at 06:03:06PM +0200, Wolfgang Grandegger wrote:
...
> So we would stay with the clk_* functions.  I assume clk_get() would
> return NULL, clk_get_rate() would just return fsl_get_sys_freq() and
> the other functions would do nothing.  Doesn't this really polute what
> clk_* functions are supposed to do?  Aren't we making flexcan dictate
> a different behavior for powerpc than for the arm (and possibly other)
> architectures?

Well, I see it as one way to provide compatibility with the ARM port. If
the PowerPC people don't like it, we can switch to something else,
whatever they suggest.

Wolfgang.

^ permalink raw reply

* Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
From: Marc Kleine-Budde @ 2011-08-08 18:53 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA, U Bhaskar-B22300
In-Reply-To: <4E4001E1.3030508-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>


[-- Attachment #1.1: Type: text/plain, Size: 1333 bytes --]

On 08/08/2011 05:33 PM, Wolfgang Grandegger wrote:
>> ACK - The device tree bindings as in mainline's Documentation is a mess.
>> If the powerpc guys are happy with a clock interfaces based approach
>> somewhere in arch/ppc, I'm more than happy to remove:
>> - fsl,flexcan-clock-source (not implemented, even in the fsl driver)
>>
>> - fsl,flexcan-clock-divider \__ replace with code in arch/ppc, or
>> - clock-frequency           /   a single clock-frequency attribute
> 
> In the "net-next-2.6" tree there is also:
> 
>  $ grep flexcan arch/powerpc/boots/dts/*.dts
>   p1010rdb.dts:			fsl,flexcan-clock-source = "platform";
>   p1010rdb.dts:			fsl,flexcan-clock-source = "platform";
>   p1010si.dtsi:			compatible = "fsl,flexcan-v1.0";
>   p1010si.dtsi:			fsl,flexcan-clock-divider = <2>;
>   p1010si.dtsi:			compatible = "fsl,flexcan-v1.0";
>   p1010si.dtsi:			fsl,flexcan-clock-divider = <2>;
> 
> Especially the fsl,flexcan-clock-divider = <2>; might make people think,
> that they could set something else.

ARGH... :D

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 262 bytes --]

[-- Attachment #2: Type: text/plain, Size: 188 bytes --]

_______________________________________________
Socketcan-core mailing list
Socketcan-core-0fE9KPoRgkgATYTw5x5z8w@public.gmane.org
https://lists.berlios.de/mailman/listinfo/socketcan-core

^ permalink raw reply

* Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
From: Robin Holt @ 2011-08-08 19:14 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA, Marc Kleine-Budde,
	U Bhaskar-B22300
In-Reply-To: <4E402CF1.1040300-5Yr1BZd7O62+XT7JhA+gdA@public.gmane.org>

On Mon, Aug 08, 2011 at 08:37:37PM +0200, Wolfgang Grandegger wrote:
> On 08/08/2011 06:08 PM, Robin Holt wrote:
> > On Mon, Aug 08, 2011 at 06:03:06PM +0200, Wolfgang Grandegger wrote:
> ...
> > So we would stay with the clk_* functions.  I assume clk_get() would
> > return NULL, clk_get_rate() would just return fsl_get_sys_freq() and
> > the other functions would do nothing.  Doesn't this really polute what
> > clk_* functions are supposed to do?  Aren't we making flexcan dictate
> > a different behavior for powerpc than for the arm (and possibly other)
> > architectures?
> 
> Well, I see it as one way to provide compatibility with the ARM port. If
> the PowerPC people don't like it, we can switch to something else,
> whatever they suggest.

I have spent the last few hours and I think I found the communication
problem and I think it is me.

I assumed long ago we would be better off implementing a Kconfig language
which does "select PPC_CLOCK".  This assumption was in part because I
did not understand what I was doing when I started this (still don't
honestly), but I did know the freescale patch series did a select
PPC_CLOCK.

Here is my patch for introducing the p1010 clock source.  Am I finally
starting to understand your guidance?

Thanks,
Robin

------------------------------------------------------------------------

diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
index 498534c..ed4cf92 100644
--- a/arch/powerpc/platforms/85xx/Kconfig
+++ b/arch/powerpc/platforms/85xx/Kconfig
@@ -26,6 +26,10 @@ config MPC8560_ADS
 	help
 	  This option enables support for the MPC 8560 ADS board
 
+config 85xx_HAVE_CAN_FLEXCAN
+	bool
+	select HAVE_CAN_FLEXCAN if NET && CAN
+
 config MPC85xx_CDS
 	bool "Freescale MPC85xx CDS"
 	select DEFAULT_UIMAGE
@@ -70,6 +74,8 @@ config MPC85xx_RDB
 config P1010_RDB
 	bool "Freescale P1010RDB"
 	select DEFAULT_UIMAGE
+	select 85xx_HAVE_CAN_FLEXCAN
+	select PPC_CLOCK if CAN_FLEXCAN
 	help
 	  This option enables support for the MPC85xx RDB (P1010 RDB) board
 
diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
index a971b32..64ad7a4 100644
--- a/arch/powerpc/platforms/85xx/Makefile
+++ b/arch/powerpc/platforms/85xx/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_MPC85xx_DS)  += mpc85xx_ds.o
 obj-$(CONFIG_MPC85xx_MDS) += mpc85xx_mds.o
 obj-$(CONFIG_MPC85xx_RDB) += mpc85xx_rdb.o
 obj-$(CONFIG_P1010_RDB)   += p1010rdb.o
+obj-$(CONFIG_PPC_CLOCK)   += clock.o
 obj-$(CONFIG_P1022_DS)    += p1022_ds.o
 obj-$(CONFIG_P1023_RDS)   += p1023_rds.o
 obj-$(CONFIG_P2040_RDB)   += p2040_rdb.o corenet_ds.o
diff --git a/arch/powerpc/platforms/85xx/p1010rdb.c b/arch/powerpc/platforms/85xx/p1010rdb.c
index d7387fa..29e04d6 100644
--- a/arch/powerpc/platforms/85xx/p1010rdb.c
+++ b/arch/powerpc/platforms/85xx/p1010rdb.c
@@ -81,6 +81,13 @@ static void __init p1010_rdb_setup_arch(void)
 	printk(KERN_INFO "P1010 RDB board from Freescale Semiconductor\n");
 }
 
+static void __init p1010_rdb_init(void)
+{
+#ifdef PPC_CLOCK
+	p1010_rdb_clk_init();
+#endif
+}
+
 static struct of_device_id __initdata p1010rdb_ids[] = {
 	{ .type = "soc", },
 	{ .compatible = "soc", },
@@ -111,6 +118,7 @@ define_machine(p1010_rdb) {
 	.name			= "P1010 RDB",
 	.probe			= p1010_rdb_probe,
 	.setup_arch		= p1010_rdb_setup_arch,
+	.init			= p1010_rdb_init,
 	.init_IRQ		= p1010_rdb_pic_init,
 #ifdef CONFIG_PCI
 	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
-- 
1.7.2.1

^ permalink raw reply related

* Re: 802.3ad bonding brain damaged?
From: Phillip Susi @ 2011-08-08 20:06 UTC (permalink / raw)
  To: David Lamparter; +Cc: netdev
In-Reply-To: <1312790234.7020.26.camel@arkology.n2.diac24.net>

On 8/8/2011 3:57 AM, David Lamparter wrote:
> No, it isn't. 802.3ad/.1AX explicitly requires that no packet
> re-ordering may ever occur, which can only be guaranteed by enqueueing
> packets for one host on one TX interface. This behaviour is mandated by
> 802.1AX-2008 page 15 which reads:

Outch, that does cause a big problem for store-and-forward switching. 
You basically can't split up packets from a single stream without very 
careful cut-through switching, which we obviously can't do in Linux. 
That seems a rather silly requirement given that higher level protocols 
already deal with packet reordering.  Why not an option to say stuff the 
standard?

^ permalink raw reply

* Re: [PATCHv4] Bridge: Always send NETDEV_CHANGEADDR up on br MAC change.
From: Stephen Hemminger @ 2011-08-08 20:12 UTC (permalink / raw)
  To: Andrei Warkentin; +Cc: netdev
In-Reply-To: <1312578250-2944-2-git-send-email-andreiw@motorola.com>

On Fri,  5 Aug 2011 16:04:10 -0500
Andrei Warkentin <andreiw@motorola.com> wrote:

> This ensures the neighbor entries associated with the bridge
> dev are flushed, also invalidating the associated cached L2 headers.
> 
> This means we br_add_if/br_del_if ports to implement hand-over and
> not wind up with bridge packets going out with stale MAC.
> 
> This means we can also change MAC of port device and also not wind
> up with bridge packets going out with stale MAC.
> 
> This builds on Stephen Hemminger's patch, also handling the br_del_if
> case and the port MAC change case.
> 
> Cc: Stephen Hemminger <shemminger@vyatta.com>
> Signed-off-by: Andrei Warkentin <andreiw@motorola.com>

Acked-by: Stephen Hemminger <shemminger@vyatta.com>

^ permalink raw reply

* Re: 802.3ad bonding brain damaged?
From: Chris Adams @ 2011-08-08 20:08 UTC (permalink / raw)
  To: netdev
In-Reply-To: <4E4041B5.5040908@cfl.rr.com>

Once upon a time, Phillip Susi <psusi@cfl.rr.com> said:
> On 8/8/2011 3:57 AM, David Lamparter wrote:
> >No, it isn't. 802.3ad/.1AX explicitly requires that no packet
> >re-ordering may ever occur, which can only be guaranteed by enqueueing
> >packets for one host on one TX interface. This behaviour is mandated by
> >802.1AX-2008 page 15 which reads:
> 
> Outch, that does cause a big problem for store-and-forward switching. 
> You basically can't split up packets from a single stream without very 
> careful cut-through switching, which we obviously can't do in Linux. 
> That seems a rather silly requirement given that higher level protocols 
> already deal with packet reordering.  Why not an option to say stuff the 
> standard?

Packet reordering introduces jitter, which is bad for things like VOIP.
-- 
Chris Adams <cmadams@hiwaay.net>
Systems and Network Administrator - HiWAAY Internet Services
I don't speak for anybody but myself - that's enough trouble.

^ permalink raw reply

* Re: 802.3ad bonding brain damaged?
From: Chris Friesen @ 2011-08-08 20:14 UTC (permalink / raw)
  To: Phillip Susi; +Cc: David Lamparter, netdev
In-Reply-To: <4E4041B5.5040908@cfl.rr.com>

On 08/08/2011 02:06 PM, Phillip Susi wrote:
> On 8/8/2011 3:57 AM, David Lamparter wrote:
>> No, it isn't. 802.3ad/.1AX explicitly requires that no packet
>> re-ordering may ever occur, which can only be guaranteed by enqueueing
>> packets for one host on one TX interface. This behaviour is mandated by
>> 802.1AX-2008 page 15 which reads:
>
> Outch, that does cause a big problem for store-and-forward switching.
> You basically can't split up packets from a single stream without very
> careful cut-through switching, which we obviously can't do in Linux.
> That seems a rather silly requirement given that higher level protocols
> already deal with packet reordering. Why not an option to say stuff the
> standard?

Bonding doesn't know about "higher level protocols".  Also, assuming 
that higher level protocols already deal with reordering can be 
dangerous.  I've dealt with network protocols and apps that assumed 
there would be no reordering because at the time they were written they 
used point-to-point links.  They actually work fairly well with single 
links, so it would be reasonable to try and keep them working with 
bonded links.

Chris

-- 
Chris Friesen
Software Developer
GENBAND
chris.friesen@genband.com
www.genband.com

^ permalink raw reply

* Re: [RFC 5/5] [powerpc] Implement a p1010rdb clock source.
From: Robin Holt @ 2011-08-08 20:27 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: Robin Holt, socketcan-core, netdev, U Bhaskar-B22300,
	Marc Kleine-Budde
In-Reply-To: <20110808191403.GG4926@sgi.com>

On Mon, Aug 08, 2011 at 02:14:03PM -0500, Robin Holt wrote:
> On Mon, Aug 08, 2011 at 08:37:37PM +0200, Wolfgang Grandegger wrote:
> > On 08/08/2011 06:08 PM, Robin Holt wrote:
> > > On Mon, Aug 08, 2011 at 06:03:06PM +0200, Wolfgang Grandegger wrote:
> > ...
> > > So we would stay with the clk_* functions.  I assume clk_get() would
> > > return NULL, clk_get_rate() would just return fsl_get_sys_freq() and
> > > the other functions would do nothing.  Doesn't this really polute what
> > > clk_* functions are supposed to do?  Aren't we making flexcan dictate
> > > a different behavior for powerpc than for the arm (and possibly other)
> > > architectures?
> > 
> > Well, I see it as one way to provide compatibility with the ARM port. If
> > the PowerPC people don't like it, we can switch to something else,
> > whatever they suggest.
> 
> I have spent the last few hours and I think I found the communication
> problem and I think it is me.
> 
> I assumed long ago we would be better off implementing a Kconfig language
> which does "select PPC_CLOCK".  This assumption was in part because I
> did not understand what I was doing when I started this (still don't
> honestly), but I did know the freescale patch series did a select
> PPC_CLOCK.
> 
> Here is my patch for introducing the p1010 clock source.  Am I finally
> starting to understand your guidance?
> 
> Thanks,
> Robin
> 

Here is _ALL_ of the patch.  Sorry about the earlier noise.

Robin

------------------------------------------------------------------------


diff --git a/arch/powerpc/platforms/85xx/Kconfig b/arch/powerpc/platforms/85xx/Kconfig
index 498534c..ed4cf92 100644
--- a/arch/powerpc/platforms/85xx/Kconfig
+++ b/arch/powerpc/platforms/85xx/Kconfig
@@ -26,6 +26,10 @@ config MPC8560_ADS
 	help
 	  This option enables support for the MPC 8560 ADS board
 
+config 85xx_HAVE_CAN_FLEXCAN
+	bool
+	select HAVE_CAN_FLEXCAN if NET && CAN
+
 config MPC85xx_CDS
 	bool "Freescale MPC85xx CDS"
 	select DEFAULT_UIMAGE
@@ -70,6 +74,8 @@ config MPC85xx_RDB
 config P1010_RDB
 	bool "Freescale P1010RDB"
 	select DEFAULT_UIMAGE
+	select 85xx_HAVE_CAN_FLEXCAN
+	select PPC_CLOCK if CAN_FLEXCAN
 	help
 	  This option enables support for the MPC85xx RDB (P1010 RDB) board
 
diff --git a/arch/powerpc/platforms/85xx/Makefile b/arch/powerpc/platforms/85xx/Makefile
index a971b32..64ad7a4 100644
--- a/arch/powerpc/platforms/85xx/Makefile
+++ b/arch/powerpc/platforms/85xx/Makefile
@@ -11,6 +11,7 @@ obj-$(CONFIG_MPC85xx_DS)  += mpc85xx_ds.o
 obj-$(CONFIG_MPC85xx_MDS) += mpc85xx_mds.o
 obj-$(CONFIG_MPC85xx_RDB) += mpc85xx_rdb.o
 obj-$(CONFIG_P1010_RDB)   += p1010rdb.o
+obj-$(CONFIG_PPC_CLOCK)   += clock.o
 obj-$(CONFIG_P1022_DS)    += p1022_ds.o
 obj-$(CONFIG_P1023_RDS)   += p1023_rds.o
 obj-$(CONFIG_P2040_RDB)   += p2040_rdb.o corenet_ds.o
diff --git a/arch/powerpc/platforms/85xx/clock.c b/arch/powerpc/platforms/85xx/clock.c
new file mode 100644
index 0000000..a25cbf3
--- /dev/null
+++ b/arch/powerpc/platforms/85xx/clock.c
@@ -0,0 +1,59 @@
+
+#include <linux/device.h>
+#include <linux/err.h>
+
+#include <asm/clk_interface.h>
+
+#include <sysdev/fsl_soc.h>
+
+/*
+ * p1010rdb needs to provide a clock source for the flexcan driver.
+ */
+struct clk {
+	unsigned long rate;
+} p1010_rdb_system_clock;
+
+static struct clk *p1010_rdb_clk_get(struct device *dev, const char *id)
+{
+	const char *dev_init_name;
+
+	if (!dev)
+		return ERR_PTR(-ENOENT);
+
+	/*
+	 * The can devices are named ffe1c000.can0 and ffe1d000.can1 on
+	 * the p1010rdb.  Check for the "can" portion of that name before
+	 * returning a clock source.
+	 */
+	dev_init_name = dev_name(dev);
+	if (strlen(dev_init_name) != 13)
+		return ERR_PTR(-ENOENT);
+	dev_init_name += 9;
+	if (strncmp(dev_init_name, "can", 3))
+		return ERR_PTR(-ENOENT);
+
+	return &p1010_rdb_system_clock;
+}
+
+static void p1010_rdb_clk_put(struct clk *clk)
+{
+	return;
+}
+
+static unsigned long p1010_rdb_clk_get_rate(struct clk *clk)
+{
+	return clk->rate;
+}
+
+static struct clk_interface p1010_rdb_clk_functions = {
+	.clk_get		= p1010_rdb_clk_get,
+	.clk_get_rate		= p1010_rdb_clk_get_rate,
+	.clk_put		= p1010_rdb_clk_put,
+};
+
+void __init p1010_rdb_clk_init(void)
+{
+	p1010_rdb_system_clock.rate = fsl_get_sys_freq();
+	clk_functions = p1010_rdb_clk_functions;
+}
+
diff --git a/arch/powerpc/platforms/85xx/p1010rdb.c b/arch/powerpc/platforms/85xx/p1010rdb.c
index d7387fa..29e04d6 100644
--- a/arch/powerpc/platforms/85xx/p1010rdb.c
+++ b/arch/powerpc/platforms/85xx/p1010rdb.c
@@ -81,6 +81,13 @@ static void __init p1010_rdb_setup_arch(void)
 	printk(KERN_INFO "P1010 RDB board from Freescale Semiconductor\n");
 }
 
+static void __init p1010_rdb_init(void)
+{
+#ifdef PPC_CLOCK
+	p1010_rdb_clk_init();
+#endif
+}
+
 static struct of_device_id __initdata p1010rdb_ids[] = {
 	{ .type = "soc", },
 	{ .compatible = "soc", },
@@ -111,6 +118,7 @@ define_machine(p1010_rdb) {
 	.name			= "P1010 RDB",
 	.probe			= p1010_rdb_probe,
 	.setup_arch		= p1010_rdb_setup_arch,
+	.init			= p1010_rdb_init,
 	.init_IRQ		= p1010_rdb_pic_init,
 #ifdef CONFIG_PCI
 	.pcibios_fixup_bus	= fsl_pcibios_fixup_bus,
-- 
1.7.2.1

^ permalink raw reply related


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