Netdev List
 help / color / mirror / Atom feed
* RE: [E1000-devel] [PATCH net-next 3/4] e1000e: advertise transmit time stamping
From: Allan, Bruce W @ 2012-07-23 16:07 UTC (permalink / raw)
  To: Richard Cochran, netdev@vger.kernel.org
  Cc: e1000-devel@lists.sourceforge.net, Willem de Bruijn, David Miller
In-Reply-To: <e0e31f0229b5bfda1684122498b4d6fb3195cf26.1342976654.git.richardcochran@gmail.com>

> -----Original Message-----
> From: Richard Cochran [mailto:richardcochran@gmail.com]
> Sent: Sunday, July 22, 2012 10:16 AM
> To: netdev@vger.kernel.org
> Cc: e1000-devel@lists.sourceforge.net; Willem de Bruijn; David Miller
> Subject: [E1000-devel] [PATCH net-next 3/4] e1000e: advertise transmit
> time stamping
> 
> This driver now offers software transmit time stamping, so it should
> advertise that fact via ethtool. Compile tested only.
> 
> Signed-off-by: Richard Cochran <richardcochran@gmail.com>
> 
> Cc: Willem de Bruijn <willemb@google.com>
> Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> Cc: e1000-devel@lists.sourceforge.net
> ---
>  drivers/net/ethernet/intel/e1000e/ethtool.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/ethtool.c
> b/drivers/net/ethernet/intel/e1000e/ethtool.c
> index 105d554..0349e24 100644
> --- a/drivers/net/ethernet/intel/e1000e/ethtool.c
> +++ b/drivers/net/ethernet/intel/e1000e/ethtool.c
> @@ -2061,6 +2061,7 @@ static const struct ethtool_ops e1000_ethtool_ops
> = {
>  	.get_coalesce		= e1000_get_coalesce,
>  	.set_coalesce		= e1000_set_coalesce,
>  	.get_rxnfc		= e1000_get_rxnfc,
> +	.get_ts_info		= ethtool_op_get_ts_info,
>  };
> 
>  void e1000e_set_ethtool_ops(struct net_device *netdev)

Thanks Richard.  I have a similar patch amongst the PTP 1588 work I'm currently doing
for e1000e, but this one is good for now.

Acked-by: Bruce Allan <bruce.w.allan@intel.com>

^ permalink raw reply

* RE: New commands to configure IOV features
From: Rose, Gregory V @ 2012-07-23 16:37 UTC (permalink / raw)
  To: Don Dutile, Chris Friesen
  Cc: Ben Hutchings, David Miller, yuvalmin@broadcom.com,
	netdev@vger.kernel.org, linux-pci@vger.kernel.org
In-Reply-To: <500D59BF.9040006@redhat.com>

> -----Original Message-----
> From: Don Dutile [mailto:ddutile@redhat.com]
> Sent: Monday, July 23, 2012 7:04 AM
> To: Chris Friesen
> Cc: Ben Hutchings; David Miller; yuvalmin@broadcom.com; Rose, Gregory V;
> netdev@vger.kernel.org; linux-pci@vger.kernel.org
> Subject: Re: New commands to configure IOV features
> 
> On 07/20/2012 07:42 PM, Chris Friesen wrote:
> > On 07/20/2012 02:01 PM, Ben Hutchings wrote:
> >> On Fri, 2012-07-20 at 13:29 -0600, Chris Friesen wrote:
> >
> >>> Once the device exists, then domain-specific APIs would be used to
> >>> configure it the same way that they would configure a physical device.
> >>
> >> To an extent, but not entirely.
> >>
> >> Currently, the assigned MAC address and (optional) VLAN tag for each
> >> networking VF are configured via the PF net device (though this is
> >> done though the rtnetlink API rather than ethtool).
> >
> > I actually have a use-case where the guest needs to be able to modify
> the MAC addresses of network devices that are actually VFs.
> >
> > The guest is bonding the network devices together, so the bonding driver
> in the guest expects to be able to set all the slaves to the same MAC
> address.
> >
> > As I read the ixgbe driver, this should be possible as long as the host
> hasn't explicitly set the MAC address of the VF. Is that correct?
> >
> > Chris
> 
> Interesting tug of war: hypervisors will want to set the macaddrs for
> security reasons,
>                          some guests may want to set macaddr for (valid?)
> config reasons.

It is a matter of trust.  The ability to set your own MAC address filters is a potential security issue, so host administrators have the ability to determine whether they trust the VF (and implicitly, the domain in which the VF resides).  There is also a sort of half-way solution.  By turning off anti-spoofing you can allow the VF to use source MAC addresses that are not actually assigned in HW filters.  This was done to support some bonding scenarios where the VF will need to transmit with a different source address.

Many applications using SR-IOV are embedded devices such as switches, edge relay devices, IP forwarding/filtering appliances, routers, etc.  More often than not the host administrator can trust domains that the VFs are assigned to because those domains are completely under the control of the local host.  In those cases the VFs are trusted and can be allowed to set their own MAC filters and use any source MAC address they please.

Other applications might need to assign VF devices to non-trusted domains.  Perhaps a service provider has leased a virtual machine domain to a subscriber who has purchased QoS levels that can only be met with the performance levels available with SR-IOV VF devices.  Other scenarios exist.  In these cases it is worthwhile to be able to restrict the VF's ability to set MAC filters and use source MAD addresses not assigned to it.

Rather than a tug of war I just view it as balancing security concerns with levels of additional capability and functionality.  That goes on all the time.

- Greg

^ permalink raw reply

* Re: [PATCH net/for-next V1 1/1] IB/ipoib: break linkage to neighbouring system
From: Or Gerlitz @ 2012-07-23 16:58 UTC (permalink / raw)
  To: Eric Dumazet, roland-DgEjT+Ai2ygdnm+yROfE0A,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, Christoph Lameter
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA, erezsh-VPRAkNaXOzVWk0Htik3J/w,
	Shlomo Pongratz, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CAJZOPZ+kRcBjJgB_HaMqeuB5E-SLSqskgoaLZ_hvVx4KffHgpA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On 20/07/2012 18:49, Or Gerlitz wrote:
> On Thu, Jul 19, 2012 at 4:18 PM, Or Gerlitz<ogerlitz-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>  wrote:
>> From: Shlomo Pongratz<shlomop-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
>>
>> Dave Miller<davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>  provided a detailed description of why the
>> way IPoIB is using neighbours for its own ipoib_neigh struct is buggy:
> [...]
>
>> This patch aims to solve the race conditions found in the IPoIB driver.
>>
>> The patch breaks the connection between the core networking neighbour structure
>> and the ipoib_neigh structure. Except for avoiding the race, it allows to in
>> under a setup where SKBs carrying IP packets that don't have any associated
>> neighbour are transmitted through IPoIB.
>>
>> We add an ipoib_neigh hash table with 1024 buckets. The hash table key is the destin
>> hardware address. Thus the ipoib_neigh is fetched from the hash table and not
>> dereferenced from the stashed location at the neighbour structure. The hash table uses
>> both RCU and reference count mechanisms to guarantee that no ipoib_neigh instance is
>> ever deleted while in use.
>>
>> Fetching the ipoib_neigh structure instance from the hash also makes the special
>> code in ipoib_start_xmit that handles remote and local bonding failover redundant.
>>
>> Aged ipoib_neigh instances are deleted by a garbage collection task that runs every
>> 30 seconds and deletes every ipoib_neigh instance that was idle for at least 60
>> seconds. The deletion is safe since the ipoib_neigh instances are protected
>> using RCU and reference count mechanisms.
>
> Hi Dave, Roland, Eric
>
> So how does this look? in the right direction? anything that need to be fixed?

Sorry for the possible spam, but resending (last time forgot to put Eric 
on the "to" list so he might missed it, also added Christoph) - this is 
a fix for very long time bug in IPoIB and we do want it to be reviewed 
&& and hopefully accepted, or if needed get feedback and fix/change.

So, any further feedback? there was one feedback on V0, not to use read 
lock for RCU protected
hash table lookup, and it was addressed in V1.

Or.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RE: New commands to configure IOV features
From: Rose, Gregory V @ 2012-07-23 17:06 UTC (permalink / raw)
  To: Chris Friesen, Don Dutile
  Cc: Ben Hutchings, David Miller, yuvalmin@broadcom.com,
	netdev@vger.kernel.org, linux-pci@vger.kernel.org
In-Reply-To: <500D6932.8090306@genband.com>

> -----Original Message-----
> From: Chris Friesen [mailto:chris.friesen@genband.com]
> Sent: Monday, July 23, 2012 8:10 AM
> To: Don Dutile
> Cc: Ben Hutchings; David Miller; yuvalmin@broadcom.com; Rose, Gregory V;
> netdev@vger.kernel.org; linux-pci@vger.kernel.org
> Subject: Re: New commands to configure IOV features
> 
> On 07/23/2012 08:03 AM, Don Dutile wrote:
> > On 07/20/2012 07:42 PM, Chris Friesen wrote:
> >>
> >> I actually have a use-case where the guest needs to be able to modify
> >> the MAC addresses of network devices that are actually VFs.
> >>
> >> The guest is bonding the network devices together, so the bonding
> >> driver in the guest expects to be able to set all the slaves to the
> >> same MAC address.
> >>
> >> As I read the ixgbe driver, this should be possible as long as the
> >> host hasn't explicitly set the MAC address of the VF. Is that correct?
> >>
> >> Chris
> >
> > Interesting tug of war: hypervisors will want to set the macaddrs for
> > security reasons,
> >                         some guests may want to set macaddr for
> > (valid?) config reasons.
> >
> 
> In our case we have control over both guest an host anyways, so it's
> less of a security issue.  In the general case though I could see it
> being an interesting problem.
> 
> Back to the original discussion though--has anyone got any ideas about
> the best way to trigger runtime creation of VFs?  I don't know what the
> binary APIs looks like, but via sysfs I could see something like
> 
> echo number_of_new_vfs_to_create >
> /sys/bus/pci/devices/<address>/create_vfs

The original proposals for creation and management of virtual functions were very much along these lines.  However, at the time most of the distributions that used virtualization were based upon the 2.6.18 kernel.  Red Hat Enterprise Linux 5.x and Citrix Xen Server were both using kernels derived from the 2.6.18 kernel.  In order to implement a sysfs based management approach we would have had to break the 2.6.18 kernel ABI, which is of course a non-starter.  We were able to implement an SR-IOV solution without breaking the ABI but it required use of a module parameter to inform the PF driver of how many VFs it should create.

This was fine during the first couple of years in which not many folks were using SR-IOV outside of a lab and the number of platforms that supported SR-IOV was very limited.  As an experimental solution it has worked pretty well.

The last year and a half or so we have seen SR-IOV go from an experimental technology to one that is becoming increasingly deployed in real world applications and now the limitations of that original approach are more apparent.  

> 
> Something else that occurred to me--is there buy-in from driver
> maintainers?

That's a good question.  I've seen a lot of resistance to using sysfs based interfaces in drivers but usually that is when a driver wants to implement a private interface that other drivers wouldn't want or be able to use.  I'm less sure about a sysfs solution that could be deployed by all SR-IOV capable devices of any type, be they Ethernet controllers, SCSI controllers, etc.  I'm not sure what the objection would be in case of a general purpose solution, however I've been told that in general sysfs based solutions are often frowned upon by kernel maintainers.  Perhaps that is because many of them are not generic solutions to a well defined problem.

  I know the Intel ethernet drivers (what I'm most familiar
> with) would need to be substantially modified to support on-the-fly
> addition of new vfs.

Actually, it wouldn't be that bad.  But yes, there'd have to be some way for a driver to register a callback routine with the PCI interface so that it could be notified when changes have been made to the SR-IOV configuration of the device.  This would require a new API, and some driver changes.  In the case of the Intel drivers it wouldn't be too intrusive and it would definitely help us to meet some customer requirements.  The current model using a module parameter forces all ports controlled by a PF driver to use the same number of VFs per function.  This is clunky and there are a lot of users that would like the ability to assign differing numbers of VFs to respective physical functions.

I should also note that we need to be careful about what we mean by the phrase "support on-the-fly addition of new vfs".

You cannot just add VFs without first tearing down the VFs that are currently allocated.  This is a limitation of the PCIE SR-IOV spec IIRC, and in any case it is true with Intel SR-IOV capable devices.  The numVFs parameter in the SR-IOV capability structure is not writeable while the VF enable bit is set.  To change that value you must first clear the VF enable bit and when you do that all your current VFs cease to exist.  You can then write a different number of VFs and re-enable them but during a short interval the VFs that were already there are destroyed and completely reset when they come back on-line.

- Greg

^ permalink raw reply

* Re: [PATCH net/for-next V1 1/1] IB/ipoib: break linkage to neighbouring system
From: Eric Dumazet @ 2012-07-23 17:17 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: roland, davem, Christoph Lameter, linux-rdma, erezsh,
	Shlomo Pongratz, netdev
In-Reply-To: <500D82AD.2040508@mellanox.com>

On Mon, 2012-07-23 at 19:58 +0300, Or Gerlitz wrote:

> Sorry for the possible spam, but resending (last time forgot to put Eric 
> on the "to" list so he might missed it, also added Christoph) - this is 
> a fix for very long time bug in IPoIB and we do want it to be reviewed 
> && and hopefully accepted, or if needed get feedback and fix/change.
> 
> So, any further feedback? there was one feedback on V0, not to use read 
> lock for RCU protected
> hash table lookup, and it was addressed in V1.
> 

I have no idea of what you are talking about, I have not the patch or a
copy of it ;)

^ permalink raw reply

* Re: unsubscribe netdev
From: Peter Portante @ 2012-07-23 17:26 UTC (permalink / raw)
  To: netdev
In-Reply-To: <C8E995C2-7D48-451F-8AEA-69BF2DA59F1F@redhat.com>

unsubscribe netdev

^ permalink raw reply

* Re: [PATCH 1/2] ipvs: ip_vs_ftp depends on nf_conntrack_ftp helper
From: Pablo Neira Ayuso @ 2012-07-23 17:39 UTC (permalink / raw)
  To: Simon Horman
  Cc: Julian Anastasov, lvs-devel, netdev, netfilter-devel,
	Wensong Zhang, Hans Schillstrom, Jesper Dangaard Brouer
In-Reply-To: <20120723064818.GK6603@verge.net.au>

On Mon, Jul 23, 2012 at 03:48:18PM +0900, Simon Horman wrote:
> On Thu, Jul 12, 2012 at 10:43:22PM +0300, Julian Anastasov wrote:
> > 
> > 	Hello,
> > 
> > On Thu, 12 Jul 2012, Pablo Neira Ayuso wrote:
> > 
> > > On Wed, Jul 11, 2012 at 09:25:26AM +0900, Simon Horman wrote:
> > > > From: Julian Anastasov <ja@ssi.bg>
> > > > 
> > > > 	The FTP application indirectly depends on the
> > > > nf_conntrack_ftp helper for proper NAT support. If the
> > > > module is not loaded, IPVS can resize the packets for the
> > > > command connection, eg. PASV response but the SEQ adjustment
> > > > logic in ipv4_confirm is not called without helper.
> > > > 
> > > > Signed-off-by: Julian Anastasov <ja@ssi.bg>
> > > > Signed-off-by: Simon Horman <horms@verge.net.au>
> > > > ---
> > > >  net/netfilter/ipvs/Kconfig | 3 ++-
> > > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/net/netfilter/ipvs/Kconfig b/net/netfilter/ipvs/Kconfig
> > > > index f987138..8b2cffd 100644
> > > > --- a/net/netfilter/ipvs/Kconfig
> > > > +++ b/net/netfilter/ipvs/Kconfig
> > > > @@ -250,7 +250,8 @@ comment 'IPVS application helper'
> > > >  
> > > >  config	IP_VS_FTP
> > > >    	tristate "FTP protocol helper"
> > > > -        depends on IP_VS_PROTO_TCP && NF_CONNTRACK && NF_NAT
> > > > +	depends on IP_VS_PROTO_TCP && NF_CONNTRACK && NF_NAT && \
> > > > +		NF_CONNTRACK_FTP
> > > 
> > > If you require FTP NAT support, then this depends on NF_NAT_FTP
> > > instead of NF_CONNTRACK_FTP.
> > 
> > 	No, I just checked again, it works without nf_nat_ftp,
> > only nf_nat, nf_conntrack_ftp and iptable_nat are needed.
> > We use packet mangling part from nf_nat (nf_nat_mangle_tcp_packet).
> 
> Is there a consensus on this?

Fine with me, just wanted to make sure this what you wanted. Thanks
Simon.

^ permalink raw reply

* [net-next] net/ipv4/ip_vti.c: Fix __rcu warnings detected by sparse.
From: Saurabh @ 2012-07-23 17:52 UTC (permalink / raw)
  To: netdev



With CONFIG_SPARSE_RCU_POINTER=y sparse identified references which did not
specificy __rcu in ip_vti.c

Signed-off-by: Saurabh Mohan <saurabh.mohan@vyatta.com>
Reported-by: Fengguang Wu <fengguang.wu@intel.com>

---
diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index c41b5c3..3511ffb 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -55,7 +55,7 @@ struct vti_net {
 	struct ip_tunnel __rcu *tunnels_r[HASH_SIZE];
 	struct ip_tunnel __rcu *tunnels_l[HASH_SIZE];
 	struct ip_tunnel __rcu *tunnels_wc[1];
-	struct ip_tunnel **tunnels[4];
+	struct ip_tunnel __rcu **tunnels[4];
 
 	struct net_device *fb_tunnel_dev;
 };
@@ -160,8 +160,8 @@ static struct ip_tunnel *vti_tunnel_lookup(struct net *net,
 	return NULL;
 }
 
-static struct ip_tunnel **__vti_bucket(struct vti_net *ipn,
-				       struct ip_tunnel_parm *parms)
+static struct ip_tunnel __rcu **__vti_bucket(struct vti_net *ipn,
+					     struct ip_tunnel_parm *parms)
 {
 	__be32 remote = parms->iph.daddr;
 	__be32 local = parms->iph.saddr;
@@ -179,8 +179,8 @@ static struct ip_tunnel **__vti_bucket(struct vti_net *ipn,
 	return &ipn->tunnels[prio][h];
 }
 
-static inline struct ip_tunnel **vti_bucket(struct vti_net *ipn,
-					    struct ip_tunnel *t)
+static inline struct ip_tunnel __rcu **vti_bucket(struct vti_net *ipn,
+						  struct ip_tunnel *t)
 {
 	return __vti_bucket(ipn, &t->parms);
 }

^ permalink raw reply related

* Re: [PATCH 00/16] Remove the ipv4 routing cache
From: Paweł Staszewski @ 2012-07-23 17:54 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev
In-Reply-To: <1343027752.2626.10175.camel@edumazet-glaptop>

W dniu 2012-07-23 09:15, Eric Dumazet pisze:
> On Sun, 2012-07-22 at 17:39 -0700, David Miller wrote:
>> Just FYI, I'm pushing this work out to net-next now.
>> --
> Excellent !
>
> Thanks a lot David
>
>
> --
> 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
>
Yes
Excellent work.

I make some real life tests with 10Gbit traffic and make some rdos.
With kernel 3.4.5 when I reach 6M routing cache entries - traffic is not 
forwarded - on my hardware with 64G RAM

With kernel - 3.5.0-rc7-next-20120720 + David M. patches - route cache 
removal + fix
Traffic is forwarder all the time at speed 8.2Mpps RX from traffic 
generator and 8.2Mpps TX to the sink

Also I see performance improvement.

With kernel 3.4.5 I can reach maximum od 7.5Mpps with my hardware
And with kernel 3.5.0-rc7-next-20120720 i can reach 8.2Mpps - forwarding 
UDP performance - can be more - but my TX pktgen that use 6 cores can't 
do more.

Really thanks for this :)


BR
Paweł Staszewski

^ permalink raw reply

* Re: [PATCH] mlx4: Add support for EEH error recovery
From: Kleber Sacilotto de Souza @ 2012-07-23 18:12 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: David Miller, netdev, jackm, yevgenyp, cascardo, brking, shlomop
In-Reply-To: <500D556F.4000409@mellanox.com>

On 07/23/2012 10:45 AM, Or Gerlitz wrote:

> On 7/23/2012 4:18 PM, Kleber Sacilotto de Souza wrote:
>> Exactly. The callbacks implemented are from standard PCI error recovery
>> (Documentation/PCI/pci-error-recovery.txt) and the changes doesn't
>> assume any platform in specific. The code was tested only on powerpc
>> systems [...]
> 
> So how did you test that? using the kernel provided error injection
> support and user space tool (which?) or in another way? we've trying
> quickly here to inject errors using /sbin/ear-inject from
> ras-utils-6.1-1.el6.x86_64 on a kernel built with
> 
> CONFIG_PCIEAER=y
> CONFIG_PCIEAER_INJECT=m


For powerpc we have an IBM internal user space tool that injects the
error on the bus with the aid of the system firmware. The kernel used
was built with the option:

CONFIG_EEH=y

and without the AER options. I will run some more tests with the AER
options activated.

> 
> and it failed to inject errors, SB details.
> 
> Or.
>> since I don't have any mlx4 card on other platforms, however,
>> these changes shouldn't make the error recover any worse than the
>> current state.
> 
>> # lspci | grep 08.00.1
>> 08:00.1 Ethernet controller: Intel Corporation 82575EB Gigabit Network
>> Connection (rev 02)
> 
>> # cat /tmp/intel.aer
>> AER
>> BUS 8 DEV 0 FN 1
>> COR_STATUS BAD_TLP
>> HEADER_LOG 0 1 2 3
> 
>> # /sbin/aer-inject < /tmp/intel.aer
>> Error: Failed to write, Invalid argument
> 
> 
> 
>> # strace -F -f /sbin/aer-inject < /tmp/intel.aer
>> [...]
> 
>> open("/dev/aer_inject", O_WRONLY)       = 3
>> write(3, "\10\0\1\0\0\0\0\0@\0\0\0\0\0\0\0\1\0\0\0\2\0\0\0\3\0\0\0",
>> 28) = -1 EINVAL (Invalid argument)
>> write(2, "Error: ", 7Error: )                  = 7
>> write(2, "Failed to write", 15Failed to write)         = 15
>> write(2, ", Invalid argument\n", 19, Invalid argument
>> )    = 19
>> exit_group(-1)                          = ?
> 
> 
> 
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 



-- 
Kleber Sacilotto de Souza
IBM Linux Technology Center

^ permalink raw reply

* Re: New commands to configure IOV features
From: Stephen Hemminger @ 2012-07-23 18:36 UTC (permalink / raw)
  To: Chris Friesen
  Cc: Don Dutile, Ben Hutchings, David Miller, yuvalmin, gregory.v.rose,
	netdev, linux-pci
In-Reply-To: <500D6932.8090306@genband.com>

On Mon, 23 Jul 2012 09:09:38 -0600
Chris Friesen <chris.friesen@genband.com> wrote:

> On 07/23/2012 08:03 AM, Don Dutile wrote:
> > On 07/20/2012 07:42 PM, Chris Friesen wrote:
> >>
> >> I actually have a use-case where the guest needs to be able to modify 
> >> the MAC addresses of network devices that are actually VFs.
> >>
> >> The guest is bonding the network devices together, so the bonding 
> >> driver in the guest expects to be able to set all the slaves to the 
> >> same MAC address.
> >>
> >> As I read the ixgbe driver, this should be possible as long as the 
> >> host hasn't explicitly set the MAC address of the VF. Is that correct?
> >>
> >> Chris
> >
> > Interesting tug of war: hypervisors will want to set the macaddrs for 
> > security reasons,
> >                         some guests may want to set macaddr for 
> > (valid?) config reasons.
> >
> 
> In our case we have control over both guest an host anyways, so it's 
> less of a security issue.  In the general case though I could see it 
> being an interesting problem.
> 
> Back to the original discussion though--has anyone got any ideas about 
> the best way to trigger runtime creation of VFs?  I don't know what the 
> binary APIs looks like, but via sysfs I could see something like
> 
> echo number_of_new_vfs_to_create >  
> /sys/bus/pci/devices/<address>/create_vfs
> 
> Something else that occurred to me--is there buy-in from driver 
> maintainers?  I know the Intel ethernet drivers (what I'm most familiar 
> with) would need to be substantially modified to support on-the-fly 
> addition of new vfs.  Currently they assume that the number of vfs is 
> known at module init time.
> 

Why couldn't rtnl_link_ops be used for this. It is already the preferred
interface to create vlan's, bond devices, and other virtual devices?
The one issue is that do the created VF's exist in kernel as devices
or only visible to guest?

^ permalink raw reply

* Re: [PATCH net/for-next V1 1/1] IB/ipoib: break linkage to neighbouring system
From: Or Gerlitz @ 2012-07-23 18:37 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Or Gerlitz, roland-DgEjT+Ai2ygdnm+yROfE0A,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, Christoph Lameter,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA, erezsh-VPRAkNaXOzVWk0Htik3J/w,
	Shlomo Pongratz, netdev-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1343063863.2626.11028.camel@edumazet-glaptop>

On Mon, Jul 23, 2012 at 8:17 PM, Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:

> I have no idea of what you are talking about, I have not the patch or a
> copy of it ;)

http://marc.info/?t=134271491300002&r=1&w=2
http://marc.info/?t=134271491300005&r=1&w=2
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* RE: New commands to configure IOV features
From: Rose, Gregory V @ 2012-07-23 18:40 UTC (permalink / raw)
  To: Stephen Hemminger, Chris Friesen
  Cc: Don Dutile, Ben Hutchings, David Miller, yuvalmin@broadcom.com,
	netdev@vger.kernel.org, linux-pci@vger.kernel.org
In-Reply-To: <20120723113607.56ce7aaf@nehalam.linuxnetplumber.net>

> -----Original Message-----
> From: Stephen Hemminger [mailto:shemminger@vyatta.com]
> Sent: Monday, July 23, 2012 11:36 AM
> To: Chris Friesen
> Cc: Don Dutile; Ben Hutchings; David Miller; yuvalmin@broadcom.com; Rose,
> Gregory V; netdev@vger.kernel.org; linux-pci@vger.kernel.org
> Subject: Re: New commands to configure IOV features
> 
> On Mon, 23 Jul 2012 09:09:38 -0600
> Chris Friesen <chris.friesen@genband.com> wrote:
> 
> > On 07/23/2012 08:03 AM, Don Dutile wrote:
> > > On 07/20/2012 07:42 PM, Chris Friesen wrote:
> > >>
> > >> I actually have a use-case where the guest needs to be able to
> > >> modify the MAC addresses of network devices that are actually VFs.
> > >>
> > >> The guest is bonding the network devices together, so the bonding
> > >> driver in the guest expects to be able to set all the slaves to the
> > >> same MAC address.
> > >>
> > >> As I read the ixgbe driver, this should be possible as long as the
> > >> host hasn't explicitly set the MAC address of the VF. Is that
> correct?
> > >>
> > >> Chris
> > >
> > > Interesting tug of war: hypervisors will want to set the macaddrs
> > > for security reasons,
> > >                         some guests may want to set macaddr for
> > > (valid?) config reasons.
> > >
> >
> > In our case we have control over both guest an host anyways, so it's
> > less of a security issue.  In the general case though I could see it
> > being an interesting problem.
> >
> > Back to the original discussion though--has anyone got any ideas about
> > the best way to trigger runtime creation of VFs?  I don't know what
> > the binary APIs looks like, but via sysfs I could see something like
> >
> > echo number_of_new_vfs_to_create >
> > /sys/bus/pci/devices/<address>/create_vfs
> >
> > Something else that occurred to me--is there buy-in from driver
> > maintainers?  I know the Intel ethernet drivers (what I'm most
> > familiar
> > with) would need to be substantially modified to support on-the-fly
> > addition of new vfs.  Currently they assume that the number of vfs is
> > known at module init time.
> >
> 
> Why couldn't rtnl_link_ops be used for this. It is already the preferred
> interface to create vlan's, bond devices, and other virtual devices?
> The one issue is that do the created VF's exist in kernel as devices or
> only visible to guest?

I would say that rtnl_link_ops are network oriented and not appropriate for something like a storage controller or graphics device, which are two other common SR-IOV capable devices.

I think it should be oriented toward the PCIe interface and subsystems in the kernel.

- Greg

^ permalink raw reply

* Re: ibmveth bug?
From: Nishanth Aravamudan @ 2012-07-23 19:03 UTC (permalink / raw)
  To: Benjamin Herrenschmidt
  Cc: santil, anton, paulus, netdev, linux-kernel, Anton Blanchard
In-Reply-To: <1342831966.29855.6.camel@pasglop>

Hi Ben,

On 21.07.2012 [10:52:46 +1000], Benjamin Herrenschmidt wrote:
> On Fri, 2012-07-20 at 15:41 -0700, Nishanth Aravamudan wrote:
> > Ping on this ... we've tripped the same issue on a different system, it
> > would appear. Would appreciate if anyone can provide answers to the
> > questions below.
> 
> Well, I haven't hit it but your description makes sense. Why not use
> dma_alloc_coherent though ? Rather than kmalloc followed by map ?

Thanks for the response!

dma_alloc_coherent here makes sense. I honestly haven't though too much
about solutions yet, as the problem isn't consistent. For instance, the
one reproducer right now is hitting it only at install time, and only if
another particular PCI device is also plugged in to the machine.

> At least on power we give you page alignment for these, so the problem
> is solved magically :-) Another option is GFP + dma_map_page which is
> roughly equivalent.
> 
> If you think it's a waste of space based on the queue size, then just
> add an extra pointer, I wouldn't bother with a cache for something only
> allocated when the driver initializes.

Agreed, will try to code something up this week.

Thanks,
Nish

> 
> Cheers,
> Ben.
> 
> > Thanks,
> > Nish
> > 
> > On 15.05.2012 [10:01:41 -0700], Nishanth Aravamudan wrote:
> > > Hi Santiago,
> > > 
> > > Are you still working on ibmveth?
> > > 
> > > I've found a very sporadic bug with ibmveth in some testing. PAPR
> > > requires that:
> > > 
> > > "Validate the Buffer Descriptor of the receive queue buffer (I/O
> > > addresses for entire buffer length starting at the spec- ified I/O
> > > address are translated by the RTCE table, length is a multiple of 16
> > > bytes, and alignment is on a 16 byte boundary) else H_Parameter."
> > > 
> > > but from what I can tell ibmveth.c is not enforcing this last condition:
> > > 
> > > 	adapter->rx_queue.queue_addr =
> > > 		kmalloc(adapter->rx_queue.queue_len, GFP_KERNEL);
> > > 
> > > 	...
> > > 
> > > 	adapter->rx_queue.queue_dma = dma_map_single(dev,
> > > 		adapter->rx_queue.queue_addr, adapter->rx_queue.queue_len,
> > > 		DMA_BIDIRECTIONAL);
> > > 
> > > 	...
> > > 
> > > 	rxq_desc.fields.address = adapter->rx_queue.queue_dma;
> > > 
> > > 	...
> > > 	
> > > 
> > > 	lpar_rc = ibmveth_register_logical_lan(adapter, rxq_desc,
> > > 		mac_address);
> > > 	netdev_err(netdev, "buffer TCE:0x%llx filter TCE:0x%llx rxq "
> > > 	 	"desc:0x%llx MAC:0x%llx\n", adapter->buffer_list_dma,
> > > 	 	adapter->filter_list_dma, rxq_desc.desc, mac_address);
> > > 
> > > And I got on one install attempt:
> > > 
> > > [ 39.978430] ibmveth 30000004: eth0: h_register_logical_lan failed with -4
> > > [ 39.978449] ibmveth 30000004: eth0: buffer TCE:0x1000 filter TCE:0x10000 rxq desc:0x80006010000200a8 MAC:0x56754de8e904
> > > 
> > > rxq desc, as you can see is not 16byte aligned. kmalloc() only
> > > guarantees 8-byte alignment (as does gcc, I think). Initially, I thought
> > > we could just overallocate the queue_addr and ALIGN() down, but then we
> > > would need to save the original kmalloc pointer in a new struct member
> > > per rx_queue.
> > > 
> > > So a couple of questions:
> > > 
> > > 1) Is my analysis accurate? :)
> > > 
> > > 2) How gross would it be to save an extra pointer for every rx_queue?
> > > 
> > > 3) Based upon 2), is it better to just go ahead and create our own
> > > kmem_cache (which gets an alignment specified)?
> > > 
> > > For 3), I started coding this, but couldn't find a clean place to
> > > allocate the kmem_cache itself, as the size of each object depends on
> > > the run-time characteristics (afaict), but needs to be specified at
> > > cache creation time. Any insight you could provide would be great!
> > > 
> > > Thanks,
> > > Nish
> > >  
> > > -- 
> > > Nishanth Aravamudan <nacc@us.ibm.com>
> > > IBM Linux Technology Center
> > 
> 
> 

-- 
Nishanth Aravamudan <nacc@us.ibm.com>
IBM Linux Technology Center

^ permalink raw reply

* Re: [PATCH 11/16] ipv4: Cache input routes in fib_info nexthops.
From: Julian Anastasov @ 2012-07-23 20:00 UTC (permalink / raw)
  To: David Miller; +Cc: netdev
In-Reply-To: <alpine.LFD.2.00.1207231157590.1626@ja.ssi.bg>


	Hello,

On Mon, 23 Jul 2012, Julian Anastasov wrote:

> 	The problem with rt_iif is worse. May be we
> can cache only the first iif, other packets will see
> different iif in nh_rth_input and will get non-cached
> result. For boxes with 2 or more interfaces only one
> can use the cache. One setup can have large traffic
> from LAN, other can be server for remote clients.

	May be we can replace rt_iif with skb->dev->ifindex ?
If we have dst, there must be skb->dev ? Only for
output routes we should check places that rely on
inet_iif().

Regards

--
Julian Anastasov <ja@ssi.bg>

^ permalink raw reply

* Re: [patch] openvswitch: potential NULL deref in sample()
From: Jesse Gross @ 2012-07-23 19:54 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: dev-yBygre7rU0TnMu66kgdUjQ, netdev-u79uwXL29TY76Z2rM5mHXA,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA, David S. Miller
In-Reply-To: <20120723074628.GA30892-mgFCXtclrQlZLf2FXnZxJA@public.gmane.org>

On Mon, Jul 23, 2012 at 12:46 AM, Dan Carpenter
<dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org> wrote:
> If there is no OVS_SAMPLE_ATTR_ACTIONS set then "acts_list" is NULL and
> it leads to a NULL dereference when we call nla_len(acts_list).  This
> is a static checker fix, not something I have seen in testing.
>
> Signed-off-by: Dan Carpenter <dan.carpenter-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org>

This can never happen in practice because the action list is validated
at the time that userspace installs the flow.  There are plenty of
things in this category that would appear to be unsafe because we'd
much rather do sanity checking on a per-flow basis rather than
per-packet.

^ permalink raw reply

* Re: [PATCH 11/16] ipv4: Cache input routes in fib_info nexthops.
From: David Miller @ 2012-07-23 19:58 UTC (permalink / raw)
  To: ja; +Cc: netdev
In-Reply-To: <alpine.LFD.2.00.1207232245230.1619@ja.ssi.bg>

From: Julian Anastasov <ja@ssi.bg>
Date: Mon, 23 Jul 2012 23:00:57 +0300 (EEST)

> 	May be we can replace rt_iif with skb->dev->ifindex ?
> If we have dst, there must be skb->dev ? Only for
> output routes we should check places that rely on
> inet_iif().

Right, just as I was drinking my first coffee after reading
your previous email I was thinking about this.

The only time skb->dev->ifindex can change from rt->rt_iif is when we
demux a tunnel, but at that point we would first reinject and do
another route lookup, at which point rt->rt_iif would match again.

And this is the intended semantic of this field anyways.

I'll look into what we can do here, and I'll also try to come up
with some ideas wrt. the DIRECTSRC issue as well.

Thanks Julian.

^ permalink raw reply

* Re: [PATCH] USB: plusb: Add support for PL-2501
From: David Miller @ 2012-07-23 20:08 UTC (permalink / raw)
  To: bas; +Cc: linux-kernel, sshtylyov, linux-usb, netdev, greg
In-Reply-To: <alpine.LNX.2.02.1207231540580.1183@bas>

From: kyak <bas@bmail.ru>
Date: Mon, 23 Jul 2012 15:44:11 +0400 (MSK)

> From: Mikhail Peselnik <peselnik@gmail.com>
> 
> This patch adds support for PL-2501 by adding the appropriate USB
> ID's. This chip is used in several USB 'Easy Trasfer' Cables.
> 
> Signed-off-by: Mikhail Peselnik <peselnik@gmail.com>
> Tested-by: Mikhail Peselnik <peselnik@gmail.com>

This does not apply cleanly to my net-next tree at all.

^ permalink raw reply

* Re: [PATCH] ipv4: Remove redundant assignment
From: David Miller @ 2012-07-23 20:09 UTC (permalink / raw)
  To: mlin; +Cc: netdev
In-Reply-To: <1343052681.4600.5.camel@monkey32>

From: Lin Ming <mlin@ss.pku.edu.cn>
Date: Mon, 23 Jul 2012 22:11:21 +0800

> It is redundant to set no_addr and accept_local to 0 and then set them
> with other values just after that.
> 
> Signed-off-by: Lin Ming <mlin@ss.pku.edu.cn>

Applied, thanks.

^ permalink raw reply

* Re: [net-next] net/ipv4/ip_vti.c: Fix __rcu warnings detected by sparse.
From: David Miller @ 2012-07-23 20:09 UTC (permalink / raw)
  To: saurabh.mohan; +Cc: netdev
In-Reply-To: <20120723175204.GA2762@debian-saurabh-64.vyatta.com>

From: Saurabh <saurabh.mohan@vyatta.com>
Date: Mon, 23 Jul 2012 10:52:04 -0700

> 
> 
> With CONFIG_SPARSE_RCU_POINTER=y sparse identified references which did not
> specificy __rcu in ip_vti.c
> 
> Signed-off-by: Saurabh Mohan <saurabh.mohan@vyatta.com>
> Reported-by: Fengguang Wu <fengguang.wu@intel.com>

Applied.

^ permalink raw reply

* Re: [PATCH 00/16] Remove the ipv4 routing cache
From: David Miller @ 2012-07-23 20:10 UTC (permalink / raw)
  To: pstaszewski; +Cc: eric.dumazet, netdev
In-Reply-To: <500D8FEE.7050802@itcare.pl>

From: Paweł Staszewski <pstaszewski@itcare.pl>
Date: Mon, 23 Jul 2012 19:54:54 +0200

> Really thanks for this :)

Thanks for testing.

^ permalink raw reply

* [PATCH] decnet: Don't set RTCF_DIRECTSRC.
From: David Miller @ 2012-07-23 20:17 UTC (permalink / raw)
  To: netdev


It's an ipv4 defined route flag, and only ipv4 uses it.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/decnet/dn_route.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/net/decnet/dn_route.c b/net/decnet/dn_route.c
index 23cc11d..85a3604 100644
--- a/net/decnet/dn_route.c
+++ b/net/decnet/dn_route.c
@@ -1424,7 +1424,6 @@ static int dn_route_input_slow(struct sk_buff *skb)
 		/* Packet was intra-ethernet, so we know its on-link */
 		if (cb->rt_flags & DN_RT_F_IE) {
 			gateway = cb->src;
-			flags |= RTCF_DIRECTSRC;
 			goto make_route;
 		}
 
@@ -1437,7 +1436,6 @@ static int dn_route_input_slow(struct sk_buff *skb)
 
 		/* Close eyes and pray */
 		gateway = cb->src;
-		flags |= RTCF_DIRECTSRC;
 		goto make_route;
 	default:
 		goto e_inval;
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 1/2] ipv4: Really ignore ICMP address requests/replies.
From: David Miller @ 2012-07-23 20:23 UTC (permalink / raw)
  To: netdev


Alexey removed kernel side support for requests, and the
only thing we do for replies is log a message if something
doesn't look right.

As Alexey's comment indicates, this belongs in userspace (if
anywhere), and thus we can safely just get rid of this code.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/ipv4/icmp.c |   84 ++-----------------------------------------------------
 1 file changed, 2 insertions(+), 82 deletions(-)

diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
index ea3a996..f2a06be 100644
--- a/net/ipv4/icmp.c
+++ b/net/ipv4/icmp.c
@@ -837,86 +837,6 @@ out_err:
 	goto out;
 }
 
-
-/*
- *	Handle ICMP_ADDRESS_MASK requests.  (RFC950)
- *
- * RFC1122 (3.2.2.9).  A host MUST only send replies to
- * ADDRESS_MASK requests if it's been configured as an address mask
- * agent.  Receiving a request doesn't constitute implicit permission to
- * act as one. Of course, implementing this correctly requires (SHOULD)
- * a way to turn the functionality on and off.  Another one for sysctl(),
- * I guess. -- MS
- *
- * RFC1812 (4.3.3.9).	A router MUST implement it.
- *			A router SHOULD have switch turning it on/off.
- *		      	This switch MUST be ON by default.
- *
- * Gratuitous replies, zero-source replies are not implemented,
- * that complies with RFC. DO NOT implement them!!! All the idea
- * of broadcast addrmask replies as specified in RFC950 is broken.
- * The problem is that it is not uncommon to have several prefixes
- * on one physical interface. Moreover, addrmask agent can even be
- * not aware of existing another prefixes.
- * If source is zero, addrmask agent cannot choose correct prefix.
- * Gratuitous mask announcements suffer from the same problem.
- * RFC1812 explains it, but still allows to use ADDRMASK,
- * that is pretty silly. --ANK
- *
- * All these rules are so bizarre, that I removed kernel addrmask
- * support at all. It is wrong, it is obsolete, nobody uses it in
- * any case. --ANK
- *
- * Furthermore you can do it with a usermode address agent program
- * anyway...
- */
-
-static void icmp_address(struct sk_buff *skb)
-{
-#if 0
-	net_dbg_ratelimited("a guy asks for address mask. Who is it?\n");
-#endif
-}
-
-/*
- * RFC1812 (4.3.3.9).	A router SHOULD listen all replies, and complain
- *			loudly if an inconsistency is found.
- * called with rcu_read_lock()
- */
-
-static void icmp_address_reply(struct sk_buff *skb)
-{
-	struct rtable *rt = skb_rtable(skb);
-	struct net_device *dev = skb->dev;
-	struct in_device *in_dev;
-	struct in_ifaddr *ifa;
-
-	if (skb->len < 4 || !(rt->rt_flags&RTCF_DIRECTSRC))
-		return;
-
-	in_dev = __in_dev_get_rcu(dev);
-	if (!in_dev)
-		return;
-
-	if (in_dev->ifa_list &&
-	    IN_DEV_LOG_MARTIANS(in_dev) &&
-	    IN_DEV_FORWARD(in_dev)) {
-		__be32 _mask, *mp;
-
-		mp = skb_header_pointer(skb, 0, sizeof(_mask), &_mask);
-		BUG_ON(mp == NULL);
-		for (ifa = in_dev->ifa_list; ifa; ifa = ifa->ifa_next) {
-			if (*mp == ifa->ifa_mask &&
-			    inet_ifa_match(ip_hdr(skb)->saddr, ifa))
-				break;
-		}
-		if (!ifa)
-			net_info_ratelimited("Wrong address mask %pI4 from %s/%pI4\n",
-					     mp,
-					     dev->name, &ip_hdr(skb)->saddr);
-	}
-}
-
 static void icmp_discard(struct sk_buff *skb)
 {
 }
@@ -1080,10 +1000,10 @@ static const struct icmp_control icmp_pointers[NR_ICMP_TYPES + 1] = {
 		.handler = icmp_discard,
 	},
 	[ICMP_ADDRESS] = {
-		.handler = icmp_address,
+		.handler = icmp_discard,
 	},
 	[ICMP_ADDRESSREPLY] = {
-		.handler = icmp_address_reply,
+		.handler = icmp_discard,
 	},
 };
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 2/2] ipv4: Remove all RTCF_DIRECTSRC handliing.
From: David Miller @ 2012-07-23 20:23 UTC (permalink / raw)
  To: netdev


The last and final kernel user, ICMP address replies,
has been removed.

Signed-off-by: David S. Miller <davem@davemloft.net>
---
 net/ipv4/route.c |   11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 9add088..34017be 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1391,9 +1391,6 @@ static int __mkroute_input(struct sk_buff *skb,
 		goto cleanup;
 	}
 
-	if (err)
-		flags |= RTCF_DIRECTSRC;
-
 	if (out_dev == in_dev && err &&
 	    (IN_DEV_SHARED_MEDIA(out_dev) ||
 	     inet_addr_onlink(out_dev, saddr, FIB_RES_GW(*res))))
@@ -1416,7 +1413,7 @@ static int __mkroute_input(struct sk_buff *skb,
 
 	do_cache = false;
 	if (res->fi) {
-		if (!(flags & RTCF_DIRECTSRC) && !itag) {
+		if (!itag) {
 			rth = FIB_RES_NH(*res).nh_rth_input;
 			if (rt_cache_valid(rth)) {
 				dst_hold(&rth->dst);
@@ -1558,8 +1555,6 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 					  dev, in_dev, &itag);
 		if (err < 0)
 			goto martian_source_keep_err;
-		if (err)
-			flags |= RTCF_DIRECTSRC;
 		goto local_input;
 	}
 
@@ -1580,8 +1575,6 @@ brd_input:
 					  in_dev, &itag);
 		if (err < 0)
 			goto martian_source_keep_err;
-		if (err)
-			flags |= RTCF_DIRECTSRC;
 	}
 	flags |= RTCF_BROADCAST;
 	res.type = RTN_BROADCAST;
@@ -1590,7 +1583,7 @@ brd_input:
 local_input:
 	do_cache = false;
 	if (res.fi) {
-		if (!(flags & RTCF_DIRECTSRC) && !itag) {
+		if (!itag) {
 			rth = FIB_RES_NH(res).nh_rth_input;
 			if (rt_cache_valid(rth)) {
 				dst_hold(&rth->dst);
-- 
1.7.10.4

^ permalink raw reply related

* Re: [PATCH] mlx4: Add support for EEH error recovery
From: Kleber Sacilotto de Souza @ 2012-07-23 20:53 UTC (permalink / raw)
  To: Or Gerlitz
  Cc: David Miller, netdev, jackm, yevgenyp, cascardo, brking, shlomop
In-Reply-To: <500D93F5.4090305@linux.vnet.ibm.com>

On 07/23/2012 03:12 PM, Kleber Sacilotto de Souza wrote:

> On 07/23/2012 10:45 AM, Or Gerlitz wrote:
> 
>> On 7/23/2012 4:18 PM, Kleber Sacilotto de Souza wrote:
>>> Exactly. The callbacks implemented are from standard PCI error recovery
>>> (Documentation/PCI/pci-error-recovery.txt) and the changes doesn't
>>> assume any platform in specific. The code was tested only on powerpc
>>> systems [...]
>>
>> So how did you test that? using the kernel provided error injection
>> support and user space tool (which?) or in another way? we've trying
>> quickly here to inject errors using /sbin/ear-inject from
>> ras-utils-6.1-1.el6.x86_64 on a kernel built with
>>
>> CONFIG_PCIEAER=y
>> CONFIG_PCIEAER_INJECT=m
> 
> 
> For powerpc we have an IBM internal user space tool that injects the
> error on the bus with the aid of the system firmware. The kernel used
> was built with the option:
> 
> CONFIG_EEH=y
> 
> and without the AER options. I will run some more tests with the AER
> options activated.


I tested the powerpc error injection with

CONFIG_EEH=y
CONFIG_PCIEAER=y
CONFIG_PCIEAER_INJECT=m

and with the aer_inject module loaded and it didn't affect the EEH
recovery, the adapter recovered as expected.

> 
>>
>> and it failed to inject errors, SB details.
>>
>> Or.
>>> since I don't have any mlx4 card on other platforms, however,
>>> these changes shouldn't make the error recover any worse than the
>>> current state.
>>
>>> # lspci | grep 08.00.1
>>> 08:00.1 Ethernet controller: Intel Corporation 82575EB Gigabit Network
>>> Connection (rev 02)
>>
>>> # cat /tmp/intel.aer
>>> AER
>>> BUS 8 DEV 0 FN 1
>>> COR_STATUS BAD_TLP
>>> HEADER_LOG 0 1 2 3
>>
>>> # /sbin/aer-inject < /tmp/intel.aer
>>> Error: Failed to write, Invalid argument
>>
>>
>>
>>> # strace -F -f /sbin/aer-inject < /tmp/intel.aer
>>> [...]
>>
>>> open("/dev/aer_inject", O_WRONLY)       = 3
>>> write(3, "\10\0\1\0\0\0\0\0@\0\0\0\0\0\0\0\1\0\0\0\2\0\0\0\3\0\0\0",
>>> 28) = -1 EINVAL (Invalid argument)
>>> write(2, "Error: ", 7Error: )                  = 7
>>> write(2, "Failed to write", 15Failed to write)         = 15
>>> write(2, ", Invalid argument\n", 19, Invalid argument
>>> )    = 19
>>> exit_group(-1)                          = ?
>>
>>
>>
>>
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> 
> 



-- 
Kleber Sacilotto de Souza
IBM Linux Technology Center

^ 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