Netdev List
 help / color / mirror / Atom feed
* Re: [patch 1/3] drivers/net/cxgb3/t3_hw.c: use new hex_to_bin() method
From: David Miller @ 2010-07-21 18:17 UTC (permalink / raw)
  To: akpm; +Cc: netdev, ext-andriy.shevchenko, divy
In-Reply-To: <201007202225.o6KMPGnA021443@imap1.linux-foundation.org>

From: akpm@linux-foundation.org
Date: Tue, 20 Jul 2010 15:25:16 -0700

> From: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
> 
> Get rid of own implementation of hex_to_bin().
> 
> Signed-off-by: Andy Shevchenko <ext-andriy.shevchenko@nokia.com>
> Acked-by: Divy Le Ray <divy@chelsio.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

Applied.

^ permalink raw reply

* Re: [patch 2/3] arch/um/drivers: remove duplicate structure field initialization
From: David Miller @ 2010-07-21 18:17 UTC (permalink / raw)
  To: akpm; +Cc: netdev, julia, shemminger
In-Reply-To: <201007202225.o6KMPHA3021446@imap1.linux-foundation.org>

From: akpm@linux-foundation.org
Date: Tue, 20 Jul 2010 15:25:17 -0700

> From: Julia Lawall <julia@diku.dk>
> 
> There are two initializations of ndo_set_mac_address, one to a local
> function that is not used otherwise and one to a function that is defined
> elsewhere.
> 
> The semantic match that finds this problem is as follows:
> (http://coccinelle.lip6.fr/)
 ...
> 
> akpm:
> 
> - Use the standard eth_mac_addr() in uml_net_set_mac()
> 
> - Remove unneeded and racy local set_ether_mac()
> 
> - Remove duplicated (and incorrect)
>   uml_netdev_ops.ndo_set_mac_address initializer.
> 
> Fixes 8bb95b39a16ed55226810596f92216c53329d2fe ("uml: convert network
> device to netdevice ops").
> 
> [akpm@linux-foundation.org: rework as above]
> Signed-off-by: Julia Lawall <julia@diku.dk>
> Cc: Stephen Hemminger <shemminger@vyatta.com>
> Cc: "David S. Miller" <davem@davemloft.net>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

Applied.

^ permalink raw reply

* Re: [patch 3/3] drivers/net/82596.c: fix warning
From: David Miller @ 2010-07-21 18:18 UTC (permalink / raw)
  To: akpm; +Cc: netdev, segooon
In-Reply-To: <201007202225.o6KMPIQd021449@imap1.linux-foundation.org>

From: akpm@linux-foundation.org
Date: Tue, 20 Jul 2010 15:25:18 -0700

> From: Andrew Morton <akpm@linux-foundation.org>
> 
> drivers/net/82596.c: In function 'i596_open':
> drivers/net/82596.c:1044: warning: label 'err_irq_dev' defined but not used
> 
> Caused by "82596: free resources on error"
> 
> Cc: Kulikov Vasiliy <segooon@gmail.com>
> Cc: David S. Miller <davem@davemloft.net>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

When this hit the -mm tree, I replied to the -mm postinging that I applied
it to net-next-2.6

So it's integrated already :-)

^ permalink raw reply

* Re: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: Casey Leedom @ 2010-07-21 18:29 UTC (permalink / raw)
  To: David Miller
  Cc: shemminger, andy, harald, bhutchings, sassmann, netdev,
	linux-kernel, gospo, gregory.v.rose, alexander.h.duyck
In-Reply-To: <20100721.103249.107094774.davem@davemloft.net>

| From: David Miller <davem@davemloft.net>
| Date: Wednesday, July 21, 2010 10:32 am
| 
| From: Stephen Hemminger <shemminger@vyatta.com>
| Date: Wed, 21 Jul 2010 10:28:16 -0700
| 
| > IMHO no local assigned address should be used by udev. The cxgb4 driver
| > should be using random value.
| > 
| > Does anyone have an example of locally assigned address that has
| > persistence so that udev could use it.
| 
| The cxgb4 vf addresses are not random because they are fetched from the
| card's NVRAM/EEPROM/firmware/whatever and thus are persistent.
| 
| We definitely want udev to use persistent rules for them.
|
| This whole issue only exists because of the Intel VF case, where it
| lacks persistent addresses but somehow we want to assign persistent
| names to it's VF interfaces.

  Yes, we _explicitly_ wanted to have persistent MAC Addresses for our PCI-E SR-
IOV Virtual Functions for a whole raft of reasons.  The two most important were:

 1. Linux' model for persistent device naming today seems to be
    oriented around persistent network device addresses.

 2. Lots of data centers use MAC addresses for things like DHCP/BOOTP,
    security/filtering, etc.

Our design goal was to look as much like a normal Ethernet MAC as possible in 
order to reduce the need for software/behavior changes.

| One idea I've proposed in other discussions about this is that if the
| address is not persistent (either via the MAC address bit or the sysfs
| value we're thinking of providing here) we use the device's geographic
| location ("device path") as the key for udev stuff.

  Another option might be to have a new Net Device Operations call to ask the 
adapter for a Unique Key.  This could be formed for most devices via a tuple of 
the {PCI Vendor ID, PCI Device ID, Adapter Serial Number, Port Number, [and if 
applicable] Adapter Function ID}.  Of course this could be a fairly long string 
... :-)

Casey

^ permalink raw reply

* Re: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: David Miller @ 2010-07-21 18:39 UTC (permalink / raw)
  To: leedom
  Cc: shemminger, andy, harald, bhutchings, sassmann, netdev,
	linux-kernel, gospo, gregory.v.rose, alexander.h.duyck
In-Reply-To: <201007211129.48288.leedom@chelsio.com>

From: Casey Leedom <leedom@chelsio.com>
Date: Wed, 21 Jul 2010 11:29:47 -0700

>   Another option might be to have a new Net Device Operations call to ask the 
> adapter for a Unique Key.  This could be formed for most devices via a tuple of 
> the {PCI Vendor ID, PCI Device ID, Adapter Serial Number, Port Number, [and if 
> applicable] Adapter Function ID}.  Of course this could be a fairly long string 
> ... :-)

If a unique key were available, it could be used to generate a persistent
MAC address.

And this sort of means that these drivers could use bits of the
device's geographic ID the construct persistent MAC addresses, but
only if done in a MAC namespace that could be guarenteed unique on the
local system.

^ permalink raw reply

* RE: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: Rose, Gregory V @ 2010-07-21 18:43 UTC (permalink / raw)
  To: Casey Leedom, David Miller
  Cc: shemminger@vyatta.com, andy@greyhouse.net, harald@redhat.com,
	bhutchings@solarflare.com, sassmann@redhat.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	gospo@redhat.com, Duyck, Alexander H
In-Reply-To: <201007211129.48288.leedom@chelsio.com>

>-----Original Message-----
>From: Casey Leedom [mailto:leedom@chelsio.com]
>Sent: Wednesday, July 21, 2010 11:30 AM
>To: David Miller
>Cc: shemminger@vyatta.com; andy@greyhouse.net; harald@redhat.com;
>bhutchings@solarflare.com; sassmann@redhat.com; netdev@vger.kernel.org;
>linux-kernel@vger.kernel.org; gospo@redhat.com; Rose, Gregory V; Duyck,
>Alexander H
>Subject: Re: [PATCH net-next] sysfs: add entry to indicate network
>interfaces with random MAC address
>
>| From: David Miller <davem@davemloft.net>
>| Date: Wednesday, July 21, 2010 10:32 am
>|
>| From: Stephen Hemminger <shemminger@vyatta.com>
>| Date: Wed, 21 Jul 2010 10:28:16 -0700
>|
>| > IMHO no local assigned address should be used by udev. The cxgb4
>driver
>| > should be using random value.
>| >
>| > Does anyone have an example of locally assigned address that has
>| > persistence so that udev could use it.
>|
>| The cxgb4 vf addresses are not random because they are fetched from
>the
>| card's NVRAM/EEPROM/firmware/whatever and thus are persistent.
>|
>| We definitely want udev to use persistent rules for them.
>|
>| This whole issue only exists because of the Intel VF case, where it
>| lacks persistent addresses but somehow we want to assign persistent
>| names to it's VF interfaces.
>
>  Yes, we _explicitly_ wanted to have persistent MAC Addresses for our
>PCI-E SR-
>IOV Virtual Functions for a whole raft of reasons.  The two most
>important were:
>
> 1. Linux' model for persistent device naming today seems to be
>    oriented around persistent network device addresses.
>
> 2. Lots of data centers use MAC addresses for things like DHCP/BOOTP,
>    security/filtering, etc.
>
>Our design goal was to look as much like a normal Ethernet MAC as
>possible in
>order to reduce the need for software/behavior changes.

I'm curious, what happens when the VM using the VF migrates to a new machine and has another VF assigned to with a different MAC address?

Intel's view of things is that we don't use persistent MAC addresses in our VFs because the MAC address belongs to the VM and when it migrates it's going to want to use another VF with the same MAC address.  If they're persistent I'm wondering how that can be done.

This discussion has come about because some folks want to use the VF in the Host VMM.  The original design goal for Intel was that VFs would be assigned to VMs and that VMM vendors would want to assign MAC addresses with their own assigned OUI's.

- Greg

^ permalink raw reply

* Re: [PATCH] LSM: Add post recvmsg() hook.
From: David Miller @ 2010-07-21 18:45 UTC (permalink / raw)
  To: penguin-kernel
  Cc: kuznet, pekkas, jmorris, yoshfuji, kaber, paul.moore, netdev,
	linux-security-module
In-Reply-To: <201007171017.DFC73498.SFFFOMLVJOHOtQ@I-love.SAKURA.ne.jp>

From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
Date: Sat, 17 Jul 2010 10:17:10 +0900

> NETWORKING [IPv4/IPv6] maintainers and Paul, is below patch fine for you?

Unfortunately, after further consideration, I must reject this patch
and also the post accept() LSM hook one.

Sorry.

I looked into history of the discussions on this issue, and I have found
that the core issue with these hooks has not been addressed.

We must ensure that if:

1) Application makes poll() on UDP socket in blocking mode, and UDP
   reports that receive data is available

and

2) Application, after such a poll() call, makes a blocking recvmsg() call
   and no other activity has occurred on the socket meanwhile

Then we MUST return immediately with that available data.

This LSM hook, when it triggers, can violate this rule, even if you do
this looping thing.

The post accept() hook has the same problems.

Here is where we originally discussed this, in detail:

http://www.spinics.net/lists/netdev/msg95660.html

Therefore, I think this shows that what Tomoyo is trying to do is
fatally flawed.  We brought this fundamental issue up to you about a
year ago, and the issue is still not addressed.

So consider very seriously, that what you are trying to do cannot be
performed without breaking applications and API behavioral
expectations.

^ permalink raw reply

* Re: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: David Miller @ 2010-07-21 18:48 UTC (permalink / raw)
  To: gregory.v.rose
  Cc: leedom, shemminger, andy, harald, bhutchings, sassmann, netdev,
	linux-kernel, gospo, alexander.h.duyck
In-Reply-To: <43F901BD926A4E43B106BF17856F0755F184620A@orsmsx508.amr.corp.intel.com>

From: "Rose, Gregory V" <gregory.v.rose@intel.com>
Date: Wed, 21 Jul 2010 11:43:19 -0700

> Intel's view of things is that we don't use persistent MAC addresses
> in our VFs because the MAC address belongs to the VM and when it
> migrates it's going to want to use another VF with the same MAC
> address.  If they're persistent I'm wondering how that can be done.
> 
> This discussion has come about because some folks want to use the VF
> in the Host VMM.  The original design goal for Intel was that VFs
> would be assigned to VMs and that VMM vendors would want to assign
> MAC addresses with their own assigned OUI's.

If the VM itself is the "physical entity" of the system, the logical
conclusion I come to is that some kind of key should be obtained
through the VM to uniquely give the device a persistent MAC.

You could do things like have the PF controller use the root filesystem
ID label to construct the VF's MAC address, or something like that.

^ permalink raw reply

* Re: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: David Miller @ 2010-07-21 18:50 UTC (permalink / raw)
  To: gregory.v.rose
  Cc: leedom, shemminger, andy, harald, bhutchings, sassmann, netdev,
	linux-kernel, gospo, alexander.h.duyck
In-Reply-To: <20100721.114851.200597269.davem@davemloft.net>

From: David Miller <davem@davemloft.net>
Date: Wed, 21 Jul 2010 11:48:51 -0700 (PDT)

> You could do things like have the PF controller use the root filesystem
> ID label to construct the VF's MAC address, or something like that.

And here I of course mean the root filesystem of the guest the VF will
be given to.

^ permalink raw reply

* [patch] usbnet: fix 100% CPU use on suspended device
From: Elly Jones @ 2010-07-21 18:51 UTC (permalink / raw)
  To: netdev

Subject: [patch] usbnet: fix 100% CPU use on suspended device
From: Elly Jones <ellyjones@google.com>

This patch causes the usbnet module not to attempt to submit URBs to the device
if the device is not ready to accept them. This fixes a misbehavior trigged by
the Qualcomm Gobi driver (released under GPL through their Code Aurora
initiative) which causes the usbnet core to consume 100% of CPU attempting and
failing to submit URBs. This patch is against Linus's 2.6 repo commit
a9f7f2e74ae0e6a801a2433dc8e9124d73da0cb4.
Signed-off-by: Elizabeth Jones <ellyjones@google.com>
---
diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 81c76ad..df7e72e 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -1172,6 +1172,7 @@ static void usbnet_bh (unsigned long param)
 	// or are we maybe short a few urbs?
 	} else if (netif_running (dev->net) &&
 		   netif_device_present (dev->net) &&
+		   dev->udev->can_submit &&
 		   !timer_pending (&dev->delay) &&
 		   !test_bit (EVENT_RX_HALT, &dev->flags)) {
 		int	temp = dev->rxq.qlen;

^ permalink raw reply related

* RE: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: Rose, Gregory V @ 2010-07-21 19:02 UTC (permalink / raw)
  To: David Miller
  Cc: leedom@chelsio.com, shemminger@vyatta.com, andy@greyhouse.net,
	harald@redhat.com, bhutchings@solarflare.com, sassmann@redhat.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	gospo@redhat.com, Duyck, Alexander H
In-Reply-To: <20100721.115023.69942880.davem@davemloft.net>

>-----Original Message-----
>From: David Miller [mailto:davem@davemloft.net]
>Sent: Wednesday, July 21, 2010 11:50 AM
>To: Rose, Gregory V
>Cc: leedom@chelsio.com; shemminger@vyatta.com; andy@greyhouse.net;
>harald@redhat.com; bhutchings@solarflare.com; sassmann@redhat.com;
>netdev@vger.kernel.org; linux-kernel@vger.kernel.org; gospo@redhat.com;
>Duyck, Alexander H
>Subject: Re: [PATCH net-next] sysfs: add entry to indicate network
>interfaces with random MAC address
>
>From: David Miller <davem@davemloft.net>
>Date: Wed, 21 Jul 2010 11:48:51 -0700 (PDT)
>
>> You could do things like have the PF controller use the root
>filesystem
>> ID label to construct the VF's MAC address, or something like that.
>
>And here I of course mean the root filesystem of the guest the VF will
>be given to.

I suppose you could do that but then the VM is going to have to be allowed to set its own MAC address.  There is a lot of opposition and concern about allowing VMs to set their own MAC address.

Then there's also support in the kernel now for assigning VF MAC and VLAN via the iproute2 package "ip link" commands.  The administrative SW that controls VMs should be assigning MAC addresses to VFs as needed.  When the VF is de-assigned from the guest because the VM is migrating then the administrative SW can assign another VF to the VM at the migration target machine and assign the same MAC address then.  This way it is all done administratively from the (supposedly) secure host VMM domain and we're not allowing VMs to set their own MAC address.

Ultimately I think I agree that some sort of approach such as you and others have been suggesting should be taken.  I'm agnostic about which one because my view of how things should work is a bit different.  But if the community thinks that finding some way to set a persistent MAC address for a VF is useful then I'd more than happy to make sure our drivers support that also.

As soon as it is decided what that is of course.

;-)

- Greg

^ permalink raw reply

* Re: [PATCH 0/2] Support untagged symlinks to tagged directories.
From: Greg KH @ 2010-07-21 19:02 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Greg KH, Andrew Morton, Rafael J. Wysocki, Maciej W. Rozycki,
	Kay Sievers, Johannes Berg, netdev
In-Reply-To: <m139vd5sms.fsf_-_@fess.ebiederm.org>

On Tue, Jul 20, 2010 at 10:08:27PM -0700, Eric W. Biederman wrote:
> 
> Greg KH <gregkh@suse.de> writes:
> 
> > On Mon, Jul 19, 2010 at 01:34:51PM -0700, Andrew Morton wrote:
> >> On Thu, 8 Jul 2010 16:06:01 -0700
> >> Greg KH <greg@kroah.com> wrote:
> >> 
> >> > On Thu, Jul 08, 2010 at 03:28:53PM -0700, Eric W. Biederman wrote:
> >> > > Greg KH <greg@kroah.com> writes:
> >> > > 
> >> > > > With this patch, how does the existing code fail as the drivers aren't
> >> > > > fixed up?
> >> > > >
> >> > > > I like this change, just worried it will cause problems if it gets into
> >> > > > .35, without your RFC patch.  Will it?
> >> > > 
> >> 
> >> geethanks!
> >> 
> >> On the FC6 test box I have no networking.
> >
> > Ick.
> >
> > Eric, any ideas?
> 
> Yes.  I just found some time to test my fixes and things are looking
> good.  It really is just two one line fixes.
> 
> On the other part of this debug with SYSFS_DEPRECATED enabled it
> with mac80211_hwsim drivers works fine no problems.  I expect the
> bnep driver will also be fine.
> 
> What is affecting those two is arguably a bug in the non-deprecated
> sysfs mode.
> 
> Regardless here are my fixes.  I have split this into a patch for
> the warning and a patch for sysfs_delete_link.  Because at least
> the sysfs_delete_link code needs to make into 2.6.35 if we can.

Do these patches obsolete the existing one I have in my tree entitled:
	Subject: sysfs: Don't allow the creation of symlinks we can't remove
or are they on top of that one?

thanks,

greg k-h

^ permalink raw reply

* Re: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: David Miller @ 2010-07-21 19:33 UTC (permalink / raw)
  To: gregory.v.rose
  Cc: leedom, shemminger, andy, harald, bhutchings, sassmann, netdev,
	linux-kernel, gospo, alexander.h.duyck
In-Reply-To: <43F901BD926A4E43B106BF17856F0755F1846247@orsmsx508.amr.corp.intel.com>

From: "Rose, Gregory V" <gregory.v.rose@intel.com>
Date: Wed, 21 Jul 2010 12:02:17 -0700

>>From: David Miller <davem@davemloft.net>
>>Date: Wed, 21 Jul 2010 11:48:51 -0700 (PDT)
>>
>>> You could do things like have the PF controller use the root
>>filesystem
>>> ID label to construct the VF's MAC address, or something like that.
>>
>>And here I of course mean the root filesystem of the guest the VF will
>>be given to.
> 
> I suppose you could do that but then the VM is going to have to be
> allowed to set its own MAC address.  There is a lot of opposition
> and concern about allowing VMs to set their own MAC address.

Why would that be necessary?  The host with the PF creating the guest
has access to the "device" and thus the root filesystem of the guest,
and thus could pull in the root filesystem "key" and instantiate the
VF's MAC before booting the guest.

That was the idea, the control node sets up the VF MAC before the guest
boots or can have access to the VF device.

This is completely agnostic of migration or anything like that.  The
procedure for setting the VF MAC is always the same.

^ permalink raw reply

* Re: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: Casey Leedom @ 2010-07-21 19:25 UTC (permalink / raw)
  To: David Miller
  Cc: shemminger, andy, harald, bhutchings, sassmann, netdev,
	linux-kernel, gospo, gregory.v.rose, alexander.h.duyck
In-Reply-To: <20100721.113906.102700682.davem@davemloft.net>

| From: David Miller <davem@davemloft.net>
| Date: Wednesday, July 21, 2010 11:39 am
| 
| From: Casey Leedom <leedom@chelsio.com>
| Date: Wed, 21 Jul 2010 11:29:47 -0700
| 
| >   Another option might be to have a new Net Device Operations call to ask
| > the adapter for a Unique Key.  This could be formed for most devices via a
| > tuple of the {PCI Vendor ID, PCI Device ID, Adapter Serial Number, Port
| > Number, [and if applicable] Adapter Function ID}.  Of course this could
| > be a fairly long string ... :-)
| 
| If a unique key were available, it could be used to generate a persistent
| MAC address.

  True but ... the Unique Key name space is probably a lot larger (in bits) than 
the MAC Address name space (~(48-2) bits) ... :-)

| And this sort of means that these drivers could use bits of the
| device's geographic ID the construct persistent MAC addresses, but
| only if done in a MAC namespace that could be guarenteed unique on the
| local system.

  Yep.  That's the problem of trying to construct a Unique Locally Assigned MAC 
Address from a Unique Name in a larger name space.

| From: "Rose, Gregory V" <gregory.v.rose@intel.com>
| Date: Wednesday, July 21, 2010 11:43 am
| 
| I'm curious, what happens when the VM using the VF migrates to a new
| machine and has another VF assigned to with a different MAC address?

  To be honest, I still haven't wrapped my head around how Virtual Machines are 
ever going to be able to migrate when they have arbitrary PCI Devices "assigned" 
(KVM Terminology) to them (AKA "PCI Pass Through").  Allowing VMs to directly 
touch real and arbitrary hardware devices means that some abstraction of "saving 
the device state" and "restoring the device state" can be successfully 
negotiated ... which would be hard even if you quiesce the device and you 
migrate to another Physical Host with identical PCI Hardware which is then 
"assigned" to the migrated VM.  Hard, hard, hard.

  This is why most of the Hypervisor systems have used synthetic Pseudo Devices 
to allow that state of those Virtual Devices to be migrated (including the MAC 
Addresses which we've been talking about).  I actually think that the Microsoft 
HyperV approach of Virtual Ingress Queues may be a better solution.  You still 
need to make the VM-to-Hypervisor transitions but you get to avoid the Free List 
memory copy costs which are actually the dominant cost in the RX path to VMs 
using software vNICs.

  But that's straying far from the topic at hand.  The short answer is pretty 
much what David suggests: the _hardware_ PCI-E SR-IOV Virtual Function provides 
persistent, non-random MAC Addresses for use by the VF Driver -- if it wants to 
use them.  A VF Driver running in a VM is capable of specifying arbitrary MAC 
Addresses for use with the VF and may ignore the hardware MAC Addresses provided 
by the VF.  This is little different from the current situation with software 
vNICs which use manufactured MAC Addresses (which are persistent in all of the 
Hypervisor systems at which I've looked).

| From: David Miller <davem@davemloft.net>
| Date: Wednesday, July 21, 2010 11:48 am
| 
| If the VM itself is the "physical entity" of the system, the logical
| conclusion I come to is that some kind of key should be obtained
| through the VM to uniquely give the device a persistent MAC.

  Which is, as above, what all Hypervisor systems which I've looked at do.

| You could do things like have the PF controller use the root filesystem
| ID label to construct the VF's MAC address, or something like that.

  It's actually stored in the VM's meta-data.  When a VM migrates from one 
Physical Host to another all of the VM's transient and persistent state must be 
available to the new Physical Host.  Xen, for instance, has the concept of a 
Physical Host Pool where all of the Physical Hosts have common access to shared 
resources like Network Attached Storage, LAN/VLANs and the shared VM meta-data.

Casey

^ permalink raw reply

* RE: [PATCH net-next] sysfs: add entry to indicate network interfaces with random MAC address
From: Rose, Gregory V @ 2010-07-21 19:35 UTC (permalink / raw)
  To: David Miller
  Cc: leedom@chelsio.com, shemminger@vyatta.com, andy@greyhouse.net,
	harald@redhat.com, bhutchings@solarflare.com, sassmann@redhat.com,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	gospo@redhat.com, Duyck, Alexander H
In-Reply-To: <20100721.123324.237334251.davem@davemloft.net>

>-----Original Message-----
>From: David Miller [mailto:davem@davemloft.net]
>Sent: Wednesday, July 21, 2010 12:33 PM
>To: Rose, Gregory V
>Cc: leedom@chelsio.com; shemminger@vyatta.com; andy@greyhouse.net;
>harald@redhat.com; bhutchings@solarflare.com; sassmann@redhat.com;
>netdev@vger.kernel.org; linux-kernel@vger.kernel.org; gospo@redhat.com;
>Duyck, Alexander H
>Subject: Re: [PATCH net-next] sysfs: add entry to indicate network
>interfaces with random MAC address
>
>From: "Rose, Gregory V" <gregory.v.rose@intel.com>
>Date: Wed, 21 Jul 2010 12:02:17 -0700
>
>>>From: David Miller <davem@davemloft.net>
>>>Date: Wed, 21 Jul 2010 11:48:51 -0700 (PDT)
>>>
>>>> You could do things like have the PF controller use the root
>>>filesystem
>>>> ID label to construct the VF's MAC address, or something like that.
>>>
>>>And here I of course mean the root filesystem of the guest the VF will
>>>be given to.
>>
>> I suppose you could do that but then the VM is going to have to be
>> allowed to set its own MAC address.  There is a lot of opposition
>> and concern about allowing VMs to set their own MAC address.
>
>Why would that be necessary?  The host with the PF creating the guest
>has access to the "device" and thus the root filesystem of the guest,
>and thus could pull in the root filesystem "key" and instantiate the
>VF's MAC before booting the guest.
>
>That was the idea, the control node sets up the VF MAC before the guest
>boots or can have access to the VF device.

I misunderstood you.  My bad.

Thank for the further explanation.

- Greg


^ permalink raw reply

* Re: [PATCH net-next-2.6] ixgbe: fix ethtool stats
From: Jeff Kirsher @ 2010-07-21 20:17 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, Jesse Brandeburg, PJ Waskiewicz, netdev
In-Reply-To: <1279679904.2492.20.camel@edumazet-laptop>

On Tue, Jul 20, 2010 at 19:38, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 20 juillet 2010 à 15:06 -0700, Jeff Kirsher a écrit :
>> On Tue, Jul 20, 2010 at 10:28, Eric Dumazet <eric.dumazet@gmail.com> wrote:
>> > Note : I am currently unable to test following patch, could you please
>> > Intel guys test it and Ack (or Nack) it ?
>> >
>> > Thanks !
>> >
>> > [PATCH net-next-2.6] ixgbe: fix ethtool stats
>> >
>> > In latest changes about 64bit stats on 32bit arches,
>> > [commit 28172739f0a276eb8 (net: fix 64 bit counters on 32 bit arches)],
>> > I missed ixgbe uses a bit of magic in its ixgbe_gstrings_stats
>> > definition.
>> >
>> > IXGBE_NETDEV_STAT() must now assume offsets relative to
>> > rtnl_link_stats64, not relative do dev->stats.
>> >
>> > As a bonus, we also get 64bit stats on ethtool -S
>> >
>> > Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>> > ---
>> >  drivers/net/ixgbe/ixgbe_ethtool.c |   42 ++++++++++++++--------------
>> >  1 file changed, 21 insertions(+), 21 deletions(-)
>> >
>>
>> Thanks Eric, I have added it to my queue.
>>
>
> Thanks !
>
> By the way, my ixgbe conf doesnt like net-next-2.6 at all.
> (No link is established in my fiber loop configuration)
>
> current linux-2.6 git runs correctly, link at 10Gb, so there is a
> regression somewhere.
>
> As this machine is quite slow (I dont have anymore my Nehalem dev
> machine, had to use an old setup), a bisection would take one month...
>
>

Eric - can you send your setup and .config so that we can take a look
at why your not getting a link?

-- 
Cheers,
Jeff

^ permalink raw reply

* Re: [PATCH 0/2] Support untagged symlinks to tagged directories.
From: Eric W. Biederman @ 2010-07-21 20:20 UTC (permalink / raw)
  To: Greg KH
  Cc: Greg KH, Andrew Morton, Rafael J. Wysocki, Maciej W. Rozycki,
	Kay Sievers, Johannes Berg, netdev
In-Reply-To: <20100721190253.GA25494@kroah.com>

Greg KH <greg@kroah.com> writes:

> On Tue, Jul 20, 2010 at 10:08:27PM -0700, Eric W. Biederman wrote:
>> 
>> Greg KH <gregkh@suse.de> writes:
>> 
>> > On Mon, Jul 19, 2010 at 01:34:51PM -0700, Andrew Morton wrote:
>> >> On Thu, 8 Jul 2010 16:06:01 -0700
>> >> Greg KH <greg@kroah.com> wrote:
>> >> 
>> >> > On Thu, Jul 08, 2010 at 03:28:53PM -0700, Eric W. Biederman wrote:
>> >> > > Greg KH <greg@kroah.com> writes:
>> >> > > 
>> >> > > > With this patch, how does the existing code fail as the drivers aren't
>> >> > > > fixed up?
>> >> > > >
>> >> > > > I like this change, just worried it will cause problems if it gets into
>> >> > > > .35, without your RFC patch.  Will it?
>> >> > > 
>> >> 
>> >> geethanks!
>> >> 
>> >> On the FC6 test box I have no networking.
>> >
>> > Ick.
>> >
>> > Eric, any ideas?
>> 
>> Yes.  I just found some time to test my fixes and things are looking
>> good.  It really is just two one line fixes.
>> 
>> On the other part of this debug with SYSFS_DEPRECATED enabled it
>> with mac80211_hwsim drivers works fine no problems.  I expect the
>> bnep driver will also be fine.
>> 
>> What is affecting those two is arguably a bug in the non-deprecated
>> sysfs mode.
>> 
>> Regardless here are my fixes.  I have split this into a patch for
>> the warning and a patch for sysfs_delete_link.  Because at least
>> the sysfs_delete_link code needs to make into 2.6.35 if we can.
>
> Do these patches obsolete the existing one I have in my tree entitled:
> 	Subject: sysfs: Don't allow the creation of symlinks we can't remove
> or are they on top of that one?

No. These patches are incremental on top of what you have.

In particular patch 1 fixes what is in 2.6.35-rc5, and patch 2
fixes the additional check in:
'sysfs: Don't allow the creation of symlinks we can't remove'

Eric

^ permalink raw reply

* RE: [PATCH net-2.6] ixgbe/igb: catch invalid VF settings
From: Rose, Gregory V @ 2010-07-21 20:23 UTC (permalink / raw)
  To: Chris Wright, Andy Gospodarek; +Cc: netdev@vger.kernel.org
In-Reply-To: <20100721165055.GE13188@sequoia.sous-sol.org>

>-----Original Message-----
>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
>On Behalf Of Chris Wright
>Sent: Wednesday, July 21, 2010 9:51 AM
>To: Andy Gospodarek
>Cc: netdev@vger.kernel.org; chrisw@sous-sol.org
>Subject: Re: [PATCH net-2.6] ixgbe/igb: catch invalid VF settings
>
>* Andy Gospodarek (andy@greyhouse.net) wrote:
>> Some ixgbe cards put an invalid VF device ID in the PCIe SR-IOV
>> capability.  The ixgbe driver is only valid for PFs or non SR-IOV
>> hardware.  It seems that the same problem could occur on igb hardware
>as
>> well, so if we discover we are trying to initialize a VF in
>ixbge_probe
>> or igb_probe, print an error and exit.
>>
>> Based on a patch for ixgbe from Chris Wright <chrisw@sous-sol.org>.
>>
>> Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
>> Cc: Chris Wright <chrisw@sous-sol.org>
>
>Acked-by: Chris Wright <chrisw@sous-sol.org>
>
>Only seen this on ixgbe, but since the result there is a kernel panic
>makes sense to be defensive.
>
>thanks,
>-chris

Acked-by: Greg Rose <gregory.v.rose@intel.com>

Looks good, thanks.

- Greg


^ permalink raw reply

* Re: [PATCH] Add missing read memory barrier to Intel Ethernet device drivers
From: Jeff Kirsher @ 2010-07-21 20:28 UTC (permalink / raw)
  To: Sonny Rao; +Cc: netdev, e1000-devel
In-Reply-To: <20100721162241.GW21853@us.ibm.com>

On Wed, Jul 21, 2010 at 09:22, Sonny Rao <sonnyrao@us.ibm.com> wrote:
> From: Milton Miller <miltonm@bga.com>
>
> The PowerPC architecture does not require loads to independent bytes to be
> ordered without adding an explicit barrier.
>
> In ixgbe_clean_rx_irq we load the status bit then load the packet data.
> With packet split disabled if these loads go out of order we get a
> stale packet, but we will notice the bad sequence numbers and drop it.
>
> The problem occurs with packet split enabled where the TCP/IP header and data
> are in different descriptors. If the reads go out of order we may have data
> that doesn't match the TCP/IP header. Since we use hardware checksumming this
> bad data is never verified and it makes it all the way to the application.
>
> This bug was found during stress testing and adding this barrier has been shown
> to fix it.  The bug can manifest as a data integrity issue (bad payload data)
> or as a BUG in skb_pull().
>
> This was a nasty bug to hunt down, if people agree with the fix I think
> it's a candidate for stable.
>
> Previously Submitted to e1000-devel only for ixgbe
>
> http://marc.info/?l=e1000-devel&m=126593062701537&w=3
>
> We've now seen this problem hit with other device drivers (e1000e mostly)
> So I'm resubmitting with fixes for other Intel Device Drivers with
> similar issues:  ixgb, e100, e1000, and e1000e
>
> Signed-off-by: Milton Miller <miltonm@bga.com>
> Signed-off-by: Anton Blanchard <anton@samba.org>
> Signed-off-by: Sonny Rao <sonnyrao@us.ibm.com>
> cc: stable <stable@kernel.org>
>

Thanks Milton, I have added the patch to my queue.

-- 
Cheers,
Jeff

^ permalink raw reply

* Re: [Uclinux-dist-devel] [PATCH 2/2] net: dsa: introduce MICREL KSZ8893MQL/BL ethernet switch chip support
From: Karl Beldan @ 2010-07-21 20:31 UTC (permalink / raw)
  To: Mike Frysinger
  Cc: Lennert Buytenhek, netdev, uclinux-dist-devel, David S. Miller
In-Reply-To: <AANLkTinoVsz1YsWc3VSAqPPgH_L19B541A72zkWPxjM8@mail.gmail.com>

On Wed, Jul 21, 2010 at 6:05 PM, Mike Frysinger <vapier.adi@gmail.com> wrote:
> On Wed, Jul 21, 2010 at 11:16, Lennert Buytenhek wrote:
>> On Wed, Jul 21, 2010 at 09:37:22AM -0400, Mike Frysinger wrote:
>>> +static int ksz8893m_setup(struct dsa_switch *ds)
>>> +{
>>> +     int i;
>>> +     int ret;
>>> +
>>> +     ret = ksz8893m_switch_reset(ds);
>>> +     if (ret < 0)
>>> +             return ret;
>>
>> It's pretty ugly that the mdiobus is passed in via the normal means,
>> but a reference to the SPI bus to use is just stuffed into some global
>> variable.
>>
>> Can you not access all registers via MII?
>
> it depends on the host mdio bus.  if it supports the semi-standard
> behavior of toggling the OP field of MDIO frames, then yes, you can do
> it via MII.  but i dont think the current mdio framework in the kernel
> keeps track of that functionality, so there isnt a way in the driver
> to say "is this possible, else fall back to SPI".
>
Are you referring to SMI ?

> certainly the part that was used to develop this driver does not
> support this behavior thus SPI is the only way of accessing the
> extended registers.  i guess the driver could be extended so that
> people could pick which mode they want to program the registers via
> platform resources, but we have no way of testing that, so i say let
> the person who actually can use & wants that functionality implement
> it.
>
>> (If not, struct dsa_chip_data will need go be extended with another
>> struct device pointer that we can use to find the spi bus with.)
>
> if the framework supports, i can convert the driver to it, but i'm not
> sure we have the time atm to tackle reworking common frameworks.
>
If someone tackles this, it would be nice that they bear in mind that
phylib's drivers also need spi/smi/i2c register access.

>>> +static int ksz8893m_port_to_phy_addr(int port)
>>> +{
>>> +     if (port >= 1 && port <= KSZ8893M_PORT_NUM)
>>> +             return port;
>>> +
>>> +     pr_warning("use default phy addr 3\n");
>>> +     return 3;
>>
>> Does this ever happen?  You should just be able to return -1 here, IMHO.
>
> i dont recall seeing a warning, but presumably if it it did occur,
> something else needs fixing.  so -1 is OK.
>
I would remove this.

-- 
Karl

^ permalink raw reply

* Re: [PATCH net-2.6] ixgbe/igb: catch invalid VF settings
From: Jeff Kirsher @ 2010-07-21 20:31 UTC (permalink / raw)
  To: David Miller; +Cc: Chris Wright, Andy Gospodarek, netdev@vger.kernel.org
In-Reply-To: <43F901BD926A4E43B106BF17856F0755F18462F6@orsmsx508.amr.corp.intel.com>

On Wed, Jul 21, 2010 at 13:23, Rose, Gregory V <gregory.v.rose@intel.com> wrote:
>>-----Original Message-----
>>From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org]
>>On Behalf Of Chris Wright
>>Sent: Wednesday, July 21, 2010 9:51 AM
>>To: Andy Gospodarek
>>Cc: netdev@vger.kernel.org; chrisw@sous-sol.org
>>Subject: Re: [PATCH net-2.6] ixgbe/igb: catch invalid VF settings
>>
>>* Andy Gospodarek (andy@greyhouse.net) wrote:
>>> Some ixgbe cards put an invalid VF device ID in the PCIe SR-IOV
>>> capability.  The ixgbe driver is only valid for PFs or non SR-IOV
>>> hardware.  It seems that the same problem could occur on igb hardware
>>as
>>> well, so if we discover we are trying to initialize a VF in
>>ixbge_probe
>>> or igb_probe, print an error and exit.
>>>
>>> Based on a patch for ixgbe from Chris Wright <chrisw@sous-sol.org>.
>>>
>>> Signed-off-by: Andy Gospodarek <andy@greyhouse.net>
>>> Cc: Chris Wright <chrisw@sous-sol.org>
>>
>>Acked-by: Chris Wright <chrisw@sous-sol.org>
>>
>>Only seen this on ixgbe, but since the result there is a kernel panic
>>makes sense to be defensive.
>>
>>thanks,
>>-chris
>
> Acked-by: Greg Rose <gregory.v.rose@intel.com>
>
> Looks good, thanks.
>
> - Greg
>

Dave, I do not plan on adding this patch to my queue, so feel free to
take this patch after review.

-- 
Cheers,
Jeff

^ permalink raw reply

* Re: [PATCH 0/2] Support untagged symlinks to tagged directories.
From: Greg KH @ 2010-07-21 20:36 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Greg KH, Andrew Morton, Rafael J. Wysocki, Maciej W. Rozycki,
	Kay Sievers, Johannes Berg, netdev
In-Reply-To: <m18w544mer.fsf@fess.ebiederm.org>

On Wed, Jul 21, 2010 at 01:20:28PM -0700, Eric W. Biederman wrote:
> Greg KH <greg@kroah.com> writes:
> 
> > On Tue, Jul 20, 2010 at 10:08:27PM -0700, Eric W. Biederman wrote:
> >> 
> >> Greg KH <gregkh@suse.de> writes:
> >> 
> >> > On Mon, Jul 19, 2010 at 01:34:51PM -0700, Andrew Morton wrote:
> >> >> On Thu, 8 Jul 2010 16:06:01 -0700
> >> >> Greg KH <greg@kroah.com> wrote:
> >> >> 
> >> >> > On Thu, Jul 08, 2010 at 03:28:53PM -0700, Eric W. Biederman wrote:
> >> >> > > Greg KH <greg@kroah.com> writes:
> >> >> > > 
> >> >> > > > With this patch, how does the existing code fail as the drivers aren't
> >> >> > > > fixed up?
> >> >> > > >
> >> >> > > > I like this change, just worried it will cause problems if it gets into
> >> >> > > > .35, without your RFC patch.  Will it?
> >> >> > > 
> >> >> 
> >> >> geethanks!
> >> >> 
> >> >> On the FC6 test box I have no networking.
> >> >
> >> > Ick.
> >> >
> >> > Eric, any ideas?
> >> 
> >> Yes.  I just found some time to test my fixes and things are looking
> >> good.  It really is just two one line fixes.
> >> 
> >> On the other part of this debug with SYSFS_DEPRECATED enabled it
> >> with mac80211_hwsim drivers works fine no problems.  I expect the
> >> bnep driver will also be fine.
> >> 
> >> What is affecting those two is arguably a bug in the non-deprecated
> >> sysfs mode.
> >> 
> >> Regardless here are my fixes.  I have split this into a patch for
> >> the warning and a patch for sysfs_delete_link.  Because at least
> >> the sysfs_delete_link code needs to make into 2.6.35 if we can.
> >
> > Do these patches obsolete the existing one I have in my tree entitled:
> > 	Subject: sysfs: Don't allow the creation of symlinks we can't remove
> > or are they on top of that one?
> 
> No. These patches are incremental on top of what you have.
> 
> In particular patch 1 fixes what is in 2.6.35-rc5, and patch 2
> fixes the additional check in:
> 'sysfs: Don't allow the creation of symlinks we can't remove'

Ok, thanks, I've now added them to my tree and will send them to Linus
in a day or so.

greg k-h

^ permalink raw reply

* Re: [PATCH] e1000e: Drop a useless statement
From: Jeff Kirsher @ 2010-07-21 20:37 UTC (permalink / raw)
  To: Jean Delvare
  Cc: David S. Miller, netdev, Auke Kok, Bruce Allan, Jesse Brandeburg
In-Reply-To: <201007201730.42852.jdelvare@suse.de>

On Tue, Jul 20, 2010 at 08:30, Jean Delvare <jdelvare@suse.de> wrote:
> err is set again a few lines below.
>
> Signed-off-by: Jean Delvare <jdelvare@suse.de>
> Cc: Auke Kok <auke-jan.h.kok@intel.com>
> Cc: Bruce Allan <bruce.w.allan@intel.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---
> I sent this patch to the e1000-devel list on June 8th, 2010, but
> didn't receive any answer:
> http://sourceforge.net/mailarchive/forum.php?thread_name=201006081820.25381.jdelvare%40suse.de&forum_name=e1000-devel
>
>  drivers/net/e1000e/netdev.c |    2 --
>  1 file changed, 2 deletions(-)
>
> --- a/drivers/net/e1000e/netdev.c
> +++ b/drivers/net/e1000e/netdev.c
> @@ -5557,8 +5557,6 @@ static int __devinit e1000_probe(struct
>        if (err)
>                goto err_sw_init;
>
> -       err = -EIO;
> -
>        memcpy(&hw->mac.ops, ei->mac_ops, sizeof(hw->mac.ops));
>        memcpy(&hw->nvm.ops, ei->nvm_ops, sizeof(hw->nvm.ops));
>        memcpy(&hw->phy.ops, ei->phy_ops, sizeof(hw->phy.ops));
>
> --
> Jean Delvare
> Suse L3

Thanks Jean, sorry for the delayed response.  I have added the patch
to my queue.

-- 
Cheers,
Jeff

^ permalink raw reply

* Re: [PATCH] e1000e: Fix irq_synchronize in MSI-X case
From: Jeff Kirsher @ 2010-07-21 20:38 UTC (permalink / raw)
  To: Jean Delvare; +Cc: David S. Miller, netdev, Bruce Allan, Jesse Brandeburg
In-Reply-To: <201007201712.05671.jdelvare@suse.de>

On Tue, Jul 20, 2010 at 08:12, Jean Delvare <jdelvare@suse.de> wrote:
> Synchronize all IRQs when in MSI-X IRQ mode.
>
> Signed-off-by: Jean Delvare <jdelvare@suse.de>
> Cc: Bruce Allan <bruce.w.allan@intel.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> ---
> I sent this patch to the e1000-devel list on June 8th, 2010, but
> didn't receive any answer:
> http://sourceforge.net/mailarchive/forum.php?thread_name=201006081818.59098.jdelvare%40suse.de&forum_name=e1000-devel
>
> I don't know how critical synchronize_irq() is, so I don't know if
> this patch should go to stable branches or not.
>
>  drivers/net/e1000e/netdev.c |    8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> --- a/drivers/net/e1000e/netdev.c
> +++ b/drivers/net/e1000e/netdev.c
> @@ -1939,7 +1939,13 @@ static void e1000_irq_disable(struct e10
>        if (adapter->msix_entries)
>                ew32(EIAC_82574, 0);
>        e1e_flush();
> -       synchronize_irq(adapter->pdev->irq);
> +
> +       if (adapter->msix_entries) {
> +               synchronize_irq(adapter->msix_entries[0].vector);
> +               synchronize_irq(adapter->msix_entries[1].vector);
> +               synchronize_irq(adapter->msix_entries[2].vector);
> +       } else
> +               synchronize_irq(adapter->pdev->irq);
>  }
>
>  /**
>
> --
> Jean Delvare
> Suse L3

Thanks, I have added the patch to my queue.

-- 
Cheers,
Jeff

^ permalink raw reply

* Re: [PATCH] CAN: Add Flexcan CAN controller driver
From: Marc Kleine-Budde @ 2010-07-21 20:42 UTC (permalink / raw)
  To: Wolfgang Grandegger
  Cc: socketcan-core-0fE9KPoRgkgATYTw5x5z8w,
	netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <4C405CEC.3000701-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>


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

Marc Kleine-Budde wrote:
> Wolfgang Grandegger wrote:
>> I realized a few issues. You can add my "acked-by" when they are fixed.
> 
> thanks for the review.

[...]

>>> +static void flexcan_poll_err_frame(struct net_device *dev,
>>> +				   struct can_frame *cf, u32 reg_esr)
>>> +{
>>> +	struct flexcan_priv *priv = netdev_priv(dev);
>>> +	int error_warning = 0, rx_errors = 0, tx_errors = 0;
>>> +
>>> +	if (reg_esr & FLEXCAN_ESR_BIT1_ERR) {
>>> +		rx_errors = 1;
>>> +		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>> +		cf->data[2] |= CAN_ERR_PROT_BIT1;
>>> +	}
>>> +
>>> +	if (reg_esr & FLEXCAN_ESR_BIT0_ERR) {
>>> +		rx_errors = 1;
>>> +		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>> +		cf->data[2] |= CAN_ERR_PROT_BIT0;
>>> +	}
>>> +
>>> +	if (reg_esr & FLEXCAN_ESR_FRM_ERR) {
>>> +		rx_errors = 1;
>>> +		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>> +		cf->data[2] |= CAN_ERR_PROT_FORM;
>>> +	}
>>> +
>>> +	if (reg_esr & FLEXCAN_ESR_STF_ERR) {
>>> +		rx_errors = 1;
>>> +		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>>> +		cf->data[2] |= CAN_ERR_PROT_STUFF;
>>> +	}
>>> +
>>> +
>>> +	if (reg_esr & FLEXCAN_ESR_ACK_ERR) {
>>> +		tx_errors = 1;
>>> +		cf->can_id |= CAN_ERR_ACK;
>> This is a bus-error as well. Therefore I think it should be:
>>
>> 	if (reg_esr & FLEXCAN_ESR_ACK_ERR) {
>> 		tx_errors = 1;
>> 		cf->can_id |= CAN_ERR_ACK;
>> 		cf->can_id |= CAN_ERR_PROT | CAN_ERR_BUSERROR;
>> 		cf->data[3] |= CAN_ERR_PROT_LOC_ACK; /* ACK slot */
>> 	}
>>
>> I need to check what CAN_ERR_ACK is intended for. Then, cf->can_id could
>> be preset with "CAN_ERR_PROT | CAN_ERR_BUSERROR" at the beginning.

This controller issues an ACK error if there are no other nodes on the
CAN bus to send a ACK that the message has been received. Or all
remaining Nodes are in bus off state.

From the datasheet:
"This bit indicates that an acknowledge (ACK) error has been detected by
the transmitter node; that is, a dominant bit has not been detected
during the ACK SLOT."

>>
>>> +	}
>>> +
>>> +	if (error_warning)
>>> +		priv->can.can_stats.error_warning++;
>> Hm, error_warning is always 0 !?
> 
> this must go into the state handling function, will fix.
>>> +	if (rx_errors)
>>> +		dev->stats.rx_errors++;
>>> +	if (tx_errors)
>>> +		dev->stats.tx_errors++;
>>> +
>>> +}

[...]

>>> +static int flexcan_poll(struct napi_struct *napi, int quota)
>>> +{
>>> +	struct net_device *dev = napi->dev;
>>> +	const struct flexcan_priv *priv = netdev_priv(dev);
>>> +	struct flexcan_regs __iomem *regs = priv->base;
>>> +	u32 reg_iflag1, reg_esr;
>>> +	int work_done = 0;
>>> +
>>> +	reg_iflag1 = readl(&regs->iflag1);
>>> +
>>> +	/* first handle RX-FIFO */
>>> +	while (reg_iflag1 & FLEXCAN_IFLAG_RX_FIFO_AVAILABLE &&
>>> +	       work_done < quota) {
>>> +		flexcan_read_frame(dev);
>>> +
>>> +		work_done++;
>>> +		reg_iflag1 = readl(&regs->iflag1);
>>> +	}
>>> +
>>> +	/*
>>> +	 * The error bits are clear on read,
>>> +	 * so use saved value from irq handler.
>>> +	 */
>>> +	reg_esr = readl(&regs->esr) | priv->reg_esr;
>> Re-reading reg_esr may cause lost of state changes.
> 
> To my understanding of the datasheet and my observation, only the error
> bits are cleared on read. The bit defining the status
> (FLEXCAN_ESR_FLT_CONF_MASK) == error active, error passive and bus off
> are not cleared on read.
> 
> However there are two bits defining RX and TX warning level, I'll check
> these.

I just checked with real hardware , only the error bits are cleared on read.

>>> +	if (work_done < quota) {
>>> +		flexcan_poll_err(dev, reg_esr);
>> An error frame is created here for each call of flexcan_poll(), not only
>> in case of errors.
> 
> Doh, will fix this.
> 
>>> +		work_done++;
>>> +	}
>>> +
>>> +	if (work_done < quota) {
>>> +		napi_complete(napi);
>>> +		/* enable IRQs */
>>> +		writel(FLEXCAN_IFLAG_DEFAULT, &regs->imask1);
>>> +		writel(priv->reg_ctrl_default, &regs->ctrl);
>>> +	}
>>> +
>>> +	return work_done;
>>> +}

a reworked patch will follow soon.

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: 260 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


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