* Re: [RFC] Zero-length write() does not generate a datagram on connected socket
From: David Miller @ 2007-09-27 20:53 UTC (permalink / raw)
To: shemminger; +Cc: netdev
In-Reply-To: <20070924153435.14249225@freepuppy.rosehill>
From: Stephen Hemminger <shemminger@linux-foundation.org>
Date: Mon, 24 Sep 2007 15:34:35 -0700
> The bug http://bugzilla.kernel.org/show_bug.cgi?id=5731
> describes an issue where write() can't be used to generate a zero-length
> datagram (but send, and sendto do work).
>
> I think the following is needed:
>
> --- a/net/socket.c 2007-08-20 09:54:28.000000000 -0700
> +++ b/net/socket.c 2007-09-24 15:31:25.000000000 -0700
> @@ -777,8 +777,11 @@ static ssize_t sock_aio_write(struct kio
> if (pos != 0)
> return -ESPIPE;
>
> - if (iocb->ki_left == 0) /* Match SYS5 behaviour */
> - return 0;
> + if (unlikely(iocb->ki_left == 0)) {
> + struct socket *sock = iocb->ki_filp->private_data;
> + if (sock->type == SOCK_STREAM)
> + return 0;
> + }
>
> x = alloc_sock_iocb(iocb, &siocb);
> if (!x)
We should simply remove the check completely.
There is no need to add special code for different types of protocols
and sockets.
As is hinted in the bugzilla, the exact same thing can happen with a
suitably constructed sendto() or sendmsg() call. write() on a socket
is a sendmsg() with a NULL msg_control and a single entry iovec, plain
and simple.
It's how BSD and many other systems behave, and I double checked
Steven's Volume 2 just to make sure.
So I'm going to check in the following to fix this bugzilla. There is
a similarly ugly test for len==0 in sys_read() on sockets. If someone
would do some research on the validity of that thing I'd really
appreciate it :-)
diff --git a/net/socket.c b/net/socket.c
index 7d44453..b09eb90 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -777,9 +777,6 @@ static ssize_t sock_aio_write(struct kiocb *iocb, const struct iovec *iov,
if (pos != 0)
return -ESPIPE;
- if (iocb->ki_left == 0) /* Match SYS5 behaviour */
- return 0;
-
x = alloc_sock_iocb(iocb, &siocb);
if (!x)
return -ENOMEM;
^ permalink raw reply related
* Re: [PATCH 1/4] net: Dynamically allocate the per cpu counters for the loopback device.
From: Eric W. Biederman @ 2007-09-27 20:44 UTC (permalink / raw)
To: David Miller; +Cc: netdev, containers
In-Reply-To: <20070927.115200.56183114.davem@davemloft.net>
David Miller <davem@davemloft.net> writes:
> From: ebiederm@xmission.com (Eric W. Biederman)
> Date: Thu, 27 Sep 2007 01:48:00 -0600
>
>> I'm not doing get_cpu/put_cpu so does the comment make sense
>> in relationship to per_cpu_ptr?
>
> It is possible. But someone would need to go check for
> sure.
Verified.
hard_start_xmit is called inside of a
rcu_read_lock_bh(),rcu_read_unlock_bh() pair. Which means
the code will only run on one cpu.
Therefore we do not need get_cpu/put_cpu.
In addition per_cpu_ptr is valid. As it is just a lookup
into a NR_CPUS sized array by smp_processor_id() to return
the address of the specific cpu.
The only difference between per_cpu_ptr and __get_cpu_var()
are the implementation details between statically allocated
and dynamically allocated per cpu state.
So the comment is still valid, and still interesting it just
should say per_cpu_ptr instead of __get_cpu_var.
Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
---
diff --git a/drivers/net/loopback.c b/drivers/net/loopback.c
index 0f9d8c6..756e267 100644
--- a/drivers/net/loopback.c
+++ b/drivers/net/loopback.c
@@ -154,7 +154,7 @@ static int loopback_xmit(struct sk_buff *skb, struct net_device *dev)
#endif
dev->last_rx = jiffies;
- /* it's OK to use __get_cpu_var() because BHs are off */
+ /* it's OK to use per_cpu_ptr() because BHs are off */
pcpu_lstats = netdev_priv(dev);
lb_stats = per_cpu_ptr(pcpu_lstats, smp_processor_id());
lb_stats->bytes += skb->len;
^ permalink raw reply related
* RE: [ofa-general] [PATCH v3] iw_cxgb3: Support"iwarp-only"interfacesto avoid 4-tuple conflicts.
From: Kanevsky, Arkady @ 2007-09-27 20:19 UTC (permalink / raw)
To: Sean Hefty, Sean Hefty, Steve Wise; +Cc: netdev, rdreier, linux-kernel, general
In-Reply-To: <000101c8013a$41b374f0$a7cc180a@amr.corp.intel.com>
Sean,
IB aside,
it looks like an ULP which is capable of being both RDMA aware and RDMA
not-aware,
like iSER and iSCSI, NFS-RDMA and NFS, SDP and sockets,
will be treated as two separete ULPs.
Each has its own IP address, since there is a different IP address for
iWARP
port and "regular" Ethernet port. So it falls on the users of ULPs to
"handle" it
via DNS or some other services.
Is this "acceptable" to users? I doubt it.
Recall that ULPs are going in opposite directions by having a different
port number for RDMA aware and RDMA unaware versions of the ULP.
This way, ULP "connection manager" handles RDMA-ness under the covers,
while users plug an IP address for a server to connect to.
Thanks,
Arkady Kanevsky email: arkady@netapp.com
Network Appliance Inc. phone: 781-768-5395
1601 Trapelo Rd. - Suite 16. Fax: 781-895-1195
Waltham, MA 02451 central phone: 781-768-5300
> -----Original Message-----
> From: Sean Hefty [mailto:sean.hefty@intel.com]
> Sent: Thursday, September 27, 2007 3:12 PM
> To: Kanevsky, Arkady; Sean Hefty; Steve Wise
> Cc: netdev@vger.kernel.org; rdreier@cisco.com;
> linux-kernel@vger.kernel.org; general@lists.openfabrics.org
> Subject: RE: [ofa-general] [PATCH v3] iw_cxgb3:
> Support"iwarp-only"interfacesto avoid 4-tuple conflicts.
>
> >What is the model on how client connects, say for iSCSI, when client
> >and server both support, iWARP and 10GbE or 1GbE, and would like to
> >setup "most" performant "connection" for ULP?
>
> For the "most" performance connection, the ULP would use IB,
> and all these problems go away. :)
>
> This proposal is for each iwarp interface to have its own IP
> address. Clients would need an iwarp usable address of the
> server and would connect using rdma_connect(). If that call
> (or rdma_resolve_addr/route) fails, the client could try
> connecting using sockets, aoi, or some other interface. I
> don't see that Steve's proposal changes anything from the
> client's perspective.
>
> - Sean
> _______________________________________________
> general mailing list
> general@lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>
> To unsubscribe, please visit
> http://openib.org/mailman/listinfo/openib-general
>
^ permalink raw reply
* RE: [ofa-general] [PATCH v3] iw_cxgb3: Support "iwarp-only" interfaces to avoid 4-tuple conflicts.
From: Sean Hefty @ 2007-09-27 20:14 UTC (permalink / raw)
To: 'Steve Wise', Sean Hefty; +Cc: rdreier, netdev, linux-kernel, general
In-Reply-To: <46FC03B8.1030106@opengridcomputing.com>
>It is ok to block while holding a mutex, yes?
It's okay, I just didn't try to trace through the code to see if it ever tries
to acquire the same mutex in the thread that needs to signal the event.
- Sean
^ permalink raw reply
* Re: [PKT_SCHED]: Add stateless NAT
From: David Miller @ 2007-09-27 19:52 UTC (permalink / raw)
To: kaber; +Cc: johnpol, herbert, netdev, kuznet, hadi
In-Reply-To: <46FBB296.8020901@trash.net>
From: Patrick McHardy <kaber@trash.net>
Date: Thu, 27 Sep 2007 15:39:34 +0200
> Evgeniy Polyakov wrote:
> > On Thu, Sep 27, 2007 at 09:20:37PM +0800, Herbert Xu (herbert@gondor.apana.org.au) wrote:
> >
> >>How about putting it in net/core/utils.c?
> >
> >
> > I knew, that was a bad idea to try to fix netfilter dependency :)
> >
> > diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
>
>
> This looks good to me.
I still think the nf_*() prefixes should all go and the extern
prototypes should go into an independant header file.
These are not netfilter routines, they are "INET" helpers.
And we should make similar treatment for all of the ipv6
packet parser helper functions that ipv6 netfilter needs.
^ permalink raw reply
* Re: [PKT_SCHED]: Add stateless NAT
From: David Miller @ 2007-09-27 19:48 UTC (permalink / raw)
To: herbert; +Cc: hadi, netdev, kuznet
In-Reply-To: <20070927125801.GB16706@gondor.apana.org.au>
From: Herbert Xu <herbert@gondor.apana.org.au>
Date: Thu, 27 Sep 2007 20:58:01 +0800
> On Thu, Sep 27, 2007 at 08:39:45AM -0400, jamal wrote:
> >
> > You also need to p->tcf_qstats.drops++ for all packets that get shot.
>
> I was rather hoping that my packets wouldn't get shot :)
> But yeah let's increment the drops counter for consistency.
>
> [PKT_SCHED]: Add stateless NAT
Applied to net-2.6.24, thanks Herbert!
^ permalink raw reply
* Re: [PATCH] net: Add network namespace clone & unshare support.
From: David Miller @ 2007-09-27 19:45 UTC (permalink / raw)
To: ebiederm; +Cc: clg, containers, netdev, akpm
In-Reply-To: <m1k5qcjaiu.fsf@ebiederm.dsl.xmission.com>
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Thu, 27 Sep 2007 11:14:33 -0600
> Thanks for pointing this out, it's on my todo list to look into,
> and ensure we resolve.
>
> I'm confused because my notes have 0x80000000 for the pid namespace,
> and 0x40000000 for the time namespace.
Eric, pick an appropriate new non-conflicting number NOW.
This adds unnecessary extra work for Andrew Morton, which he has
enough of already.
^ permalink raw reply
* Re: [PATCH] various dst_ifdown routines to catch refcounting bugs
From: David Miller @ 2007-09-27 19:44 UTC (permalink / raw)
To: ebiederm; +Cc: den, devel, netdev, containers
In-Reply-To: <m1myv8kr9c.fsf@ebiederm.dsl.xmission.com>
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Thu, 27 Sep 2007 10:27:43 -0600
> "Denis V. Lunev" <den@openvz.org> writes:
>
> > Moving dst entries into init_net.loopback_dev is not a good thing.
> > This hides obvious and non-obvious ref-counting bugs.
>
> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
Patch applied.
> I do have a question I would like to bring up, because I like avoiding
> explicit references to loopback_dev when I can.
>
> /* Dirty hack. We did it in 2.2 (in __dst_free),
> * we have _very_ good reasons not to repeat
> * this mistake in 2.3, but we have no choice
> * now. _It_ _is_ _explicit_ _deliberate_
> * _race_ _condition_.
> *
> * Commented and originally written by Alexey.
> */
>
> What is the race that is talked about in that comment. Can we just
> assign NULL instead of the loopback device when we bring a route down.
> My gut feeling is that something like:
> dst->input = dst->output = dst_discard;
> may be enough. But I don't know where the deliberate race is.
The packet output path accesses the cached route device
asynchronously, and we are resetting the device to be loopback without
any synchronization whatsoever. None is in fact possible, and we
don't want to add it because that would be way too expensive.
So another thread on the system can either see the original device or
the loopback one.
It all works out because as the device goes down we'll purge any
packets queued into the transmit queue and packet scheduler for that
device.
^ permalink raw reply
* [PATCH 2/2] sky2: fix transmit state on resume
From: Stephen Hemminger @ 2007-09-27 19:38 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev
In-Reply-To: <20070926175847.706025d0@freepuppy.rosehill>
This should fix http://bugzilla.kernel.org/show_bug.cgi?id=8667
After resume, driver has reset the chip so the current state
of transmit checksum offload state machine and DMA state machine
will be undefined.
The fix is to set the state so that first Tx will set MSS and offset
values.
Patch is against 2.6.23-rc8 after last patch:
sky2: FE+ vlan workaround
(Should also work on older releases with minor fuzz).
Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
--- a/drivers/net/sky2.c 2007-09-27 08:45:13.000000000 -0700
+++ b/drivers/net/sky2.c 2007-09-27 09:39:49.000000000 -0700
@@ -910,6 +910,20 @@ static inline struct sky2_tx_le *get_tx_
return le;
}
+static void tx_init(struct sky2_port *sky2)
+{
+ struct sky2_tx_le *le;
+
+ sky2->tx_prod = sky2->tx_cons = 0;
+ sky2->tx_tcpsum = 0;
+ sky2->tx_last_mss = 0;
+
+ le = get_tx_le(sky2);
+ le->addr = 0;
+ le->opcode = OP_ADDR64 | HW_OWNER;
+ sky2->tx_addr64 = 0;
+}
+
static inline struct tx_ring_info *tx_le_re(struct sky2_port *sky2,
struct sky2_tx_le *le)
{
@@ -1320,7 +1334,8 @@ static int sky2_up(struct net_device *de
GFP_KERNEL);
if (!sky2->tx_ring)
goto err_out;
- sky2->tx_prod = sky2->tx_cons = 0;
+
+ tx_init(sky2);
sky2->rx_le = pci_alloc_consistent(hw->pdev, RX_LE_BYTES,
&sky2->rx_le_map);
^ permalink raw reply
* Re: TCP Spike
From: Ilpo Järvinen @ 2007-09-27 19:34 UTC (permalink / raw)
To: Majumder, Rajib
Cc: 'netdev@vger.kernel.org',
'linux-kernel@vger.kernel.org'
In-Reply-To: <F444CAE5E62A714C9F45AA292785BED3223F7283@esng11p33001.sg.csfb.com>
On Thu, 27 Sep 2007, Majumder, Rajib wrote:
> We have observed 40ms latency spikes in TCP connections in "burst" type of traffic.
> This affects regular TCP sockets.
Are segments being sent full-sized, or is there perhaps some Nagle
component in it as well? I.e., are the applications using TCP_NODELAY?
> We observed this issue in kernels of 2.4.21 and kernel 2.6.5.
>
> Aparently, this seems to be fixed in 2.6.19.
>
> Can someone throw some light on this?
I think somebody, probably Alexey, enabled sending of ACK on every 2nd
segment. Previously small segment senders playing with Nagle were
complaining every now and then about performance because two small
segments did not generate ACKs but one had to accumulate, IIRC, half MSS
worth of data before ACK was sent. Could this be related to your case?
...In case you're having too much time, you can always try bisecting it
which finds out the causing commit... :-)
> Is this a congestion control/avoidance issue?
Congestion control is basically ACK clocked math for cwnd, ssthresh, etc.
state, which then results in permission to send new segments out etc.
(except for RTO part of course, which I'll ignore in the next statement).
Any delay gaps to sent packet after ACK receival, which triggered the
state changing math, isn't there due to congestion control but due to
other factors! 40ms is much below MIN_RTO (200ms), so it shouldn't be due
to RTO either... Note that also delayed ACKs are exception to the general
rule.
Congestion control is controlled like your CPU is. In your CPU there's
this whatever GHz clock which determines when the state changing events
take place, state changes don't just happen arbitarily but are _clocked_
(ACK _clocked_ in case of congestion control). Of course there will be
some propagation delay after the change to put in effect all the state
changes that are result of what occurred at clock edge (and this delay
assimilating to processing delay in the context of congestion control).
--
i.
^ permalink raw reply
* [PATCH 1/2] sky2: FE+ vlan workaround
From: Stephen Hemminger @ 2007-09-27 19:32 UTC (permalink / raw)
To: Jeff Garzik; +Cc: netdev
In-Reply-To: <20070926175847.706025d0@freepuppy.rosehill>
The FE+ workaround means the driver can no longer trust the status register
to indicate VLAN tagged frames. The fix for this is to just disable VLAN
acceleration for that chip version. Tested and works fine.
This patch applies to 2.6.23-rc8 after yesterday's patch:
sky2 FE+ receive status workaround
Signed-off-by: Stephen Hemminger <shemminger@linux-foundation.org>
--- a/drivers/net/sky2.c 2007-09-27 08:45:13.000000000 -0700
+++ b/drivers/net/sky2.c 2007-09-27 09:43:15.000000000 -0700
@@ -3970,8 +3970,12 @@ static __devinit struct net_device *sky2
dev->features |= NETIF_F_HIGHDMA;
#ifdef SKY2_VLAN_TAG_USED
- dev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
- dev->vlan_rx_register = sky2_vlan_rx_register;
+ /* The workaround for FE+ status conflicts with VLAN tag detection. */
+ if (!(sky2->hw->chip_id == CHIP_ID_YUKON_FE_P &&
+ sky2->hw->chip_rev == CHIP_REV_YU_FE2_A0)) {
+ dev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX;
+ dev->vlan_rx_register = sky2_vlan_rx_register;
+ }
#endif
/* read the mac address */
^ permalink raw reply
* Re: [ofa-general] [PATCH v3] iw_cxgb3: Support "iwarp-only" interfaces to avoid 4-tuple conflicts.
From: Steve Wise @ 2007-09-27 19:25 UTC (permalink / raw)
To: Sean Hefty; +Cc: rdreier, sean.hefty, netdev, linux-kernel, general
In-Reply-To: <46FBF8AF.9040700@ichips.intel.com>
Sean Hefty wrote:
>> The sysadmin creates "for iwarp use only" alias interfaces of the form
>> "devname:iw*" where devname is the native interface name (eg eth0) for
>> the
>> iwarp netdev device. The alias label can be anything starting with "iw".
>> The "iw" immediately after the ':' is the key used by the iw_cxgb3
>> driver.
>
> I'm still not sure about this, but haven't come up with anything better
> myself. And if there's a good chance of other rnic's needing the same
> support, I'd rather see the common code separated out, even if just
> encapsulated within this module for easy re-use.
>
> As for the code, I have a couple of questions about whether deadlock and
> a race condition are possible, plus a few minor comments.
>
Thanks for reviewing! Responses are in-line below.
>> +static void insert_ifa(struct iwch_dev *rnicp, struct in_ifaddr *ifa)
>> +{
>> + struct iwch_addrlist *addr;
>> +
>> + addr = kmalloc(sizeof *addr, GFP_KERNEL);
>> + if (!addr) {
>> + printk(KERN_ERR MOD "%s - failed to alloc memory!\n",
>> + __FUNCTION__);
>> + return;
>> + }
>> + addr->ifa = ifa;
>> + mutex_lock(&rnicp->mutex);
>> + list_add_tail(&addr->entry, &rnicp->addrlist);
>> + mutex_unlock(&rnicp->mutex);
>> +}
>
> Should this return success/failure?
>
I think so. See below...
>> +static int nb_callback(struct notifier_block *self, unsigned long event,
>> + void *ctx)
>> +{
>> + struct in_ifaddr *ifa = ctx;
>> + struct iwch_dev *rnicp = container_of(self, struct iwch_dev, nb);
>> +
>> + PDBG("%s rnicp %p event %lx\n", __FUNCTION__, rnicp, event);
>> +
>> + switch (event) {
>> + case NETDEV_UP:
>> + if (netdev_is_ours(rnicp, ifa->ifa_dev->dev) &&
>> + is_iwarp_label(ifa->ifa_label)) {
>> + PDBG("%s label %s addr 0x%x added\n",
>> + __FUNCTION__, ifa->ifa_label, ifa->ifa_address);
>> + insert_ifa(rnicp, ifa);
>> + iwch_listeners_add_addr(rnicp, ifa->ifa_address);
>
> If insert_ifa() fails, what will iwch_listeners_add_addr() do? (I'm not
> easily seeing the relationship between the address list and the listen
> list at this point.)
>
I guess insert_ifa() needs to return success/failure. Then if we failed
to add the ifa to the list we won't update the listeners.
The relationship is this:
- when a listen is done on addr 0.0.0.0, the code walks the list of
addresses to do specific listens on each address.
- when an address is added or deleted, then the list of current
listeners is walked and updated accordingly.
>> + }
>> + break;
>> + case NETDEV_DOWN:
>> + if (netdev_is_ours(rnicp, ifa->ifa_dev->dev) &&
>> + is_iwarp_label(ifa->ifa_label)) {
>> + PDBG("%s label %s addr 0x%x deleted\n",
>> + __FUNCTION__, ifa->ifa_label, ifa->ifa_address);
>> + iwch_listeners_del_addr(rnicp, ifa->ifa_address);
>> + remove_ifa(rnicp, ifa);
>> + }
>> + break;
>> + default:
>> + break;
>> + }
>> + return 0;
>> +}
>> +
>> +static void delete_addrlist(struct iwch_dev *rnicp)
>> +{
>> + struct iwch_addrlist *addr, *tmp;
>> +
>> + mutex_lock(&rnicp->mutex);
>> + list_for_each_entry_safe(addr, tmp, &rnicp->addrlist, entry) {
>> + list_del(&addr->entry);
>> + kfree(addr);
>> + }
>> + mutex_unlock(&rnicp->mutex);
>> +}
>> +
>> +static void populate_addrlist(struct iwch_dev *rnicp)
>> +{
>> + int i;
>> + struct in_device *indev;
>> +
>> + for (i = 0; i < rnicp->rdev.port_info.nports; i++) {
>> + indev = in_dev_get(rnicp->rdev.port_info.lldevs[i]);
>> + if (!indev)
>> + continue;
>> + for_ifa(indev)
>> + if (is_iwarp_label(ifa->ifa_label)) {
>> + PDBG("%s label %s addr 0x%x added\n",
>> + __FUNCTION__, ifa->ifa_label,
>> + ifa->ifa_address);
>> + insert_ifa(rnicp, ifa);
>> + }
>> + endfor_ifa(indev);
>> + }
>> +}
>> +
>> static void rnic_init(struct iwch_dev *rnicp)
>> {
>> PDBG("%s iwch_dev %p\n", __FUNCTION__, rnicp);
>> @@ -70,6 +187,12 @@ static void rnic_init(struct iwch_dev *r
>> idr_init(&rnicp->qpidr);
>> idr_init(&rnicp->mmidr);
>> spin_lock_init(&rnicp->lock);
>> + INIT_LIST_HEAD(&rnicp->addrlist);
>> + INIT_LIST_HEAD(&rnicp->listen_eps);
>> + mutex_init(&rnicp->mutex);
>> + rnicp->nb.notifier_call = nb_callback;
>> + populate_addrlist(rnicp);
>> + register_inetaddr_notifier(&rnicp->nb);
>>
>> rnicp->attr.vendor_id = 0x168;
>> rnicp->attr.vendor_part_id = 7;
>> @@ -148,6 +271,8 @@ static void close_rnic_dev(struct t3cdev
>> mutex_lock(&dev_mutex);
>> list_for_each_entry_safe(dev, tmp, &dev_list, entry) {
>> if (dev->rdev.t3cdev_p == tdev) {
>> + unregister_inetaddr_notifier(&dev->nb);
>> + delete_addrlist(dev);
>> list_del(&dev->entry);
>> iwch_unregister_device(dev);
>> cxio_rdev_close(&dev->rdev);
>> diff --git a/drivers/infiniband/hw/cxgb3/iwch.h
>> b/drivers/infiniband/hw/cxgb3/iwch.h
>> index caf4e60..7fa0a47 100644
>> --- a/drivers/infiniband/hw/cxgb3/iwch.h
>> +++ b/drivers/infiniband/hw/cxgb3/iwch.h
>> @@ -36,6 +36,8 @@ #include <linux/mutex.h>
>> #include <linux/list.h>
>> #include <linux/spinlock.h>
>> #include <linux/idr.h>
>> +#include <linux/notifier.h>
>> +#include <linux/inetdevice.h>
>>
>> #include <rdma/ib_verbs.h>
>>
>> @@ -101,6 +103,11 @@ struct iwch_rnic_attributes {
>> u32 cq_overflow_detection;
>> };
>>
>> +struct iwch_addrlist {
>> + struct list_head entry;
>> + struct in_ifaddr *ifa;
>> +};
>> +
>> struct iwch_dev {
>> struct ib_device ibdev;
>> struct cxio_rdev rdev;
>> @@ -111,6 +118,10 @@ struct iwch_dev {
>> struct idr mmidr;
>> spinlock_t lock;
>> struct list_head entry;
>> + struct notifier_block nb;
>> + struct list_head addrlist;
>> + struct list_head listen_eps;
>
> The behavior of the listen lists is similar to what's done in the
> rdma_cm: Wildcard listens are stored in a listen_any_list. When new
> devices are added, associated listens are added to each device. This is
> similar, except we're dealing with devices and addresses. I'm wondering
> if we shouldn't mimic the same behavior and track listens in
> iwch_addrlist directly. (I don't see anything wrong with this approach
> though.)
>
> What happens if an address changes between iwarp only and non-iwarp?
>
That results in a NETDEV_DOWN event indicating the iwarp only address is
getting deleted. All the affected listening endpoints are updated to
stop listening on that address. A NETDEV_UP event would happen when the
ipaddress is switched over to the TCP interface, but our callback
function ignores this since the interface name is not ethX:iw.
> How are listens on specific addresses handled from an rdma_cm level?
> Does the rdma_cm map the address to the device, call the iw_cm to
> listen, which in turn calls the device listen function?
Yes.
> The device then
> checks that the address has been marked as iwarp only? (I'm being too
> lazy to trace this through the code, but if you don't know off the top
> of your head, I will do that.)
Actually, I don't enforce this. If the app explicitly binds/listens to
a non-iwarp address, then the code happily allows it. I could fail this
case though. That would be best I guess.
>
>> + struct mutex mutex;
>> };
>>
>> static inline struct iwch_dev *to_iwch_dev(struct ib_device *ibdev)
>> diff --git a/drivers/infiniband/hw/cxgb3/iwch_cm.c
>> b/drivers/infiniband/hw/cxgb3/iwch_cm.c
>> index 1cdfcd4..afc8a48 100644
>> --- a/drivers/infiniband/hw/cxgb3/iwch_cm.c
>> +++ b/drivers/infiniband/hw/cxgb3/iwch_cm.c
>> @@ -1127,23 +1127,149 @@ static int act_open_rpl(struct t3cdev *t
>> return CPL_RET_BUF_DONE;
>> }
>>
>> -static int listen_start(struct iwch_listen_ep *ep)
>> +static int wait_for_reply(struct iwch_ep_common *epc)
>> +{
>> + PDBG("%s ep %p waiting\n", __FUNCTION__, epc);
>> + wait_event(epc->waitq, epc->rpl_done);
>> + PDBG("%s ep %p done waiting err %d\n", __FUNCTION__, epc,
>> epc->rpl_err);
>> + return epc->rpl_err;
>> +}
>
> What thread is being blocked here, and what sets the event?
>
The thread calling rdma_listen() gets blocked here until the rnic posts
a response to the listen request. Ditto for rdma_destroy_id() on a
listening endpoint. The event is set and the wakeup don in
pass_open_rpl() and close_listsrv_rpl().
>> +
>> +static struct iwch_listen_entry *alloc_listener(struct iwch_listen_ep
>> *ep,
>> + __be32 addr)
>> +{
>> + struct iwch_dev *h = to_iwch_dev(ep->com.cm_id->device);
>> + struct iwch_listen_entry *le;
>> +
>> + le = kmalloc(sizeof *le, GFP_KERNEL);
>> + if (!le) {
>> + printk(KERN_ERR MOD "%s - failed to alloc memory!\n",
>> + __FUNCTION__);
>> + return NULL;
>> + }
>> + le->stid = cxgb3_alloc_stid(h->rdev.t3cdev_p,
>> + &t3c_client, ep);
>> + if (le->stid == -1) {
>> + printk(KERN_ERR MOD "%s - cannot alloc stid.\n",
>> + __FUNCTION__);
>> + kfree(le);
>> + return NULL;
>> + }
>> + le->addr = addr;
>> + PDBG("%s stid %u addr %x port %x\n", __FUNCTION__, le->stid,
>> + ntohl(le->addr), ntohs(ep->com.local_addr.sin_port));
>> + return le;
>> +}
>> +
>> +static void dealloc_listener(struct iwch_listen_ep *ep,
>> + struct iwch_listen_entry *le)
>> +{
>> + PDBG("%s stid %u addr %x port %x\n", __FUNCTION__, le->stid,
>> + ntohl(le->addr), ntohs(ep->com.local_addr.sin_port));
>> + cxgb3_free_stid(ep->com.tdev, le->stid);
>> + kfree(le);
>> +}
>> +
>> +static void dealloc_listener_list(struct iwch_listen_ep *ep)
>> +{
>> + struct iwch_listen_entry *le, *tmp;
>> + struct iwch_dev *h = to_iwch_dev(ep->com.cm_id->device);
>> +
>> + mutex_lock(&h->mutex);
>> + list_for_each_entry_safe(le, tmp, &ep->listeners, entry) {
>> + list_del(&le->entry);
>> + dealloc_listener(ep, le);
>> + }
>> + mutex_unlock(&h->mutex);
>> +}
>> +
>> +static int alloc_listener_list(struct iwch_listen_ep *ep)
>> +{
>> + struct iwch_dev *h = to_iwch_dev(ep->com.cm_id->device);
>
> nit: in place of 'h' in several places, how about 'rnicp', which is also
> used?
>
>> + struct iwch_addrlist *addr;
>> + struct iwch_listen_entry *le;
>> + int err = 0;
>> + int added=0;
>> + mutex_lock(&h->mutex);
>> + list_for_each_entry(addr, &h->addrlist, entry) {
>> + if (ep->com.local_addr.sin_addr.s_addr == 0 ||
>> + ep->com.local_addr.sin_addr.s_addr ==
>> + addr->ifa->ifa_address) {
>> + le = alloc_listener(ep, addr->ifa->ifa_address);
>> + if (!le)
>> + break;
>> + list_add_tail(&le->entry, &ep->listeners);
>> + added++;
>> + }
>> + }
>> + mutex_unlock(&h->mutex);
>> + if (ep->com.local_addr.sin_addr.s_addr != 0 && !added)
>> + err = -EADDRNOTAVAIL;
>> + if (!err && !added)
>> + printk(KERN_WARNING MOD
>> + "No RDMA interface found for device %s\n",
>> + pci_name(h->rdev.rnic_info.pdev));
>> + return err;
>> +}
>
> Adding some white space would improve readability.
>
>> +
>> +static int listen_stop_one(struct iwch_listen_ep *ep, unsigned int
>> stid)
>> {
>> struct sk_buff *skb;
>> - struct cpl_pass_open_req *req;
>> + struct cpl_close_listserv_req *req;
>> +
>> + PDBG("%s stid %u\n", __FUNCTION__, stid);
>> + skb = get_skb(NULL, sizeof(*req), GFP_KERNEL);
>> + if (!skb) {
>> + printk(KERN_ERR MOD "%s - failed to alloc skb\n", __FUNCTION__);
>> + return -ENOMEM;
>> + }
>> + req = (struct cpl_close_listserv_req *) skb_put(skb, sizeof(*req));
>> + req->wr.wr_hi = htonl(V_WR_OP(FW_WROPCODE_FORWARD));
>> + req->cpu_idx = 0;
>> + OPCODE_TID(req) = htonl(MK_OPCODE_TID(CPL_CLOSE_LISTSRV_REQ, stid));
>> + skb->priority = 1;
>> + ep->com.rpl_err = 0;
>> + ep->com.rpl_done = 0;
>> + cxgb3_ofld_send(ep->com.tdev, skb);
>> + return wait_for_reply(&ep->com);
>> +}
>> +
>> +static int listen_stop(struct iwch_listen_ep *ep)
>> +{
>> + struct iwch_listen_entry *le;
>> + struct iwch_dev *h = to_iwch_dev(ep->com.cm_id->device);
>> + int err = 0;
>>
>> PDBG("%s ep %p\n", __FUNCTION__, ep);
>> + mutex_lock(&h->mutex);
>> + list_for_each_entry(le, &ep->listeners, entry) {
>> + err = listen_stop_one(ep, le->stid);
>
> This ends up blocking while holding a mutex, which looks like deadlock
> potential.
>
I don't think there are any deadlocks. I don't know how to avoid
blocking while holding the mutex. But its ok, I think.
>> + if (err)
>> + break;
>> + }
>> + mutex_unlock(&h->mutex);
>> + return err;
>> +}
>> +
>> +static int listen_start_one(struct iwch_listen_ep *ep, unsigned int
>> stid,
>> + __be32 addr, __be16 port)
>> +{
>> + struct sk_buff *skb;
>> + struct cpl_pass_open_req *req;
>> +
>> + PDBG("%s stid %u addr %x port %x\n", __FUNCTION__, stid,
>> ntohl(addr),
>> + ntohs(port));
>> skb = get_skb(NULL, sizeof(*req), GFP_KERNEL);
>> if (!skb) {
>> - printk(KERN_ERR MOD "t3c_listen_start failed to alloc skb!\n");
>> + printk(KERN_ERR MOD "%s - failed to alloc skb\n", __FUNCTION__);
>> return -ENOMEM;
>> }
>>
>> req = (struct cpl_pass_open_req *) skb_put(skb, sizeof(*req));
>> req->wr.wr_hi = htonl(V_WR_OP(FW_WROPCODE_FORWARD));
>> - OPCODE_TID(req) = htonl(MK_OPCODE_TID(CPL_PASS_OPEN_REQ, ep->stid));
>> - req->local_port = ep->com.local_addr.sin_port;
>> - req->local_ip = ep->com.local_addr.sin_addr.s_addr;
>> + OPCODE_TID(req) = htonl(MK_OPCODE_TID(CPL_PASS_OPEN_REQ, stid));
>> + req->local_port = port;
>> + req->local_ip = addr;
>> req->peer_port = 0;
>> req->peer_ip = 0;
>> req->peer_netmask = 0;
>> @@ -1152,8 +1278,32 @@ static int listen_start(struct iwch_list
>> req->opt1 = htonl(V_CONN_POLICY(CPL_CONN_POLICY_ASK));
>>
>> skb->priority = 1;
>> + ep->com.rpl_err = 0;
>> + ep->com.rpl_done = 0;
>> cxgb3_ofld_send(ep->com.tdev, skb);
>> - return 0;
>> + return wait_for_reply(&ep->com);
>> +}
>> +
>> +static int listen_start(struct iwch_listen_ep *ep)
>> +{
>> + struct iwch_listen_entry *le;
>> + struct iwch_dev *h = to_iwch_dev(ep->com.cm_id->device);
>> + int err = 0;
>> +
>> + PDBG("%s ep %p\n", __FUNCTION__, ep);
>> + mutex_lock(&h->mutex);
>> + list_for_each_entry(le, &ep->listeners, entry) {
>> + err = listen_start_one(ep, le->stid, le->addr,
>> + ep->com.local_addr.sin_port);
>
> Similar to above - blocking while holding a mutex. There are a couple
> of other places where this also occurs.
It is ok to block while holding a mutex, yes?
>
>> + if (err)
>> + goto fail;
>> + }
>> + mutex_unlock(&h->mutex);
>> + return err;
>> +fail:
>> + mutex_unlock(&h->mutex);
>> + listen_stop(ep);
>> + return err;
>> }
>>
>> static int pass_open_rpl(struct t3cdev *tdev, struct sk_buff *skb,
>> void *ctx)
>> @@ -1170,39 +1320,59 @@ static int pass_open_rpl(struct t3cdev *
>> return CPL_RET_BUF_DONE;
>> }
>>
>> -static int listen_stop(struct iwch_listen_ep *ep)
>> -{
>> - struct sk_buff *skb;
>> - struct cpl_close_listserv_req *req;
>> -
>> - PDBG("%s ep %p\n", __FUNCTION__, ep);
>> - skb = get_skb(NULL, sizeof(*req), GFP_KERNEL);
>> - if (!skb) {
>> - printk(KERN_ERR MOD "%s - failed to alloc skb\n", __FUNCTION__);
>> - return -ENOMEM;
>> - }
>> - req = (struct cpl_close_listserv_req *) skb_put(skb, sizeof(*req));
>> - req->wr.wr_hi = htonl(V_WR_OP(FW_WROPCODE_FORWARD));
>> - req->cpu_idx = 0;
>> - OPCODE_TID(req) = htonl(MK_OPCODE_TID(CPL_CLOSE_LISTSRV_REQ,
>> ep->stid));
>> - skb->priority = 1;
>> - cxgb3_ofld_send(ep->com.tdev, skb);
>> - return 0;
>> -}
>> -
>> static int close_listsrv_rpl(struct t3cdev *tdev, struct sk_buff *skb,
>> void *ctx)
>> {
>> struct iwch_listen_ep *ep = ctx;
>> struct cpl_close_listserv_rpl *rpl = cplhdr(skb);
>>
>> - PDBG("%s ep %p\n", __FUNCTION__, ep);
>> + PDBG("%s ep %p stid %u\n", __FUNCTION__, ep, GET_TID(rpl));
>> +
>> ep->com.rpl_err = status2errno(rpl->status);
>> ep->com.rpl_done = 1;
>> wake_up(&ep->com.waitq);
>> return CPL_RET_BUF_DONE;
>> }
>>
>> +void iwch_listeners_add_addr(struct iwch_dev *rnicp, __be32 addr)
>> +{
>> + struct iwch_listen_ep *listen_ep;
>> + struct iwch_listen_entry *le;
>> +
>> + mutex_lock(&rnicp->mutex);
>> + list_for_each_entry(listen_ep, &rnicp->listen_eps, entry) {
>> + if (listen_ep->com.local_addr.sin_addr.s_addr)
>> + continue;
>> + le = alloc_listener(listen_ep, addr);
>> + if (le) {
>> + list_add_tail(&le->entry, &listen_ep->listeners);
>> + listen_start_one(listen_ep, le->stid, addr,
>> + listen_ep->com.local_addr.sin_port);
>> + }
>> + }
>> + mutex_unlock(&rnicp->mutex);
>> +}
>> +
>> +void iwch_listeners_del_addr(struct iwch_dev *rnicp, __be32 addr)
>> +{
>> + struct iwch_listen_ep *listen_ep;
>> + struct iwch_listen_entry *le, *tmp;
>> +
>> + mutex_lock(&rnicp->mutex);
>> + list_for_each_entry(listen_ep, &rnicp->listen_eps, entry) {
>> + if (listen_ep->com.local_addr.sin_addr.s_addr)
>> + continue;
>> + list_for_each_entry_safe(le, tmp, &listen_ep->listeners,
>> + entry)
>> + if (le->addr == addr) {
>> + listen_stop_one(listen_ep, le->stid);
>> + list_del(&le->entry);
>> + dealloc_listener(listen_ep, le);
>> + }
>> + }
>> + mutex_unlock(&rnicp->mutex);
>> +}
>> +
>> static void accept_cr(struct iwch_ep *ep, __be32 peer_ip, struct
>> sk_buff *skb)
>> {
>> struct cpl_pass_accept_rpl *rpl;
>> @@ -1767,8 +1937,7 @@ int iwch_accept_cr(struct iw_cm_id *cm_i
>> goto err;
>>
>> /* wait for wr_ack */
>> - wait_event(ep->com.waitq, ep->com.rpl_done);
>> - err = ep->com.rpl_err;
>> + err = wait_for_reply(&ep->com);
>> if (err)
>> goto err;
>>
>> @@ -1887,31 +2056,23 @@ int iwch_create_listen(struct iw_cm_id *
>> ep->com.cm_id = cm_id;
>> ep->backlog = backlog;
>> ep->com.local_addr = cm_id->local_addr;
>> + INIT_LIST_HEAD(&ep->listeners);
>>
>> - /*
>> - * Allocate a server TID.
>> - */
>> - ep->stid = cxgb3_alloc_stid(h->rdev.t3cdev_p, &t3c_client, ep);
>> - if (ep->stid == -1) {
>> - printk(KERN_ERR MOD "%s - cannot alloc atid.\n", __FUNCTION__);
>> - err = -ENOMEM;
>> + err = alloc_listener_list(ep);
>> + if (err)
>> goto fail2;
>> - }
>>
>> state_set(&ep->com, LISTEN);
>> err = listen_start(ep);
>> - if (err)
>> - goto fail3;
>>
>> - /* wait for pass_open_rpl */
>> - wait_event(ep->com.waitq, ep->com.rpl_done);
>> - err = ep->com.rpl_err;
>> if (!err) {
>> cm_id->provider_data = ep;
>> + mutex_lock(&h->mutex);
>> + list_add_tail(&ep->entry, &h->listen_eps);
>> + mutex_unlock(&h->mutex);
>
> Is there a race between listen_start() being called and inserting the ep
> into the list? Could anything try to find the ep on the list after
> listen_start returns?
>
I guess if the iwarp address was removed between after the
listen_start() and before we add it to the list, then we would not stop
the listen for this address. Perhaps I need to hold the mutex around
the listen_start() -and- the insert...
>> goto out;
>> }
>> -fail3:
>> - cxgb3_free_stid(ep->com.tdev, ep->stid);
>> + dealloc_listener_list(ep);
>> fail2:
>> cm_id->rem_ref(cm_id);
>> put_ep(&ep->com);
>> @@ -1923,18 +2084,20 @@ out:
>> int iwch_destroy_listen(struct iw_cm_id *cm_id)
>> {
>> int err;
>> + struct iwch_dev *h = to_iwch_dev(cm_id->device);
>> struct iwch_listen_ep *ep = to_listen_ep(cm_id);
>>
>> PDBG("%s ep %p\n", __FUNCTION__, ep);
>>
>> might_sleep();
>> + mutex_lock(&h->mutex);
>> + list_del(&ep->entry);
>> + mutex_unlock(&h->mutex);
>> state_set(&ep->com, DEAD);
>> ep->com.rpl_done = 0;
>> ep->com.rpl_err = 0;
>> err = listen_stop(ep);
>> - wait_event(ep->com.waitq, ep->com.rpl_done);
>> - cxgb3_free_stid(ep->com.tdev, ep->stid);
>> - err = ep->com.rpl_err;
>> + dealloc_listener_list(ep);
>> cm_id->rem_ref(cm_id);
>> put_ep(&ep->com);
>> return err;
>> diff --git a/drivers/infiniband/hw/cxgb3/iwch_cm.h
>> b/drivers/infiniband/hw/cxgb3/iwch_cm.h
>> index 6107e7c..23e5a22 100644
>> --- a/drivers/infiniband/hw/cxgb3/iwch_cm.h
>> +++ b/drivers/infiniband/hw/cxgb3/iwch_cm.h
>> @@ -162,10 +162,19 @@ struct iwch_ep_common {
>> int rpl_err;
>> };
>>
>> -struct iwch_listen_ep {
>> - struct iwch_ep_common com;
>> +struct iwch_listen_entry {
>> + struct list_head entry;
>> unsigned int stid;
>> + __be32 addr;
>> +};
>> +
>> +struct iwch_listen_ep {
>> + struct iwch_ep_common com; /* Must be first entry! */
>> + struct list_head entry;
>> + struct list_head listeners;
>> int backlog;
>> + int listen_count;
>
> I didn't notice where this was used.
>
>> + int listen_rpls;
>
> or this.
>
Yea, I think this is dead code. I'll remove these.
>> };
>>
>> struct iwch_ep {
>> @@ -222,6 +231,8 @@ int iwch_resume_tid(struct iwch_ep *ep);
>> void __free_ep(struct kref *kref);
>> void iwch_rearp(struct iwch_ep *ep);
>> int iwch_ep_redirect(void *ctx, struct dst_entry *old, struct
>> dst_entry *new, struct l2t_entry *l2t);
>> +void iwch_listeners_add_addr(struct iwch_dev *rnicp, __be32 addr);
>> +void iwch_listeners_del_addr(struct iwch_dev *rnicp, __be32 addr);
>>
>> int __init iwch_cm_init(void);
>> void __exit iwch_cm_term(void);
>> _______________________________________________
>> general mailing list
>> general@lists.openfabrics.org
>> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>>
>> To unsubscribe, please visit
>> http://openib.org/mailman/listinfo/openib-general
>>
^ permalink raw reply
* Re: [PATCH 5/7] CAN: Add virtual CAN netdevice driver
From: David Miller @ 2007-09-27 19:18 UTC (permalink / raw)
To: ebiederm
Cc: urs, netdev, kaber, tglx, oliver, oliver.hartkopp, urs.thuermann,
dlezcano
In-Reply-To: <m13ax0m6ca.fsf@ebiederm.dsl.xmission.com>
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Thu, 27 Sep 2007 10:16:37 -0600
>
> I guess in particular IFF_LOOPBACK means that all packets from
> a device will come right back to the current machine, and go
> nowhere else.
>
> That usage sounds completely different then the CAN usage which
> appears to mean. Broadcast packets will be returned to this machine
> as well as being sent out onto the wire.
It's bogus and it should be removed from the CAN code, they
can add some other attribute to achieve their goals.
^ permalink raw reply
* RE: [ofa-general] [PATCH v3] iw_cxgb3: Support "iwarp-only"interfacesto avoid 4-tuple conflicts.
From: Sean Hefty @ 2007-09-27 19:11 UTC (permalink / raw)
To: 'Kanevsky, Arkady', Sean Hefty, Steve Wise
Cc: netdev, rdreier, linux-kernel, general
In-Reply-To: <C98692FD98048C41885E0B0FACD9DFB805187514@exnane01.hq.netapp.com>
>What is the model on how client connects, say for iSCSI,
>when client and server both support, iWARP and 10GbE or 1GbE,
>and would like to setup "most" performant "connection" for ULP?
For the "most" performance connection, the ULP would use IB, and all these
problems go away. :)
This proposal is for each iwarp interface to have its own IP address. Clients
would need an iwarp usable address of the server and would connect using
rdma_connect(). If that call (or rdma_resolve_addr/route) fails, the client
could try connecting using sockets, aoi, or some other interface. I don't see
that Steve's proposal changes anything from the client's perspective.
- Sean
^ permalink raw reply
* [PATCH] Added Ethernet PHY support for the Realtek 821x
From: ljd015 @ 2007-09-27 18:26 UTC (permalink / raw)
To: netdev; +Cc: Joe D'Abbraccio, Johnson Leung, Kevin Lam
From: Joe D'Abbraccio <ljd015@freescale.com>
The MPC837xERDB platform uses the RTL8211B Ethernet PHY on the
WAN port (on eth0). Also added the kernel configuration options for
selecting the PHY.
Signed-off-by: Johnson Leung <r58129@freescale.com>
Signed-off-by: Kevin Lam <r43770@freescale.com>
Signed-off-by: Joe D'Abbraccio <ljd015@freescale.com>
---
arch/powerpc/configs/mpc837x_rdb_defconfig | 1 +
drivers/net/phy/Kconfig | 5 ++
drivers/net/phy/Makefile | 1 +
drivers/net/phy/realtek.c | 84 ++++++++++++++++++++++++++++
4 files changed, 91 insertions(+), 0 deletions(-)
create mode 100644 drivers/net/phy/realtek.c
diff --git a/arch/powerpc/configs/mpc837x_rdb_defconfig b/arch/powerpc/configs/mpc837x_rdb_defconfig
index e398e9f..9837493 100644
--- a/arch/powerpc/configs/mpc837x_rdb_defconfig
+++ b/arch/powerpc/configs/mpc837x_rdb_defconfig
@@ -411,6 +411,7 @@ CONFIG_MARVELL_PHY=y
# CONFIG_SMSC_PHY is not set
# CONFIG_BROADCOM_PHY is not set
# CONFIG_ICPLUS_PHY is not set
+CONFIG_REALTEK_PHY=y
# CONFIG_FIXED_PHY is not set
CONFIG_NET_ETHERNET=y
CONFIG_MII=y
diff --git a/drivers/net/phy/Kconfig b/drivers/net/phy/Kconfig
index dd09011..9ce95c9 100644
--- a/drivers/net/phy/Kconfig
+++ b/drivers/net/phy/Kconfig
@@ -60,6 +60,11 @@ config ICPLUS_PHY
---help---
Currently supports the IP175C PHY.
+config REALTEK_PHY
+ tristate "Drivers for Realtek PHYs"
+ ---help---
+ Supports the Realtek 821x PHY.
+
config FIXED_PHY
tristate "Drivers for PHY emulation on fixed speed/link"
---help---
diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile
index 8885650..d7bfa4e 100644
--- a/drivers/net/phy/Makefile
+++ b/drivers/net/phy/Makefile
@@ -12,4 +12,5 @@ obj-$(CONFIG_SMSC_PHY) += smsc.o
obj-$(CONFIG_VITESSE_PHY) += vitesse.o
obj-$(CONFIG_BROADCOM_PHY) += broadcom.o
obj-$(CONFIG_ICPLUS_PHY) += icplus.o
+obj-$(CONFIG_REALTEK_PHY) += realtek.o
obj-$(CONFIG_FIXED_PHY) += fixed.o
diff --git a/drivers/net/phy/realtek.c b/drivers/net/phy/realtek.c
new file mode 100644
index 0000000..546c25f
--- /dev/null
+++ b/drivers/net/phy/realtek.c
@@ -0,0 +1,84 @@
+/*
+ * drivers/net/phy/realtek.c
+ *
+ * Driver for Realtek PHYs
+ *
+ * Author: Johnson Leung <r58129@freescale.com>
+ *
+ * Copyright (c) 2004 Freescale Semiconductor, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or (at your
+ * option) any later version.
+ *
+ */
+#include <linux/phy.h>
+
+#define RTL821x_PHYSR 0x11
+#define RTL821x_PHYSR_DUPLEX 0x2000
+#define RTL821x_PHYSR_SPEED 0xc000
+#define RTL821x_INER 0x12
+#define RTL821x_INER_INIT 0x6400
+#define RTL821x_INSR 0x13
+
+MODULE_DESCRIPTION("Realtek PHY driver");
+MODULE_AUTHOR("Johnson Leung");
+MODULE_LICENSE("GPL");
+
+static int rtl821x_config_init(struct phy_device *phydev)
+{
+ return 0;
+}
+
+static int rtl821x_ack_interrupt(struct phy_device *phydev)
+{
+ int err;
+
+ err = phy_read(phydev, RTL821x_INSR);
+ return (err < 0) ? err : 0;
+}
+
+static int rtl821x_config_intr(struct phy_device *phydev)
+{
+ int err;
+
+ if (phydev->interrupts == PHY_INTERRUPT_ENABLED)
+ err = phy_write(phydev, RTL821x_INER,
+ RTL821x_INER_INIT);
+ else
+ err = phy_write(phydev, RTL821x_INER, 0);
+
+ return err;
+}
+
+/* RTL8211B */
+static struct phy_driver rtl821x_driver = {
+ .phy_id = 0x0001cc912,
+ .name = "RTL821x Gigabit Ethernet",
+ .phy_id_mask = 0x001fffff,
+ .features = PHY_GBIT_FEATURES,
+ .flags = PHY_HAS_INTERRUPT,
+ .config_init = &rtl821x_config_init,
+ .config_aneg = &genphy_config_aneg,
+ .read_status = &genphy_read_status,
+ .ack_interrupt = &rtl821x_ack_interrupt,
+ .config_intr = &rtl821x_config_intr,
+ .driver = { .owner = THIS_MODULE,},
+};
+
+static int __init realtek_init(void)
+{
+ int ret;
+
+ ret = phy_driver_register(&rtl821x_driver);
+ return ret;
+}
+
+static void __exit realtek_exit(void)
+{
+ phy_driver_unregister(&rtl821x_driver);
+}
+
+module_init(realtek_init);
+module_exit(realtek_exit);
--
1.5.2.2
^ permalink raw reply related
* Re: [PKT_SCHED]: Add stateless NAT
From: David Miller @ 2007-09-27 19:08 UTC (permalink / raw)
To: hadi; +Cc: herbert, netdev, kuznet
In-Reply-To: <1190896785.4290.18.camel@localhost>
From: jamal <hadi@cyberus.ca>
Date: Thu, 27 Sep 2007 08:39:45 -0400
> nice work. I like the egress flag idea;->
> and who would have thunk stateless nat could be written in such a few
> lines ;-> I would have put the checksum as a separate action but it is
> fine the way you did it since it simplifies config.
> more comments below.
>
> On Thu, 2007-27-09 at 15:34 +0800, Herbert Xu wrote:
>
> > +config NET_ACT_NAT
> > + tristate "Stateless NAT"
> > + depends on NET_CLS_ACT
> > + select NETFILTER
>
> I am gonna have to agree with Evgeniy on this Herbert;->
> The rewards are it will improve performance for people who dont need
> netfilter.
> Ok, who is gonna move the csum utility functions out? /me looks at
> Evgeniy;->
> I could do it realsoonnow if noone raises their hands.
> In any case, it would be real nice to have but i dont see it as a show
> stopper for inclusion.
I agree that we should move the functions out. However...
You have to realize that this logic is a complete crock
of shit for %99.99999999999999999999 of Linux users out there
who get and only use distribution compiled kernels which are
going to enable everything anyways.
So we better make sure there are zero performance implications at some
point just for compiling netfilter into the tree. I know that isn't
the case currently, but that means that we aren't helping out the
majority of Linux users and are thus only adding these optimizations
for such a small sliver of users and that is totally pointless and
sucks.
^ permalink raw reply
* Re: [PATCH] proper comment for loopback initialization order
From: David Miller @ 2007-09-27 19:03 UTC (permalink / raw)
To: den; +Cc: dev, devel, netdev, containers
In-Reply-To: <20070927122527.GA10972@iris.sw.ru>
From: "Denis V. Lunev" <den@openvz.org>
Date: Thu, 27 Sep 2007 16:25:27 +0400
> Subject: [PATCH] proper comment for loopback initialization order
> From: "Denis V. Lunev" <den@openvz.org>
> To: davem@davemloft.net
> Cc: dev@openvz.org, devel@openvz.org, netdev@vger.kernel.org,
> containers@lists.osdl.org
> Date: Thu, 27 Sep 2007 16:25:27 +0400
> Sender: <den@sw.ru>
> User-Agent: Mutt/1.5.16 (2007-06-09)
>
> Loopback device is special. It should be initialized at the very
> beginning. Initialization order has been changed by
> Eric W. Biederman <ebiederm@xmission.com> and this change is non-obvious
> and important enough to add proper comment.
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
Applied, but I had to fix the coding style of your comment,
please do it like this in the future:
/* Loopback is special. It should be initialized before any other network
* device and network subsystem.
*/
Thanks.
^ permalink raw reply
* Re: [PATCH] fixed broken bootp compilation
From: David Miller @ 2007-09-27 18:58 UTC (permalink / raw)
To: den; +Cc: shemminger, netdev
In-Reply-To: <20070927104622.GA8349@iris.sw.ru>
From: "Denis V. Lunev" <den@openvz.org>
Date: Thu, 27 Sep 2007 14:46:22 +0400
> Compilation fix. Extra bracket removed.
> Broken by "[NET]: Wrap netdevice hardware header creation" from
> Stephen Hemminger <shemminger@linux-foundation.org>
>
> Signed-off-by: Denis V. Lunev <den@openvz.org>
Applied, thanks Denis.
^ permalink raw reply
* RE: [ofa-general] [PATCH v3] iw_cxgb3: Support "iwarp-only" interfacesto avoid 4-tuple conflicts.
From: Kanevsky, Arkady @ 2007-09-27 18:56 UTC (permalink / raw)
To: Sean Hefty, Steve Wise; +Cc: netdev, rdreier, linux-kernel, general
In-Reply-To: <46FBF8AF.9040700@ichips.intel.com>
Sean,
What is the model on how client connects, say for iSCSI,
when client and server both support, iWARP and 10GbE or 1GbE,
and would like to setup "most" performant "connection" for ULP?
Thanks,
Arkady Kanevsky email: arkady@netapp.com
Network Appliance Inc. phone: 781-768-5395
1601 Trapelo Rd. - Suite 16. Fax: 781-895-1195
Waltham, MA 02451 central phone: 781-768-5300
> -----Original Message-----
> From: Sean Hefty [mailto:mshefty@ichips.intel.com]
> Sent: Thursday, September 27, 2007 2:39 PM
> To: Steve Wise
> Cc: netdev@vger.kernel.org; rdreier@cisco.com;
> general@lists.openfabrics.org; linux-kernel@vger.kernel.org
> Subject: Re: [ofa-general] [PATCH v3] iw_cxgb3: Support
> "iwarp-only" interfacesto avoid 4-tuple conflicts.
>
> > The sysadmin creates "for iwarp use only" alias interfaces
> of the form
> > "devname:iw*" where devname is the native interface name
> (eg eth0) for
> > the iwarp netdev device. The alias label can be anything
> starting with "iw".
> > The "iw" immediately after the ':' is the key used by the
> iw_cxgb3 driver.
>
> I'm still not sure about this, but haven't come up with
> anything better myself. And if there's a good chance of
> other rnic's needing the same support, I'd rather see the
> common code separated out, even if just encapsulated within
> this module for easy re-use.
>
> As for the code, I have a couple of questions about whether
> deadlock and a race condition are possible, plus a few minor comments.
>
> > +static void insert_ifa(struct iwch_dev *rnicp, struct
> in_ifaddr *ifa)
> > +{
> > + struct iwch_addrlist *addr;
> > +
> > + addr = kmalloc(sizeof *addr, GFP_KERNEL);
> > + if (!addr) {
> > + printk(KERN_ERR MOD "%s - failed to alloc memory!\n",
> > + __FUNCTION__);
> > + return;
> > + }
> > + addr->ifa = ifa;
> > + mutex_lock(&rnicp->mutex);
> > + list_add_tail(&addr->entry, &rnicp->addrlist);
> > + mutex_unlock(&rnicp->mutex);
> > +}
>
> Should this return success/failure?
>
> > +static int nb_callback(struct notifier_block *self,
> unsigned long event,
> > + void *ctx)
> > +{
> > + struct in_ifaddr *ifa = ctx;
> > + struct iwch_dev *rnicp = container_of(self, struct
> iwch_dev, nb);
> > +
> > + PDBG("%s rnicp %p event %lx\n", __FUNCTION__, rnicp, event);
> > +
> > + switch (event) {
> > + case NETDEV_UP:
> > + if (netdev_is_ours(rnicp, ifa->ifa_dev->dev) &&
> > + is_iwarp_label(ifa->ifa_label)) {
> > + PDBG("%s label %s addr 0x%x added\n",
> > + __FUNCTION__, ifa->ifa_label,
> ifa->ifa_address);
> > + insert_ifa(rnicp, ifa);
> > + iwch_listeners_add_addr(rnicp,
> ifa->ifa_address);
>
> If insert_ifa() fails, what will iwch_listeners_add_addr()
> do? (I'm not easily seeing the relationship between the
> address list and the listen list at this point.)
>
> > + }
> > + break;
> > + case NETDEV_DOWN:
> > + if (netdev_is_ours(rnicp, ifa->ifa_dev->dev) &&
> > + is_iwarp_label(ifa->ifa_label)) {
> > + PDBG("%s label %s addr 0x%x deleted\n",
> > + __FUNCTION__, ifa->ifa_label,
> ifa->ifa_address);
> > + iwch_listeners_del_addr(rnicp,
> ifa->ifa_address);
> > + remove_ifa(rnicp, ifa);
> > + }
> > + break;
> > + default:
> > + break;
> > + }
> > + return 0;
> > +}
> > +
> > +static void delete_addrlist(struct iwch_dev *rnicp) {
> > + struct iwch_addrlist *addr, *tmp;
> > +
> > + mutex_lock(&rnicp->mutex);
> > + list_for_each_entry_safe(addr, tmp, &rnicp->addrlist, entry) {
> > + list_del(&addr->entry);
> > + kfree(addr);
> > + }
> > + mutex_unlock(&rnicp->mutex);
> > +}
> > +
> > +static void populate_addrlist(struct iwch_dev *rnicp) {
> > + int i;
> > + struct in_device *indev;
> > +
> > + for (i = 0; i < rnicp->rdev.port_info.nports; i++) {
> > + indev = in_dev_get(rnicp->rdev.port_info.lldevs[i]);
> > + if (!indev)
> > + continue;
> > + for_ifa(indev)
> > + if (is_iwarp_label(ifa->ifa_label)) {
> > + PDBG("%s label %s addr 0x%x added\n",
> > + __FUNCTION__, ifa->ifa_label,
> > + ifa->ifa_address);
> > + insert_ifa(rnicp, ifa);
> > + }
> > + endfor_ifa(indev);
> > + }
> > +}
> > +
> > static void rnic_init(struct iwch_dev *rnicp) {
> > PDBG("%s iwch_dev %p\n", __FUNCTION__, rnicp); @@
> -70,6 +187,12 @@
> > static void rnic_init(struct iwch_dev *r
> > idr_init(&rnicp->qpidr);
> > idr_init(&rnicp->mmidr);
> > spin_lock_init(&rnicp->lock);
> > + INIT_LIST_HEAD(&rnicp->addrlist);
> > + INIT_LIST_HEAD(&rnicp->listen_eps);
> > + mutex_init(&rnicp->mutex);
> > + rnicp->nb.notifier_call = nb_callback;
> > + populate_addrlist(rnicp);
> > + register_inetaddr_notifier(&rnicp->nb);
> >
> > rnicp->attr.vendor_id = 0x168;
> > rnicp->attr.vendor_part_id = 7;
> > @@ -148,6 +271,8 @@ static void close_rnic_dev(struct t3cdev
> > mutex_lock(&dev_mutex);
> > list_for_each_entry_safe(dev, tmp, &dev_list, entry) {
> > if (dev->rdev.t3cdev_p == tdev) {
> > + unregister_inetaddr_notifier(&dev->nb);
> > + delete_addrlist(dev);
> > list_del(&dev->entry);
> > iwch_unregister_device(dev);
> > cxio_rdev_close(&dev->rdev);
> > diff --git a/drivers/infiniband/hw/cxgb3/iwch.h
> > b/drivers/infiniband/hw/cxgb3/iwch.h
> > index caf4e60..7fa0a47 100644
> > --- a/drivers/infiniband/hw/cxgb3/iwch.h
> > +++ b/drivers/infiniband/hw/cxgb3/iwch.h
> > @@ -36,6 +36,8 @@ #include <linux/mutex.h> #include
> <linux/list.h>
> > #include <linux/spinlock.h> #include <linux/idr.h>
> > +#include <linux/notifier.h>
> > +#include <linux/inetdevice.h>
> >
> > #include <rdma/ib_verbs.h>
> >
> > @@ -101,6 +103,11 @@ struct iwch_rnic_attributes {
> > u32 cq_overflow_detection;
> > };
> >
> > +struct iwch_addrlist {
> > + struct list_head entry;
> > + struct in_ifaddr *ifa;
> > +};
> > +
> > struct iwch_dev {
> > struct ib_device ibdev;
> > struct cxio_rdev rdev;
> > @@ -111,6 +118,10 @@ struct iwch_dev {
> > struct idr mmidr;
> > spinlock_t lock;
> > struct list_head entry;
> > + struct notifier_block nb;
> > + struct list_head addrlist;
> > + struct list_head listen_eps;
>
> The behavior of the listen lists is similar to what's done in the
> rdma_cm: Wildcard listens are stored in a listen_any_list.
> When new devices are added, associated listens are added to
> each device. This is similar, except we're dealing with
> devices and addresses. I'm wondering if we shouldn't mimic
> the same behavior and track listens in iwch_addrlist
> directly. (I don't see anything wrong with this approach
> though.)
>
> What happens if an address changes between iwarp only and non-iwarp?
>
> How are listens on specific addresses handled from an rdma_cm level?
> Does the rdma_cm map the address to the device, call the
> iw_cm to listen, which in turn calls the device listen
> function? The device then checks that the address has been
> marked as iwarp only? (I'm being too lazy to trace this
> through the code, but if you don't know off the top of your
> head, I will do that.)
>
> > + struct mutex mutex;
> > };
> >
> > static inline struct iwch_dev *to_iwch_dev(struct
> ib_device *ibdev)
> > diff --git a/drivers/infiniband/hw/cxgb3/iwch_cm.c
> > b/drivers/infiniband/hw/cxgb3/iwch_cm.c
> > index 1cdfcd4..afc8a48 100644
> > --- a/drivers/infiniband/hw/cxgb3/iwch_cm.c
> > +++ b/drivers/infiniband/hw/cxgb3/iwch_cm.c
> > @@ -1127,23 +1127,149 @@ static int act_open_rpl(struct t3cdev *t
> > return CPL_RET_BUF_DONE;
> > }
> >
> > -static int listen_start(struct iwch_listen_ep *ep)
> > +static int wait_for_reply(struct iwch_ep_common *epc) {
> > + PDBG("%s ep %p waiting\n", __FUNCTION__, epc);
> > + wait_event(epc->waitq, epc->rpl_done);
> > + PDBG("%s ep %p done waiting err %d\n", __FUNCTION__,
> epc, epc->rpl_err);
> > + return epc->rpl_err;
> > +}
>
> What thread is being blocked here, and what sets the event?
>
> > +
> > +static struct iwch_listen_entry *alloc_listener(struct
> iwch_listen_ep *ep,
> > + __be32 addr)
> > +{
> > + struct iwch_dev *h = to_iwch_dev(ep->com.cm_id->device);
> > + struct iwch_listen_entry *le;
> > +
> > + le = kmalloc(sizeof *le, GFP_KERNEL);
> > + if (!le) {
> > + printk(KERN_ERR MOD "%s - failed to alloc memory!\n",
> > + __FUNCTION__);
> > + return NULL;
> > + }
> > + le->stid = cxgb3_alloc_stid(h->rdev.t3cdev_p,
> > + &t3c_client, ep);
> > + if (le->stid == -1) {
> > + printk(KERN_ERR MOD "%s - cannot alloc stid.\n",
> > + __FUNCTION__);
> > + kfree(le);
> > + return NULL;
> > + }
> > + le->addr = addr;
> > + PDBG("%s stid %u addr %x port %x\n", __FUNCTION__, le->stid,
> > + ntohl(le->addr), ntohs(ep->com.local_addr.sin_port));
> > + return le;
> > +}
> > +
> > +static void dealloc_listener(struct iwch_listen_ep *ep,
> > + struct iwch_listen_entry *le) {
> > + PDBG("%s stid %u addr %x port %x\n", __FUNCTION__, le->stid,
> > + ntohl(le->addr), ntohs(ep->com.local_addr.sin_port));
> > + cxgb3_free_stid(ep->com.tdev, le->stid);
> > + kfree(le);
> > +}
> > +
> > +static void dealloc_listener_list(struct iwch_listen_ep *ep) {
> > + struct iwch_listen_entry *le, *tmp;
> > + struct iwch_dev *h = to_iwch_dev(ep->com.cm_id->device);
> > +
> > + mutex_lock(&h->mutex);
> > + list_for_each_entry_safe(le, tmp, &ep->listeners, entry) {
> > + list_del(&le->entry);
> > + dealloc_listener(ep, le);
> > + }
> > + mutex_unlock(&h->mutex);
> > +}
> > +
> > +static int alloc_listener_list(struct iwch_listen_ep *ep) {
> > + struct iwch_dev *h = to_iwch_dev(ep->com.cm_id->device);
>
> nit: in place of 'h' in several places, how about 'rnicp',
> which is also used?
>
> > + struct iwch_addrlist *addr;
> > + struct iwch_listen_entry *le;
> > + int err = 0;
> > + int added=0;
> > + mutex_lock(&h->mutex);
> > + list_for_each_entry(addr, &h->addrlist, entry) {
> > + if (ep->com.local_addr.sin_addr.s_addr == 0 ||
> > + ep->com.local_addr.sin_addr.s_addr ==
> > + addr->ifa->ifa_address) {
> > + le = alloc_listener(ep, addr->ifa->ifa_address);
> > + if (!le)
> > + break;
> > + list_add_tail(&le->entry, &ep->listeners);
> > + added++;
> > + }
> > + }
> > + mutex_unlock(&h->mutex);
> > + if (ep->com.local_addr.sin_addr.s_addr != 0 && !added)
> > + err = -EADDRNOTAVAIL;
> > + if (!err && !added)
> > + printk(KERN_WARNING MOD
> > + "No RDMA interface found for device %s\n",
> > + pci_name(h->rdev.rnic_info.pdev));
> > + return err;
> > +}
>
> Adding some white space would improve readability.
>
> > +
> > +static int listen_stop_one(struct iwch_listen_ep *ep,
> unsigned int
> > +stid)
> > {
> > struct sk_buff *skb;
> > - struct cpl_pass_open_req *req;
> > + struct cpl_close_listserv_req *req;
> > +
> > + PDBG("%s stid %u\n", __FUNCTION__, stid);
> > + skb = get_skb(NULL, sizeof(*req), GFP_KERNEL);
> > + if (!skb) {
> > + printk(KERN_ERR MOD "%s - failed to alloc
> skb\n", __FUNCTION__);
> > + return -ENOMEM;
> > + }
> > + req = (struct cpl_close_listserv_req *) skb_put(skb,
> sizeof(*req));
> > + req->wr.wr_hi = htonl(V_WR_OP(FW_WROPCODE_FORWARD));
> > + req->cpu_idx = 0;
> > + OPCODE_TID(req) =
> htonl(MK_OPCODE_TID(CPL_CLOSE_LISTSRV_REQ, stid));
> > + skb->priority = 1;
> > + ep->com.rpl_err = 0;
> > + ep->com.rpl_done = 0;
> > + cxgb3_ofld_send(ep->com.tdev, skb);
> > + return wait_for_reply(&ep->com);
> > +}
> > +
> > +static int listen_stop(struct iwch_listen_ep *ep) {
> > + struct iwch_listen_entry *le;
> > + struct iwch_dev *h = to_iwch_dev(ep->com.cm_id->device);
> > + int err = 0;
> >
> > PDBG("%s ep %p\n", __FUNCTION__, ep);
> > + mutex_lock(&h->mutex);
> > + list_for_each_entry(le, &ep->listeners, entry) {
> > + err = listen_stop_one(ep, le->stid);
>
> This ends up blocking while holding a mutex, which looks like
> deadlock potential.
>
> > + if (err)
> > + break;
> > + }
> > + mutex_unlock(&h->mutex);
> > + return err;
> > +}
> > +
> > +static int listen_start_one(struct iwch_listen_ep *ep,
> unsigned int stid,
> > + __be32 addr, __be16 port)
> > +{
> > + struct sk_buff *skb;
> > + struct cpl_pass_open_req *req;
> > +
> > + PDBG("%s stid %u addr %x port %x\n", __FUNCTION__,
> stid, ntohl(addr),
> > + ntohs(port));
> > skb = get_skb(NULL, sizeof(*req), GFP_KERNEL);
> > if (!skb) {
> > - printk(KERN_ERR MOD "t3c_listen_start failed to
> alloc skb!\n");
> > + printk(KERN_ERR MOD "%s - failed to alloc
> skb\n", __FUNCTION__);
> > return -ENOMEM;
> > }
> >
> > req = (struct cpl_pass_open_req *) skb_put(skb, sizeof(*req));
> > req->wr.wr_hi = htonl(V_WR_OP(FW_WROPCODE_FORWARD));
> > - OPCODE_TID(req) =
> htonl(MK_OPCODE_TID(CPL_PASS_OPEN_REQ, ep->stid));
> > - req->local_port = ep->com.local_addr.sin_port;
> > - req->local_ip = ep->com.local_addr.sin_addr.s_addr;
> > + OPCODE_TID(req) = htonl(MK_OPCODE_TID(CPL_PASS_OPEN_REQ, stid));
> > + req->local_port = port;
> > + req->local_ip = addr;
> > req->peer_port = 0;
> > req->peer_ip = 0;
> > req->peer_netmask = 0;
> > @@ -1152,8 +1278,32 @@ static int listen_start(struct iwch_list
> > req->opt1 = htonl(V_CONN_POLICY(CPL_CONN_POLICY_ASK));
> >
> > skb->priority = 1;
> > + ep->com.rpl_err = 0;
> > + ep->com.rpl_done = 0;
> > cxgb3_ofld_send(ep->com.tdev, skb);
> > - return 0;
> > + return wait_for_reply(&ep->com);
> > +}
> > +
> > +static int listen_start(struct iwch_listen_ep *ep) {
> > + struct iwch_listen_entry *le;
> > + struct iwch_dev *h = to_iwch_dev(ep->com.cm_id->device);
> > + int err = 0;
> > +
> > + PDBG("%s ep %p\n", __FUNCTION__, ep);
> > + mutex_lock(&h->mutex);
> > + list_for_each_entry(le, &ep->listeners, entry) {
> > + err = listen_start_one(ep, le->stid, le->addr,
> > + ep->com.local_addr.sin_port);
>
> Similar to above - blocking while holding a mutex. There are
> a couple of other places where this also occurs.
>
> > + if (err)
> > + goto fail;
> > + }
> > + mutex_unlock(&h->mutex);
> > + return err;
> > +fail:
> > + mutex_unlock(&h->mutex);
> > + listen_stop(ep);
> > + return err;
> > }
> >
> > static int pass_open_rpl(struct t3cdev *tdev, struct sk_buff *skb,
> > void *ctx) @@ -1170,39 +1320,59 @@ static int
> pass_open_rpl(struct t3cdev *
> > return CPL_RET_BUF_DONE;
> > }
> >
> > -static int listen_stop(struct iwch_listen_ep *ep) -{
> > - struct sk_buff *skb;
> > - struct cpl_close_listserv_req *req;
> > -
> > - PDBG("%s ep %p\n", __FUNCTION__, ep);
> > - skb = get_skb(NULL, sizeof(*req), GFP_KERNEL);
> > - if (!skb) {
> > - printk(KERN_ERR MOD "%s - failed to alloc
> skb\n", __FUNCTION__);
> > - return -ENOMEM;
> > - }
> > - req = (struct cpl_close_listserv_req *) skb_put(skb,
> sizeof(*req));
> > - req->wr.wr_hi = htonl(V_WR_OP(FW_WROPCODE_FORWARD));
> > - req->cpu_idx = 0;
> > - OPCODE_TID(req) =
> htonl(MK_OPCODE_TID(CPL_CLOSE_LISTSRV_REQ, ep->stid));
> > - skb->priority = 1;
> > - cxgb3_ofld_send(ep->com.tdev, skb);
> > - return 0;
> > -}
> > -
> > static int close_listsrv_rpl(struct t3cdev *tdev, struct
> sk_buff *skb,
> > void *ctx)
> > {
> > struct iwch_listen_ep *ep = ctx;
> > struct cpl_close_listserv_rpl *rpl = cplhdr(skb);
> >
> > - PDBG("%s ep %p\n", __FUNCTION__, ep);
> > + PDBG("%s ep %p stid %u\n", __FUNCTION__, ep, GET_TID(rpl));
> > +
> > ep->com.rpl_err = status2errno(rpl->status);
> > ep->com.rpl_done = 1;
> > wake_up(&ep->com.waitq);
> > return CPL_RET_BUF_DONE;
> > }
> >
> > +void iwch_listeners_add_addr(struct iwch_dev *rnicp, __be32 addr) {
> > + struct iwch_listen_ep *listen_ep;
> > + struct iwch_listen_entry *le;
> > +
> > + mutex_lock(&rnicp->mutex);
> > + list_for_each_entry(listen_ep, &rnicp->listen_eps, entry) {
> > + if (listen_ep->com.local_addr.sin_addr.s_addr)
> > + continue;
> > + le = alloc_listener(listen_ep, addr);
> > + if (le) {
> > + list_add_tail(&le->entry,
> &listen_ep->listeners);
> > + listen_start_one(listen_ep, le->stid, addr,
> > +
> listen_ep->com.local_addr.sin_port);
> > + }
> > + }
> > + mutex_unlock(&rnicp->mutex);
> > +}
> > +
> > +void iwch_listeners_del_addr(struct iwch_dev *rnicp, __be32 addr) {
> > + struct iwch_listen_ep *listen_ep;
> > + struct iwch_listen_entry *le, *tmp;
> > +
> > + mutex_lock(&rnicp->mutex);
> > + list_for_each_entry(listen_ep, &rnicp->listen_eps, entry) {
> > + if (listen_ep->com.local_addr.sin_addr.s_addr)
> > + continue;
> > + list_for_each_entry_safe(le, tmp, &listen_ep->listeners,
> > + entry)
> > + if (le->addr == addr) {
> > + listen_stop_one(listen_ep, le->stid);
> > + list_del(&le->entry);
> > + dealloc_listener(listen_ep, le);
> > + }
> > + }
> > + mutex_unlock(&rnicp->mutex);
> > +}
> > +
> > static void accept_cr(struct iwch_ep *ep, __be32 peer_ip, struct
> > sk_buff *skb) {
> > struct cpl_pass_accept_rpl *rpl;
> > @@ -1767,8 +1937,7 @@ int iwch_accept_cr(struct iw_cm_id *cm_i
> > goto err;
> >
> > /* wait for wr_ack */
> > - wait_event(ep->com.waitq, ep->com.rpl_done);
> > - err = ep->com.rpl_err;
> > + err = wait_for_reply(&ep->com);
> > if (err)
> > goto err;
> >
> > @@ -1887,31 +2056,23 @@ int iwch_create_listen(struct iw_cm_id *
> > ep->com.cm_id = cm_id;
> > ep->backlog = backlog;
> > ep->com.local_addr = cm_id->local_addr;
> > + INIT_LIST_HEAD(&ep->listeners);
> >
> > - /*
> > - * Allocate a server TID.
> > - */
> > - ep->stid = cxgb3_alloc_stid(h->rdev.t3cdev_p, &t3c_client, ep);
> > - if (ep->stid == -1) {
> > - printk(KERN_ERR MOD "%s - cannot alloc
> atid.\n", __FUNCTION__);
> > - err = -ENOMEM;
> > + err = alloc_listener_list(ep);
> > + if (err)
> > goto fail2;
> > - }
> >
> > state_set(&ep->com, LISTEN);
> > err = listen_start(ep);
> > - if (err)
> > - goto fail3;
> >
> > - /* wait for pass_open_rpl */
> > - wait_event(ep->com.waitq, ep->com.rpl_done);
> > - err = ep->com.rpl_err;
> > if (!err) {
> > cm_id->provider_data = ep;
> > + mutex_lock(&h->mutex);
> > + list_add_tail(&ep->entry, &h->listen_eps);
> > + mutex_unlock(&h->mutex);
>
> Is there a race between listen_start() being called and
> inserting the ep into the list? Could anything try to find
> the ep on the list after listen_start returns?
>
> > goto out;
> > }
> > -fail3:
> > - cxgb3_free_stid(ep->com.tdev, ep->stid);
> > + dealloc_listener_list(ep);
> > fail2:
> > cm_id->rem_ref(cm_id);
> > put_ep(&ep->com);
> > @@ -1923,18 +2084,20 @@ out:
> > int iwch_destroy_listen(struct iw_cm_id *cm_id) {
> > int err;
> > + struct iwch_dev *h = to_iwch_dev(cm_id->device);
> > struct iwch_listen_ep *ep = to_listen_ep(cm_id);
> >
> > PDBG("%s ep %p\n", __FUNCTION__, ep);
> >
> > might_sleep();
> > + mutex_lock(&h->mutex);
> > + list_del(&ep->entry);
> > + mutex_unlock(&h->mutex);
> > state_set(&ep->com, DEAD);
> > ep->com.rpl_done = 0;
> > ep->com.rpl_err = 0;
> > err = listen_stop(ep);
> > - wait_event(ep->com.waitq, ep->com.rpl_done);
> > - cxgb3_free_stid(ep->com.tdev, ep->stid);
> > - err = ep->com.rpl_err;
> > + dealloc_listener_list(ep);
> > cm_id->rem_ref(cm_id);
> > put_ep(&ep->com);
> > return err;
> > diff --git a/drivers/infiniband/hw/cxgb3/iwch_cm.h
> > b/drivers/infiniband/hw/cxgb3/iwch_cm.h
> > index 6107e7c..23e5a22 100644
> > --- a/drivers/infiniband/hw/cxgb3/iwch_cm.h
> > +++ b/drivers/infiniband/hw/cxgb3/iwch_cm.h
> > @@ -162,10 +162,19 @@ struct iwch_ep_common {
> > int rpl_err;
> > };
> >
> > -struct iwch_listen_ep {
> > - struct iwch_ep_common com;
> > +struct iwch_listen_entry {
> > + struct list_head entry;
> > unsigned int stid;
> > + __be32 addr;
> > +};
> > +
> > +struct iwch_listen_ep {
> > + struct iwch_ep_common com; /* Must be first entry! */
> > + struct list_head entry;
> > + struct list_head listeners;
> > int backlog;
> > + int listen_count;
>
> I didn't notice where this was used.
>
> > + int listen_rpls;
>
> or this.
>
> > };
> >
> > struct iwch_ep {
> > @@ -222,6 +231,8 @@ int iwch_resume_tid(struct iwch_ep *ep); void
> > __free_ep(struct kref *kref); void iwch_rearp(struct
> iwch_ep *ep);
> > int iwch_ep_redirect(void *ctx, struct dst_entry *old, struct
> > dst_entry *new, struct l2t_entry *l2t);
> > +void iwch_listeners_add_addr(struct iwch_dev *rnicp, __be32 addr);
> > +void iwch_listeners_del_addr(struct iwch_dev *rnicp, __be32 addr);
> >
> > int __init iwch_cm_init(void);
> > void __exit iwch_cm_term(void);
> > _______________________________________________
> > general mailing list
> > general@lists.openfabrics.org
> > http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
> >
> > To unsubscribe, please visit
> > http://openib.org/mailman/listinfo/openib-general
> >
> _______________________________________________
> general mailing list
> general@lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>
> To unsubscribe, please visit
> http://openib.org/mailman/listinfo/openib-general
>
^ permalink raw reply
* Re: [RFC/PATCH 2/3] UDP memory usage accounting: accounting unit and variable
From: Hideo AOKI @ 2007-09-27 18:54 UTC (permalink / raw)
To: Andi Kleen; +Cc: Satoshi OSHIMA, netdev, yoshfuji
In-Reply-To: <p734pho9nb9.fsf@bingen.suse.de>
Hello,
I apologize for not replying sooner.
Andi Kleen wrote:
> Satoshi OSHIMA <satoshi.oshima.fk@hitachi.com> writes:
>
>> This patch introduces global variable for UDP memory accounting.
>> The unit is page.
>
> The global variable doesn't seem to be very MP scalable, especially
> if you change it for each packet. This will be a very hot cache line,
> in the worst case bouncing around a large machine.
>
> Possible alternatives:
> - Per CPU variables
> - You only change the global on socket creation time (by pre allocating a large
> amount) or when the system comes under memory pressure.
> - Batching of the global updates for multiple packets [that's a variant
> of the previous one, might be still too costly though]
>
> Also for such variables it's usually good to cache line pad them on SMP
> to avoid false sharing with something else.
>
> -Andi
Thank you so much for your suggestions.
The implementation of the patch basically followed implementation of
tcp_memory_allocated. However, I should agree that the patch introduces
atomic operations too much. Then, I'll try to use the batching to reduce
the number of atomic operations.
Best regards,
Hideo Aoki
--
Hitachi Computer Products (America) Inc.
^ permalink raw reply
* Re: [PATCH 1/4] net: Dynamically allocate the per cpu counters for the loopback device.
From: David Miller @ 2007-09-27 18:52 UTC (permalink / raw)
To: ebiederm; +Cc: netdev, containers
In-Reply-To: <m1myv8r1lb.fsf@ebiederm.dsl.xmission.com>
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Thu, 27 Sep 2007 01:48:00 -0600
> I'm not doing get_cpu/put_cpu so does the comment make sense
> in relationship to per_cpu_ptr?
It is possible. But someone would need to go check for
sure.
^ permalink raw reply
* Re: [RFC/PATCH 0/3] UDP memory usage accounting
From: Hideo AOKI @ 2007-09-27 18:51 UTC (permalink / raw)
To: Evgeniy Polyakov; +Cc: Satoshi OSHIMA, netdev, yoshfuji
In-Reply-To: <20070921125843.GA15810@2ka.mipt.ru>
Hello,
Apologies for late response.
Evgeniy Polyakov wrote:
> Hi.
>
> On Fri, Sep 21, 2007 at 09:18:07PM +0900, Satoshi OSHIMA (satoshi.oshima.fk@hitachi.com) wrote:
>> This patch set try to introduce memory usage accounting for
>> UDP(currently ipv4 only).
>>
>> Currently, memory usage of UDP can be observed as the sam of
>> usage of tx_queue and rx_queue. But I believe that the system
>> wide accounting is usefull when heavy loaded condition.
>>
>> In the next step, I would like to add memory usage quota
>> for UDP to avoid unlimited memory consumption problem
>> under DDOS attack.
>
> Could you please desribed such attack in more details?
> Each UDP socket has its queue length which can not be exceeded
> (roughly), no new sockets are created when remote side sends a packet
> (like after special steps in TCP), so where is possibility to eat all
> the mem?
I think Satoshi will answer this question soon.
>> This patch set is for 2.6.23-rc7.
>
> I seriously doubt you want to put udp specific hacks and zillions of
> atomic ops all around the code just to know exact number of bytes eaten
> for UDP.
I'll revise the patch to reduce the number of atomic operations.
> Please use udp specific code (like udp_sendmsg()) for proper accounting
> if you need that, but not hacks in generic ip code.
As far as I know, Satoshi is improving this part right now. Please wait his
response.
Many thanks for your comments.
Best regards,
Hideo Aoki
--
Hitachi Computer Products (America) Inc.
^ permalink raw reply
* Re: [ofa-general] [PATCH v3] iw_cxgb3: Support "iwarp-only" interfaces to avoid 4-tuple conflicts.
From: Sean Hefty @ 2007-09-27 18:38 UTC (permalink / raw)
To: Steve Wise; +Cc: rdreier, sean.hefty, netdev, linux-kernel, general
In-Reply-To: <20070923203649.8324.64524.stgit@dell3.ogc.int>
> The sysadmin creates "for iwarp use only" alias interfaces of the form
> "devname:iw*" where devname is the native interface name (eg eth0) for the
> iwarp netdev device. The alias label can be anything starting with "iw".
> The "iw" immediately after the ':' is the key used by the iw_cxgb3 driver.
I'm still not sure about this, but haven't come up with anything better
myself. And if there's a good chance of other rnic's needing the same
support, I'd rather see the common code separated out, even if just
encapsulated within this module for easy re-use.
As for the code, I have a couple of questions about whether deadlock and
a race condition are possible, plus a few minor comments.
> +static void insert_ifa(struct iwch_dev *rnicp, struct in_ifaddr *ifa)
> +{
> + struct iwch_addrlist *addr;
> +
> + addr = kmalloc(sizeof *addr, GFP_KERNEL);
> + if (!addr) {
> + printk(KERN_ERR MOD "%s - failed to alloc memory!\n",
> + __FUNCTION__);
> + return;
> + }
> + addr->ifa = ifa;
> + mutex_lock(&rnicp->mutex);
> + list_add_tail(&addr->entry, &rnicp->addrlist);
> + mutex_unlock(&rnicp->mutex);
> +}
Should this return success/failure?
> +static int nb_callback(struct notifier_block *self, unsigned long event,
> + void *ctx)
> +{
> + struct in_ifaddr *ifa = ctx;
> + struct iwch_dev *rnicp = container_of(self, struct iwch_dev, nb);
> +
> + PDBG("%s rnicp %p event %lx\n", __FUNCTION__, rnicp, event);
> +
> + switch (event) {
> + case NETDEV_UP:
> + if (netdev_is_ours(rnicp, ifa->ifa_dev->dev) &&
> + is_iwarp_label(ifa->ifa_label)) {
> + PDBG("%s label %s addr 0x%x added\n",
> + __FUNCTION__, ifa->ifa_label, ifa->ifa_address);
> + insert_ifa(rnicp, ifa);
> + iwch_listeners_add_addr(rnicp, ifa->ifa_address);
If insert_ifa() fails, what will iwch_listeners_add_addr() do? (I'm not
easily seeing the relationship between the address list and the listen
list at this point.)
> + }
> + break;
> + case NETDEV_DOWN:
> + if (netdev_is_ours(rnicp, ifa->ifa_dev->dev) &&
> + is_iwarp_label(ifa->ifa_label)) {
> + PDBG("%s label %s addr 0x%x deleted\n",
> + __FUNCTION__, ifa->ifa_label, ifa->ifa_address);
> + iwch_listeners_del_addr(rnicp, ifa->ifa_address);
> + remove_ifa(rnicp, ifa);
> + }
> + break;
> + default:
> + break;
> + }
> + return 0;
> +}
> +
> +static void delete_addrlist(struct iwch_dev *rnicp)
> +{
> + struct iwch_addrlist *addr, *tmp;
> +
> + mutex_lock(&rnicp->mutex);
> + list_for_each_entry_safe(addr, tmp, &rnicp->addrlist, entry) {
> + list_del(&addr->entry);
> + kfree(addr);
> + }
> + mutex_unlock(&rnicp->mutex);
> +}
> +
> +static void populate_addrlist(struct iwch_dev *rnicp)
> +{
> + int i;
> + struct in_device *indev;
> +
> + for (i = 0; i < rnicp->rdev.port_info.nports; i++) {
> + indev = in_dev_get(rnicp->rdev.port_info.lldevs[i]);
> + if (!indev)
> + continue;
> + for_ifa(indev)
> + if (is_iwarp_label(ifa->ifa_label)) {
> + PDBG("%s label %s addr 0x%x added\n",
> + __FUNCTION__, ifa->ifa_label,
> + ifa->ifa_address);
> + insert_ifa(rnicp, ifa);
> + }
> + endfor_ifa(indev);
> + }
> +}
> +
> static void rnic_init(struct iwch_dev *rnicp)
> {
> PDBG("%s iwch_dev %p\n", __FUNCTION__, rnicp);
> @@ -70,6 +187,12 @@ static void rnic_init(struct iwch_dev *r
> idr_init(&rnicp->qpidr);
> idr_init(&rnicp->mmidr);
> spin_lock_init(&rnicp->lock);
> + INIT_LIST_HEAD(&rnicp->addrlist);
> + INIT_LIST_HEAD(&rnicp->listen_eps);
> + mutex_init(&rnicp->mutex);
> + rnicp->nb.notifier_call = nb_callback;
> + populate_addrlist(rnicp);
> + register_inetaddr_notifier(&rnicp->nb);
>
> rnicp->attr.vendor_id = 0x168;
> rnicp->attr.vendor_part_id = 7;
> @@ -148,6 +271,8 @@ static void close_rnic_dev(struct t3cdev
> mutex_lock(&dev_mutex);
> list_for_each_entry_safe(dev, tmp, &dev_list, entry) {
> if (dev->rdev.t3cdev_p == tdev) {
> + unregister_inetaddr_notifier(&dev->nb);
> + delete_addrlist(dev);
> list_del(&dev->entry);
> iwch_unregister_device(dev);
> cxio_rdev_close(&dev->rdev);
> diff --git a/drivers/infiniband/hw/cxgb3/iwch.h b/drivers/infiniband/hw/cxgb3/iwch.h
> index caf4e60..7fa0a47 100644
> --- a/drivers/infiniband/hw/cxgb3/iwch.h
> +++ b/drivers/infiniband/hw/cxgb3/iwch.h
> @@ -36,6 +36,8 @@ #include <linux/mutex.h>
> #include <linux/list.h>
> #include <linux/spinlock.h>
> #include <linux/idr.h>
> +#include <linux/notifier.h>
> +#include <linux/inetdevice.h>
>
> #include <rdma/ib_verbs.h>
>
> @@ -101,6 +103,11 @@ struct iwch_rnic_attributes {
> u32 cq_overflow_detection;
> };
>
> +struct iwch_addrlist {
> + struct list_head entry;
> + struct in_ifaddr *ifa;
> +};
> +
> struct iwch_dev {
> struct ib_device ibdev;
> struct cxio_rdev rdev;
> @@ -111,6 +118,10 @@ struct iwch_dev {
> struct idr mmidr;
> spinlock_t lock;
> struct list_head entry;
> + struct notifier_block nb;
> + struct list_head addrlist;
> + struct list_head listen_eps;
The behavior of the listen lists is similar to what's done in the
rdma_cm: Wildcard listens are stored in a listen_any_list. When new
devices are added, associated listens are added to each device. This is
similar, except we're dealing with devices and addresses. I'm wondering
if we shouldn't mimic the same behavior and track listens in
iwch_addrlist directly. (I don't see anything wrong with this approach
though.)
What happens if an address changes between iwarp only and non-iwarp?
How are listens on specific addresses handled from an rdma_cm level?
Does the rdma_cm map the address to the device, call the iw_cm to
listen, which in turn calls the device listen function? The device then
checks that the address has been marked as iwarp only? (I'm being too
lazy to trace this through the code, but if you don't know off the top
of your head, I will do that.)
> + struct mutex mutex;
> };
>
> static inline struct iwch_dev *to_iwch_dev(struct ib_device *ibdev)
> diff --git a/drivers/infiniband/hw/cxgb3/iwch_cm.c b/drivers/infiniband/hw/cxgb3/iwch_cm.c
> index 1cdfcd4..afc8a48 100644
> --- a/drivers/infiniband/hw/cxgb3/iwch_cm.c
> +++ b/drivers/infiniband/hw/cxgb3/iwch_cm.c
> @@ -1127,23 +1127,149 @@ static int act_open_rpl(struct t3cdev *t
> return CPL_RET_BUF_DONE;
> }
>
> -static int listen_start(struct iwch_listen_ep *ep)
> +static int wait_for_reply(struct iwch_ep_common *epc)
> +{
> + PDBG("%s ep %p waiting\n", __FUNCTION__, epc);
> + wait_event(epc->waitq, epc->rpl_done);
> + PDBG("%s ep %p done waiting err %d\n", __FUNCTION__, epc, epc->rpl_err);
> + return epc->rpl_err;
> +}
What thread is being blocked here, and what sets the event?
> +
> +static struct iwch_listen_entry *alloc_listener(struct iwch_listen_ep *ep,
> + __be32 addr)
> +{
> + struct iwch_dev *h = to_iwch_dev(ep->com.cm_id->device);
> + struct iwch_listen_entry *le;
> +
> + le = kmalloc(sizeof *le, GFP_KERNEL);
> + if (!le) {
> + printk(KERN_ERR MOD "%s - failed to alloc memory!\n",
> + __FUNCTION__);
> + return NULL;
> + }
> + le->stid = cxgb3_alloc_stid(h->rdev.t3cdev_p,
> + &t3c_client, ep);
> + if (le->stid == -1) {
> + printk(KERN_ERR MOD "%s - cannot alloc stid.\n",
> + __FUNCTION__);
> + kfree(le);
> + return NULL;
> + }
> + le->addr = addr;
> + PDBG("%s stid %u addr %x port %x\n", __FUNCTION__, le->stid,
> + ntohl(le->addr), ntohs(ep->com.local_addr.sin_port));
> + return le;
> +}
> +
> +static void dealloc_listener(struct iwch_listen_ep *ep,
> + struct iwch_listen_entry *le)
> +{
> + PDBG("%s stid %u addr %x port %x\n", __FUNCTION__, le->stid,
> + ntohl(le->addr), ntohs(ep->com.local_addr.sin_port));
> + cxgb3_free_stid(ep->com.tdev, le->stid);
> + kfree(le);
> +}
> +
> +static void dealloc_listener_list(struct iwch_listen_ep *ep)
> +{
> + struct iwch_listen_entry *le, *tmp;
> + struct iwch_dev *h = to_iwch_dev(ep->com.cm_id->device);
> +
> + mutex_lock(&h->mutex);
> + list_for_each_entry_safe(le, tmp, &ep->listeners, entry) {
> + list_del(&le->entry);
> + dealloc_listener(ep, le);
> + }
> + mutex_unlock(&h->mutex);
> +}
> +
> +static int alloc_listener_list(struct iwch_listen_ep *ep)
> +{
> + struct iwch_dev *h = to_iwch_dev(ep->com.cm_id->device);
nit: in place of 'h' in several places, how about 'rnicp', which is also
used?
> + struct iwch_addrlist *addr;
> + struct iwch_listen_entry *le;
> + int err = 0;
> + int added=0;
> + mutex_lock(&h->mutex);
> + list_for_each_entry(addr, &h->addrlist, entry) {
> + if (ep->com.local_addr.sin_addr.s_addr == 0 ||
> + ep->com.local_addr.sin_addr.s_addr ==
> + addr->ifa->ifa_address) {
> + le = alloc_listener(ep, addr->ifa->ifa_address);
> + if (!le)
> + break;
> + list_add_tail(&le->entry, &ep->listeners);
> + added++;
> + }
> + }
> + mutex_unlock(&h->mutex);
> + if (ep->com.local_addr.sin_addr.s_addr != 0 && !added)
> + err = -EADDRNOTAVAIL;
> + if (!err && !added)
> + printk(KERN_WARNING MOD
> + "No RDMA interface found for device %s\n",
> + pci_name(h->rdev.rnic_info.pdev));
> + return err;
> +}
Adding some white space would improve readability.
> +
> +static int listen_stop_one(struct iwch_listen_ep *ep, unsigned int stid)
> {
> struct sk_buff *skb;
> - struct cpl_pass_open_req *req;
> + struct cpl_close_listserv_req *req;
> +
> + PDBG("%s stid %u\n", __FUNCTION__, stid);
> + skb = get_skb(NULL, sizeof(*req), GFP_KERNEL);
> + if (!skb) {
> + printk(KERN_ERR MOD "%s - failed to alloc skb\n", __FUNCTION__);
> + return -ENOMEM;
> + }
> + req = (struct cpl_close_listserv_req *) skb_put(skb, sizeof(*req));
> + req->wr.wr_hi = htonl(V_WR_OP(FW_WROPCODE_FORWARD));
> + req->cpu_idx = 0;
> + OPCODE_TID(req) = htonl(MK_OPCODE_TID(CPL_CLOSE_LISTSRV_REQ, stid));
> + skb->priority = 1;
> + ep->com.rpl_err = 0;
> + ep->com.rpl_done = 0;
> + cxgb3_ofld_send(ep->com.tdev, skb);
> + return wait_for_reply(&ep->com);
> +}
> +
> +static int listen_stop(struct iwch_listen_ep *ep)
> +{
> + struct iwch_listen_entry *le;
> + struct iwch_dev *h = to_iwch_dev(ep->com.cm_id->device);
> + int err = 0;
>
> PDBG("%s ep %p\n", __FUNCTION__, ep);
> + mutex_lock(&h->mutex);
> + list_for_each_entry(le, &ep->listeners, entry) {
> + err = listen_stop_one(ep, le->stid);
This ends up blocking while holding a mutex, which looks like deadlock
potential.
> + if (err)
> + break;
> + }
> + mutex_unlock(&h->mutex);
> + return err;
> +}
> +
> +static int listen_start_one(struct iwch_listen_ep *ep, unsigned int stid,
> + __be32 addr, __be16 port)
> +{
> + struct sk_buff *skb;
> + struct cpl_pass_open_req *req;
> +
> + PDBG("%s stid %u addr %x port %x\n", __FUNCTION__, stid, ntohl(addr),
> + ntohs(port));
> skb = get_skb(NULL, sizeof(*req), GFP_KERNEL);
> if (!skb) {
> - printk(KERN_ERR MOD "t3c_listen_start failed to alloc skb!\n");
> + printk(KERN_ERR MOD "%s - failed to alloc skb\n", __FUNCTION__);
> return -ENOMEM;
> }
>
> req = (struct cpl_pass_open_req *) skb_put(skb, sizeof(*req));
> req->wr.wr_hi = htonl(V_WR_OP(FW_WROPCODE_FORWARD));
> - OPCODE_TID(req) = htonl(MK_OPCODE_TID(CPL_PASS_OPEN_REQ, ep->stid));
> - req->local_port = ep->com.local_addr.sin_port;
> - req->local_ip = ep->com.local_addr.sin_addr.s_addr;
> + OPCODE_TID(req) = htonl(MK_OPCODE_TID(CPL_PASS_OPEN_REQ, stid));
> + req->local_port = port;
> + req->local_ip = addr;
> req->peer_port = 0;
> req->peer_ip = 0;
> req->peer_netmask = 0;
> @@ -1152,8 +1278,32 @@ static int listen_start(struct iwch_list
> req->opt1 = htonl(V_CONN_POLICY(CPL_CONN_POLICY_ASK));
>
> skb->priority = 1;
> + ep->com.rpl_err = 0;
> + ep->com.rpl_done = 0;
> cxgb3_ofld_send(ep->com.tdev, skb);
> - return 0;
> + return wait_for_reply(&ep->com);
> +}
> +
> +static int listen_start(struct iwch_listen_ep *ep)
> +{
> + struct iwch_listen_entry *le;
> + struct iwch_dev *h = to_iwch_dev(ep->com.cm_id->device);
> + int err = 0;
> +
> + PDBG("%s ep %p\n", __FUNCTION__, ep);
> + mutex_lock(&h->mutex);
> + list_for_each_entry(le, &ep->listeners, entry) {
> + err = listen_start_one(ep, le->stid, le->addr,
> + ep->com.local_addr.sin_port);
Similar to above - blocking while holding a mutex. There are a couple
of other places where this also occurs.
> + if (err)
> + goto fail;
> + }
> + mutex_unlock(&h->mutex);
> + return err;
> +fail:
> + mutex_unlock(&h->mutex);
> + listen_stop(ep);
> + return err;
> }
>
> static int pass_open_rpl(struct t3cdev *tdev, struct sk_buff *skb, void *ctx)
> @@ -1170,39 +1320,59 @@ static int pass_open_rpl(struct t3cdev *
> return CPL_RET_BUF_DONE;
> }
>
> -static int listen_stop(struct iwch_listen_ep *ep)
> -{
> - struct sk_buff *skb;
> - struct cpl_close_listserv_req *req;
> -
> - PDBG("%s ep %p\n", __FUNCTION__, ep);
> - skb = get_skb(NULL, sizeof(*req), GFP_KERNEL);
> - if (!skb) {
> - printk(KERN_ERR MOD "%s - failed to alloc skb\n", __FUNCTION__);
> - return -ENOMEM;
> - }
> - req = (struct cpl_close_listserv_req *) skb_put(skb, sizeof(*req));
> - req->wr.wr_hi = htonl(V_WR_OP(FW_WROPCODE_FORWARD));
> - req->cpu_idx = 0;
> - OPCODE_TID(req) = htonl(MK_OPCODE_TID(CPL_CLOSE_LISTSRV_REQ, ep->stid));
> - skb->priority = 1;
> - cxgb3_ofld_send(ep->com.tdev, skb);
> - return 0;
> -}
> -
> static int close_listsrv_rpl(struct t3cdev *tdev, struct sk_buff *skb,
> void *ctx)
> {
> struct iwch_listen_ep *ep = ctx;
> struct cpl_close_listserv_rpl *rpl = cplhdr(skb);
>
> - PDBG("%s ep %p\n", __FUNCTION__, ep);
> + PDBG("%s ep %p stid %u\n", __FUNCTION__, ep, GET_TID(rpl));
> +
> ep->com.rpl_err = status2errno(rpl->status);
> ep->com.rpl_done = 1;
> wake_up(&ep->com.waitq);
> return CPL_RET_BUF_DONE;
> }
>
> +void iwch_listeners_add_addr(struct iwch_dev *rnicp, __be32 addr)
> +{
> + struct iwch_listen_ep *listen_ep;
> + struct iwch_listen_entry *le;
> +
> + mutex_lock(&rnicp->mutex);
> + list_for_each_entry(listen_ep, &rnicp->listen_eps, entry) {
> + if (listen_ep->com.local_addr.sin_addr.s_addr)
> + continue;
> + le = alloc_listener(listen_ep, addr);
> + if (le) {
> + list_add_tail(&le->entry, &listen_ep->listeners);
> + listen_start_one(listen_ep, le->stid, addr,
> + listen_ep->com.local_addr.sin_port);
> + }
> + }
> + mutex_unlock(&rnicp->mutex);
> +}
> +
> +void iwch_listeners_del_addr(struct iwch_dev *rnicp, __be32 addr)
> +{
> + struct iwch_listen_ep *listen_ep;
> + struct iwch_listen_entry *le, *tmp;
> +
> + mutex_lock(&rnicp->mutex);
> + list_for_each_entry(listen_ep, &rnicp->listen_eps, entry) {
> + if (listen_ep->com.local_addr.sin_addr.s_addr)
> + continue;
> + list_for_each_entry_safe(le, tmp, &listen_ep->listeners,
> + entry)
> + if (le->addr == addr) {
> + listen_stop_one(listen_ep, le->stid);
> + list_del(&le->entry);
> + dealloc_listener(listen_ep, le);
> + }
> + }
> + mutex_unlock(&rnicp->mutex);
> +}
> +
> static void accept_cr(struct iwch_ep *ep, __be32 peer_ip, struct sk_buff *skb)
> {
> struct cpl_pass_accept_rpl *rpl;
> @@ -1767,8 +1937,7 @@ int iwch_accept_cr(struct iw_cm_id *cm_i
> goto err;
>
> /* wait for wr_ack */
> - wait_event(ep->com.waitq, ep->com.rpl_done);
> - err = ep->com.rpl_err;
> + err = wait_for_reply(&ep->com);
> if (err)
> goto err;
>
> @@ -1887,31 +2056,23 @@ int iwch_create_listen(struct iw_cm_id *
> ep->com.cm_id = cm_id;
> ep->backlog = backlog;
> ep->com.local_addr = cm_id->local_addr;
> + INIT_LIST_HEAD(&ep->listeners);
>
> - /*
> - * Allocate a server TID.
> - */
> - ep->stid = cxgb3_alloc_stid(h->rdev.t3cdev_p, &t3c_client, ep);
> - if (ep->stid == -1) {
> - printk(KERN_ERR MOD "%s - cannot alloc atid.\n", __FUNCTION__);
> - err = -ENOMEM;
> + err = alloc_listener_list(ep);
> + if (err)
> goto fail2;
> - }
>
> state_set(&ep->com, LISTEN);
> err = listen_start(ep);
> - if (err)
> - goto fail3;
>
> - /* wait for pass_open_rpl */
> - wait_event(ep->com.waitq, ep->com.rpl_done);
> - err = ep->com.rpl_err;
> if (!err) {
> cm_id->provider_data = ep;
> + mutex_lock(&h->mutex);
> + list_add_tail(&ep->entry, &h->listen_eps);
> + mutex_unlock(&h->mutex);
Is there a race between listen_start() being called and inserting the ep
into the list? Could anything try to find the ep on the list after
listen_start returns?
> goto out;
> }
> -fail3:
> - cxgb3_free_stid(ep->com.tdev, ep->stid);
> + dealloc_listener_list(ep);
> fail2:
> cm_id->rem_ref(cm_id);
> put_ep(&ep->com);
> @@ -1923,18 +2084,20 @@ out:
> int iwch_destroy_listen(struct iw_cm_id *cm_id)
> {
> int err;
> + struct iwch_dev *h = to_iwch_dev(cm_id->device);
> struct iwch_listen_ep *ep = to_listen_ep(cm_id);
>
> PDBG("%s ep %p\n", __FUNCTION__, ep);
>
> might_sleep();
> + mutex_lock(&h->mutex);
> + list_del(&ep->entry);
> + mutex_unlock(&h->mutex);
> state_set(&ep->com, DEAD);
> ep->com.rpl_done = 0;
> ep->com.rpl_err = 0;
> err = listen_stop(ep);
> - wait_event(ep->com.waitq, ep->com.rpl_done);
> - cxgb3_free_stid(ep->com.tdev, ep->stid);
> - err = ep->com.rpl_err;
> + dealloc_listener_list(ep);
> cm_id->rem_ref(cm_id);
> put_ep(&ep->com);
> return err;
> diff --git a/drivers/infiniband/hw/cxgb3/iwch_cm.h b/drivers/infiniband/hw/cxgb3/iwch_cm.h
> index 6107e7c..23e5a22 100644
> --- a/drivers/infiniband/hw/cxgb3/iwch_cm.h
> +++ b/drivers/infiniband/hw/cxgb3/iwch_cm.h
> @@ -162,10 +162,19 @@ struct iwch_ep_common {
> int rpl_err;
> };
>
> -struct iwch_listen_ep {
> - struct iwch_ep_common com;
> +struct iwch_listen_entry {
> + struct list_head entry;
> unsigned int stid;
> + __be32 addr;
> +};
> +
> +struct iwch_listen_ep {
> + struct iwch_ep_common com; /* Must be first entry! */
> + struct list_head entry;
> + struct list_head listeners;
> int backlog;
> + int listen_count;
I didn't notice where this was used.
> + int listen_rpls;
or this.
> };
>
> struct iwch_ep {
> @@ -222,6 +231,8 @@ int iwch_resume_tid(struct iwch_ep *ep);
> void __free_ep(struct kref *kref);
> void iwch_rearp(struct iwch_ep *ep);
> int iwch_ep_redirect(void *ctx, struct dst_entry *old, struct dst_entry *new, struct l2t_entry *l2t);
> +void iwch_listeners_add_addr(struct iwch_dev *rnicp, __be32 addr);
> +void iwch_listeners_del_addr(struct iwch_dev *rnicp, __be32 addr);
>
> int __init iwch_cm_init(void);
> void __exit iwch_cm_term(void);
> _______________________________________________
> general mailing list
> general@lists.openfabrics.org
> http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
>
> To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
>
^ permalink raw reply
* Re: RTL8111 PCI Express Gigabit driver r8169 produces slow file transfers
From: Francois Romieu @ 2007-09-27 17:24 UTC (permalink / raw)
To: Achim Frase; +Cc: netdev
In-Reply-To: <1190898841.7919.14.camel@BeeB1t>
Achim Frase <achim.frase@googlemail.com> :
> [...]
> but I hope you could help me.
Yes. Please try any of:
- current 2.6.23-git
- 2.6.23-rc8 + patch below
diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
index b85ab4a..c921ec3 100644
--- a/drivers/net/r8169.c
+++ b/drivers/net/r8169.c
@@ -1228,7 +1228,10 @@ static void rtl8169_hw_phy_config(struct net_device *dev)
return;
}
- /* phy config for RTL8169s mac_version C chip */
+ if ((tp->mac_version != RTL_GIGA_MAC_VER_02) &&
+ (tp->mac_version != RTL_GIGA_MAC_VER_03))
+ return;
+
mdio_write(ioaddr, 31, 0x0001); //w 31 2 0 1
mdio_write(ioaddr, 21, 0x1000); //w 21 15 0 1000
mdio_write(ioaddr, 24, 0x65c7); //w 24 15 0 65c7
@@ -2567,6 +2570,15 @@ static void rtl8169_tx_interrupt(struct net_device *dev,
(TX_BUFFS_AVAIL(tp) >= MAX_SKB_FRAGS)) {
netif_wake_queue(dev);
}
+ /*
+ * 8168 hack: TxPoll requests are lost when the Tx packets are
+ * too close. Let's kick an extra TxPoll request when a burst
+ * of start_xmit activity is detected (if it is not detected,
+ * it is slow enough). -- FR
+ */
+ smp_rmb();
+ if (tp->cur_tx != dirty_tx)
+ RTL_W8(TxPoll, NPQ);
}
}
--
Ueimor
^ permalink raw reply related
* Re: [PATCH] net: Add network namespace clone & unshare support.
From: Eric W. Biederman @ 2007-09-27 17:14 UTC (permalink / raw)
To: Cedric Le Goater; +Cc: David Miller, Linux Containers, netdev, Andrew Morton
In-Reply-To: <46FB6F1E.8070105@fr.ibm.com>
Cedric Le Goater <clg@fr.ibm.com> writes:
>> diff --git a/include/linux/sched.h b/include/linux/sched.h
>> index a01ac6d..e10a0a8 100644
>> --- a/include/linux/sched.h
>> +++ b/include/linux/sched.h
>> @@ -27,6 +27,7 @@
>> #define CLONE_NEWUTS 0x04000000 /* New utsname group? */
>> #define CLONE_NEWIPC 0x08000000 /* New ipcs */
>> #define CLONE_NEWUSER 0x10000000 /* New user namespace */
>> +#define CLONE_NEWNET 0x20000000 /* New network namespace */
>
> This new flag is going to conflict with the pid namespace flag
> CLONE_NEWPID in -mm. It might be worth changing it to:
>
> #define CLONE_NEWNET 0x40000000
Interesting, it would have been nice if someone had caught this
detail earlier. Oh well.
Thanks for pointing this out, it's on my todo list to look into,
and ensure we resolve.
I'm confused because my notes have 0x80000000 for the pid namespace,
and 0x40000000 for the time namespace.
> The changes in nxproxy.c and fork.c will also conflict but I don't
> think we can do much about it for now.
They should also be fairly easy conflicts to resolve.
I guess we are likely to hit this conflict in the next -mm or the
merge window, which ever comes first.
Eric
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox