* 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: [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: [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: [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
* [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: [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
* 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" 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
* [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: 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 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: [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
* 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: [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: [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: [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: [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: [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: [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: David Miller @ 2007-09-27 20:56 UTC (permalink / raw)
To: ebiederm; +Cc: netdev, containers
In-Reply-To: <m1ps03hm8a.fsf@ebiederm.dsl.xmission.com>
From: ebiederm@xmission.com (Eric W. Biederman)
Date: Thu, 27 Sep 2007 14:44:37 -0600
> 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>
I've already removed the comment, so you'll have to give
me a patch that adds it back with the new content :-)
^ permalink raw reply
* [PATCH] rfkill: Move rfkill_switch_all out of global header
From: Ivo van Doorn @ 2007-09-27 22:07 UTC (permalink / raw)
To: davem; +Cc: Dmitry Torokhov, netdev, linux-wireless
rfkill_switch_all shouldn't be called by drivers directly,
instead they should send a signal over the input device.
To prevent confusion for driver developers, move the
function into a rfkill private header.
Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
diff --git a/include/linux/rfkill.h b/include/linux/rfkill.h
index f9a50da..67096b5 100644
--- a/include/linux/rfkill.h
+++ b/include/linux/rfkill.h
@@ -2,7 +2,7 @@
#define __RFKILL_H
/*
- * Copyright (C) 2006 Ivo van Doorn
+ * Copyright (C) 2006 - 2007 Ivo van Doorn
* Copyright (C) 2007 Dmitry Torokhov
*
* This program is free software; you can redistribute it and/or modify
@@ -84,6 +84,4 @@ void rfkill_free(struct rfkill *rfkill);
int rfkill_register(struct rfkill *rfkill);
void rfkill_unregister(struct rfkill *rfkill);
-void rfkill_switch_all(enum rfkill_type type, enum rfkill_state state);
-
#endif /* RFKILL_H */
diff --git a/net/rfkill/rfkill-input.c b/net/rfkill/rfkill-input.c
index 8e4516a..eaabf08 100644
--- a/net/rfkill/rfkill-input.c
+++ b/net/rfkill/rfkill-input.c
@@ -17,6 +17,8 @@
#include <linux/init.h>
#include <linux/rfkill.h>
+#include "rfkill-input.h"
+
MODULE_AUTHOR("Dmitry Torokhov <dtor@mail.ru>");
MODULE_DESCRIPTION("Input layer to RF switch connector");
MODULE_LICENSE("GPL");
diff --git a/net/rfkill/rfkill-input.h b/net/rfkill/rfkill-input.h
new file mode 100644
index 0000000..4dae500
--- /dev/null
+++ b/net/rfkill/rfkill-input.h
@@ -0,0 +1,16 @@
+/*
+ * Copyright (C) 2007 Ivo van Doorn
+ */
+
+/*
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation.
+ */
+
+#ifndef __RFKILL_INPUT_H
+#define __RFKILL_INPUT_H
+
+void rfkill_switch_all(enum rfkill_type type, enum rfkill_state state);
+
+#endif /* __RFKILL_INPUT_H */
diff --git a/net/rfkill/rfkill.c b/net/rfkill/rfkill.c
index 03ed7fd..00ee534 100644
--- a/net/rfkill/rfkill.c
+++ b/net/rfkill/rfkill.c
@@ -1,5 +1,5 @@
/*
- * Copyright (C) 2006 Ivo van Doorn
+ * Copyright (C) 2006 - 2007 Ivo van Doorn
* Copyright (C) 2007 Dmitry Torokhov
*
* This program is free software; you can redistribute it and/or modify
^ permalink raw reply related
* Re: [PATCH] rfkill: Move rfkill_switch_all out of global header
From: David Miller @ 2007-09-27 21:57 UTC (permalink / raw)
To: ivdoorn; +Cc: dtor, netdev, linux-wireless
In-Reply-To: <200709280007.41676.IvDoorn@gmail.com>
From: Ivo van Doorn <ivdoorn@gmail.com>
Date: Fri, 28 Sep 2007 00:07:41 +0200
> rfkill_switch_all shouldn't be called by drivers directly,
> instead they should send a signal over the input device.
>
> To prevent confusion for driver developers, move the
> function into a rfkill private header.
>
> Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
Applied to net-2.6.24, thanks.
^ permalink raw reply
* Re: ax88796: add 93cx6 eeprom support
From: Andrew Morton @ 2007-09-27 22:04 UTC (permalink / raw)
To: Magnus Damm
Cc: netdev, magnus.damm, lethal, jgarzik, ben-linux, Francois Romieu
In-Reply-To: <20070927105119.6515.35298.sendpatchset@clockwork.opensource.se>
On Thu, 27 Sep 2007 19:51:19 +0900
Magnus Damm <magnus.damm@gmail.com> wrote:
> ax88796: add 93cx6 eeprom support
>
> This patch hooks up the 93cx6 eeprom code to the ax88796 driver and modifies
> the ax88796 driver to read out the mac address from the eeprom. We need
> this for the ax88796 on certain SuperH boards. The pin configuration used
> to connect the eeprom to the ax88796 on these boards is the same as pointed
> out by the ax88796 datasheet, so we can probably reuse this code for multiple
> platforms in the future.
I'm showing a minor reject between this and Francois's git-r8169.patch.
***************
*** 21,33 ****
/*
Module: eeprom_93cx6
Abstract: EEPROM reader datastructures for 93cx6 chipsets.
- Supported chipsets: 93c46 & 93c66.
*/
/*
* EEPROM operation defines.
*/
#define PCI_EEPROM_WIDTH_93C46 6
#define PCI_EEPROM_WIDTH_93C66 8
#define PCI_EEPROM_WIDTH_OPCODE 3
#define PCI_EEPROM_WRITE_OPCODE 0x05
--- 21,34 ----
/*
Module: eeprom_93cx6
Abstract: EEPROM reader datastructures for 93cx6 chipsets.
+ Supported chipsets: 93c46/93c56/93c66.
*/
/*
* EEPROM operation defines.
*/
#define PCI_EEPROM_WIDTH_93C46 6
+ #define PCI_EEPROM_WIDTH_93C56 8
#define PCI_EEPROM_WIDTH_93C66 8
#define PCI_EEPROM_WIDTH_OPCODE 3
#define PCI_EEPROM_WRITE_OPCODE 0x05
You both made the same change to eeprom_93cx6.h. That all sounds good but
it would be comforting if you could review each other's work, please...
^ permalink raw reply
* Please pull 'upstream-davem' branch of wireless-2.6 (2007-09-27)
From: John W. Linville @ 2007-09-27 21:53 UTC (permalink / raw)
To: davem; +Cc: jeff, netdev, linux-wireless
[-- Attachment #1: Type: text/plain, Size: 6716 bytes --]
Dave & Jeff,
Here are some more wireless stack and driver updates for 2.6.24. Please
pull at your earliest convenience.
Thanks!
John
P.S. I preserved the point in net-2.6.24 from which upstream-davem is
based as 'master-davem'. Hth!
---
Individual patches available here:
http://www.kernel.org/pub/linux/kernel/people/linville/wireless-2.6/upstream-davem
---
The following changes since commit 08d37cb05abbdddcbf44b8266b4f6c2c9d8a3da7:
Andrew Morton (1):
[ATM] net/atm/lec.c: printk warning fix
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git upstream-davem
Ivo van Doorn (11):
rt2x00: Correct error in calculating rssi for link tuner
rt2x00: Make *_beacon_update static
rt2x00: Increase rt2x00usb_vendor_request timeout.
rt2x00: Correctly identify rt2561turbo
rt2x00: Reorganize rt2x00dev->flags
rt2x00: Add rt2x00dev->flags to debugfs
rt2x00: make rt2x00lib_stop_link_tuner() reentrant with link_tuner work
rt2x00: Stop link tuning when radio is down
rt2x00: Fix obvious typo in comment
rt2x00: Fix panic on rmmod with rfkill enabled
rt2x00: Release rt2x00 2.0.9
Johannes Berg (17):
mac80211: revamp interface and filter configuration
mac80211: validate VLAN interfaces better
mac80211: renumber and document the hardware flags
mac80211: document a lot more
wireless networking: move frame inline functions to generic header
mac80211: remove crypto algorithm typedef
mac80211: fix virtual interface locking
mac80211: fix interface initialisation and deinitialisation
mac80211: pass frames to monitor interfaces early
mac80211: consolidate decryption
mac80211: consolidate encryption
mac80211: remove ieee80211_wep_get_keyidx
mac80211: fix vlan bug
mac80211: fix TKIP IV update
mac80211: fix iff_promiscs, iff_allmultis race
mac80211: remove all prism2 ioctls
mac80211: remove generic IE for AP interfaces
John W. Linville (1):
iwlwifi: fix imcomplete conversion to print_mac API
Michael Buesch (2):
mac80211: bss_tim_clear must use ~ instead of !
rfkill: Fix documentation typos
Michael Wu (6):
adm8211: kill interrupt loop
adm8211: Improve writing of mac addrs to registers
adm8211: kill version printks
adm8211: Use revision from pci_dev
adm8211: Pass all TXed frames to tx_status_irqsafe
adm8211: Detect interface up/down in suspend/resume hooks correctly
Zhu Yi (15):
iwlwifi: replacing IPW with IWL in error messages
iwlwifi: workaournd REPLY_COMPRESSED_BA command in iwl_rx_handle
iwlwifi: Correction for sending beacon in config_ap
iwlwifi: clear station table in rxon unconditionally
iwlwifi: Fix typo in rate sacling algorithm
iwlwifi: fix add_station to avoid FW error
iwlwifi: removing unnecessary memset in 4965 rate scale
iwlwifi: add debugfs framework to rate scale
iwlwifi: add read rate scale table debugfs function
iwlwifi: limit printouts on hot path
iwlwifi: add supp_rates to rate scale sta private data
iwlwifi: rs_rate_scale_perform clean up
iwlwifi: set fixed rate through debugfs
iwlwifi: add debugfs rate scale stats
iwlwifi: Update iwlwifi version stamp to 1.1.17
drivers/net/wireless/adm8211.c | 313 +++++-----
drivers/net/wireless/adm8211.h | 5 +-
drivers/net/wireless/b43/b43.h | 15 +-
drivers/net/wireless/b43/main.c | 248 ++++----
drivers/net/wireless/b43legacy/b43legacy.h | 20 +-
drivers/net/wireless/b43legacy/main.c | 245 ++++----
drivers/net/wireless/iwlwifi/iwl-4965-rs.c | 252 +++++++--
drivers/net/wireless/iwlwifi/iwl-4965.c | 2 +-
drivers/net/wireless/iwlwifi/iwl-debug.h | 3 +
drivers/net/wireless/iwlwifi/iwl3945-base.c | 103 ++--
drivers/net/wireless/iwlwifi/iwl4965-base.c | 66 ++-
drivers/net/wireless/iwlwifi/iwlwifi.h | 2 +-
drivers/net/wireless/p54.h | 3 +-
drivers/net/wireless/p54common.c | 80 ++-
drivers/net/wireless/rt2x00/rt2400pci.c | 159 +++---
drivers/net/wireless/rt2x00/rt2500pci.c | 171 +++---
drivers/net/wireless/rt2x00/rt2500usb.c | 181 +++---
drivers/net/wireless/rt2x00/rt2x00.h | 70 +--
drivers/net/wireless/rt2x00/rt2x00config.c | 75 +--
drivers/net/wireless/rt2x00/rt2x00debug.c | 37 ++
drivers/net/wireless/rt2x00/rt2x00dev.c | 109 +++-
drivers/net/wireless/rt2x00/rt2x00lib.h | 4 +-
drivers/net/wireless/rt2x00/rt2x00mac.c | 155 ++----
drivers/net/wireless/rt2x00/rt2x00pci.c | 31 +-
drivers/net/wireless/rt2x00/rt2x00rfkill.c | 14 +-
drivers/net/wireless/rt2x00/rt2x00ring.h | 17 +-
drivers/net/wireless/rt2x00/rt2x00usb.c | 26 +-
drivers/net/wireless/rt2x00/rt2x00usb.h | 8 +-
drivers/net/wireless/rt2x00/rt61pci.c | 172 +++---
drivers/net/wireless/rt2x00/rt73usb.c | 179 +++---
drivers/net/wireless/rt2x00/rt73usb.h | 2 +-
drivers/net/wireless/rtl8187.h | 5 +-
drivers/net/wireless/rtl8187_dev.c | 129 ++++-
drivers/net/wireless/rtl818x.h | 1 +
include/linux/ieee80211.h | 61 ++
include/linux/rfkill.h | 6 +-
include/net/mac80211.h | 894 ++++++++++++++++-----------
net/mac80211/debugfs_netdev.c | 21 -
net/mac80211/hostapd_ioctl.h | 61 --
net/mac80211/ieee80211.c | 488 +++++++--------
net/mac80211/ieee80211_i.h | 46 +-
net/mac80211/ieee80211_iface.c | 77 ++-
net/mac80211/ieee80211_ioctl.c | 281 ---------
net/mac80211/ieee80211_key.h | 2 +-
net/mac80211/ieee80211_sta.c | 39 +-
net/mac80211/key.c | 2 +-
net/mac80211/rx.c | 414 ++++++++-----
net/mac80211/tkip.c | 16 +-
net/mac80211/tkip.h | 3 +-
net/mac80211/tx.c | 72 +--
net/mac80211/wep.c | 108 +++-
net/mac80211/wep.h | 14 +-
net/mac80211/wpa.c | 30 +-
net/mac80211/wpa.h | 8 +-
54 files changed, 2961 insertions(+), 2584 deletions(-)
delete mode 100644 net/mac80211/hostapd_ioctl.h
Omnibus patch attached as "upstream-davem.patch.bz2" due to size concerns.
--
John W. Linville
linville@tuxdriver.com
[-- Attachment #2: upstream-davem.patch.bz2 --]
[-- Type: application/x-bzip2, Size: 56590 bytes --]
^ permalink raw reply
* [RFC] Make TCP prequeue configurable
From: Eric Dumazet @ 2007-09-27 22:08 UTC (permalink / raw)
To: Linux Netdev List
[-- Attachment #1: Type: text/plain, Size: 863 bytes --]
Hi all
I am sure some of you are going to tell me that prequeue is not
all black :)
Thank you
[RFC] Make TCP prequeue configurable
The TCP prequeue thing is based on old facts, and has drawbacks.
1) It adds 48 bytes per 'struct tcp_sock'
2) It adds some ugly code in hot paths
3) It has a small hit ratio on typical servers using many sockets
4) It may have a high hit ratio on UP machines running one process,
where the prequeue adds litle gain. (In fact, letting the user
doing the copy after being woke up is better for cache reuse)
5) Doing a copy to user in softirq handler is not good, because of
potential page faults :(
6) Maybe the NET_DMA thing is the only thing that might need prequeue.
This patch introduces a CONFIG_TCP_PREQUEUE, automatically selected if
CONFIG_NET_DMA is on.
Signed-off-by: Eric Dumazet <dada1@cosmosbay.com>
[-- Attachment #2: net_prequeue.patch --]
[-- Type: text/plain, Size: 7426 bytes --]
diff --git a/drivers/dma/Kconfig b/drivers/dma/Kconfig
index 8f670da..14e3f01 100644
--- a/drivers/dma/Kconfig
+++ b/drivers/dma/Kconfig
@@ -16,6 +16,7 @@ comment "DMA Clients"
config NET_DMA
bool "Network: TCP receive copy offload"
depends on DMA_ENGINE && NET
+ select TCP_PREQUEUE
default y
---help---
This enables the use of DMA engines in the network stack to
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index c6b9f92..844a05e 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -268,11 +268,13 @@ struct tcp_sock {
/* Data for direct copy to user */
struct {
+#ifdef CONFIG_TCP_PREQUEUE
struct sk_buff_head prequeue;
struct task_struct *task;
struct iovec *iov;
int memory;
int len;
+#endif
#ifdef CONFIG_NET_DMA
/* members for async copy */
struct dma_chan *dma_chan;
diff --git a/include/net/tcp.h b/include/net/tcp.h
index 185c7ec..3430d8e 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -835,10 +835,12 @@ static inline int tcp_checksum_complete(struct sk_buff *skb)
static inline void tcp_prequeue_init(struct tcp_sock *tp)
{
+#ifdef CONFIG_TCP_PREQUEUE
tp->ucopy.task = NULL;
tp->ucopy.len = 0;
tp->ucopy.memory = 0;
skb_queue_head_init(&tp->ucopy.prequeue);
+#endif
#ifdef CONFIG_NET_DMA
tp->ucopy.dma_chan = NULL;
tp->ucopy.wakeup = 0;
@@ -857,6 +859,7 @@ static inline void tcp_prequeue_init(struct tcp_sock *tp)
*/
static inline int tcp_prequeue(struct sock *sk, struct sk_buff *skb)
{
+#ifdef CONFIG_TCP_PREQUEUE
struct tcp_sock *tp = tcp_sk(sk);
if (!sysctl_tcp_low_latency && tp->ucopy.task) {
@@ -882,6 +885,7 @@ static inline int tcp_prequeue(struct sock *sk, struct sk_buff *skb)
}
return 1;
}
+#endif
return 0;
}
diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
index fb79097..b770829 100644
--- a/net/ipv4/Kconfig
+++ b/net/ipv4/Kconfig
@@ -616,5 +616,20 @@ config TCP_MD5SIG
If unsure, say N.
+config TCP_PREQUEUE
+ bool "Enable TCP prequeue"
+ default n
+ ---help---
+ TCP PREQUEUE is an 'optimization' loosely based on the famous
+ "30 instruction TCP receive" Van Jacobson mail.
+ Van's trick is to deposit buffers into socket queue
+ on a device interrupt, to call tcp_recv function
+ on the receive process context and checksum and copy
+ the buffer to user space. smart...
+
+ Some people believe this 'optimization' is not really needed
+ but for some benchmarks. Also, taking potential pagefaults in
+ softirq handler seems a high price to pay.
+
source "net/ipv4/ipvs/Kconfig"
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 7e74011..8659533 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -994,6 +994,7 @@ void tcp_cleanup_rbuf(struct sock *sk, int copied)
tcp_send_ack(sk);
}
+#ifdef CONFIG_TCP_PREQUEUE
static void tcp_prequeue_process(struct sock *sk)
{
struct sk_buff *skb;
@@ -1011,6 +1012,7 @@ static void tcp_prequeue_process(struct sock *sk)
/* Clear memory counter. */
tp->ucopy.memory = 0;
}
+#endif
static inline struct sk_buff *tcp_recv_skb(struct sock *sk, u32 seq, u32 *off)
{
@@ -1251,6 +1253,7 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
tcp_cleanup_rbuf(sk, copied);
+#ifdef CONFIG_TCP_PREQUEUE
if (!sysctl_tcp_low_latency && tp->ucopy.task == user_recv) {
/* Install new reader */
if (!user_recv && !(flags & (MSG_TRUNC | MSG_PEEK))) {
@@ -1295,7 +1298,7 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
/* __ Set realtime policy in scheduler __ */
}
-
+#endif
if (copied >= target) {
/* Do not sleep, just process backlog. */
release_sock(sk);
@@ -1307,6 +1310,7 @@ int tcp_recvmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
tp->ucopy.wakeup = 0;
#endif
+#ifdef CONFIG_TCP_PREQUEUE
if (user_recv) {
int chunk;
@@ -1330,6 +1334,7 @@ do_prequeue:
}
}
}
+#endif
if ((flags & MSG_PEEK) && peek_seq != tp->copied_seq) {
if (net_ratelimit())
printk(KERN_DEBUG "TCP(%s:%d): Application bug, race in MSG_PEEK.\n",
@@ -1430,6 +1435,7 @@ skip_copy:
break;
} while (len > 0);
+#ifdef CONFIG_TCP_PREQUEUE
if (user_recv) {
if (!skb_queue_empty(&tp->ucopy.prequeue)) {
int chunk;
@@ -1448,6 +1454,7 @@ skip_copy:
tp->ucopy.task = NULL;
tp->ucopy.len = 0;
}
+#endif
#ifdef CONFIG_NET_DMA
if (tp->ucopy.dma_chan) {
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index bbad2cd..85d3a5c 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3467,6 +3467,7 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
goto out_of_window;
/* Ok. In sequence. In window. */
+#ifdef CONFIG_TCP_PREQUEUE
if (tp->ucopy.task == current &&
tp->copied_seq == tp->rcv_nxt && tp->ucopy.len &&
sock_owned_by_user(sk) && !tp->urg_data) {
@@ -3484,7 +3485,7 @@ static void tcp_data_queue(struct sock *sk, struct sk_buff *skb)
}
local_bh_disable();
}
-
+#endif
if (eaten <= 0) {
queue_and_out:
if (eaten < 0 &&
@@ -4078,6 +4079,7 @@ static void tcp_urg(struct sock *sk, struct sk_buff *skb, struct tcphdr *th)
}
}
+#ifdef CONFIG_TCP_PREQUEUE
static int tcp_copy_to_iovec(struct sock *sk, struct sk_buff *skb, int hlen)
{
struct tcp_sock *tp = tcp_sk(sk);
@@ -4100,6 +4102,7 @@ static int tcp_copy_to_iovec(struct sock *sk, struct sk_buff *skb, int hlen)
local_bh_disable();
return err;
}
+#endif
static __sum16 __tcp_checksum_complete_user(struct sock *sk, struct sk_buff *skb)
{
@@ -4279,8 +4282,9 @@ int tcp_rcv_established(struct sock *sk, struct sk_buff *skb,
}
} else {
int eaten = 0;
- int copied_early = 0;
+#ifdef CONFIG_TCP_PREQUEUE
+ int copied_early = 0;
if (tp->copied_seq == tp->rcv_nxt &&
len - tcp_header_len <= tp->ucopy.len) {
#ifdef CONFIG_NET_DMA
@@ -4315,6 +4319,7 @@ int tcp_rcv_established(struct sock *sk, struct sk_buff *skb,
if (copied_early)
tcp_cleanup_rbuf(sk, skb->len);
}
+#endif
if (!eaten) {
if (tcp_checksum_complete_user(sk, skb))
goto csum_error;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index 9c94627..7ac5bc1 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1916,8 +1916,10 @@ int tcp_v4_destroy_sock(struct sock *sk)
__skb_queue_purge(&sk->sk_async_wait_queue);
#endif
+#ifdef CONFIG_TCP_PREQUEUE
/* Clean prequeue, it must be empty really */
__skb_queue_purge(&tp->ucopy.prequeue);
+#endif
/* Clean up a referenced TCP bind bucket. */
if (inet_csk(sk)->icsk_bind_hash)
diff --git a/net/ipv4/tcp_timer.c b/net/ipv4/tcp_timer.c
index e9b151b..5f3b38c 100644
--- a/net/ipv4/tcp_timer.c
+++ b/net/ipv4/tcp_timer.c
@@ -167,7 +167,9 @@ static int tcp_write_timeout(struct sock *sk)
static void tcp_delack_timer(unsigned long data)
{
struct sock *sk = (struct sock*)data;
+#ifdef CONFIG_TCP_PREQUEUE
struct tcp_sock *tp = tcp_sk(sk);
+#endif
struct inet_connection_sock *icsk = inet_csk(sk);
bh_lock_sock(sk);
@@ -190,6 +192,7 @@ static void tcp_delack_timer(unsigned long data)
}
icsk->icsk_ack.pending &= ~ICSK_ACK_TIMER;
+#ifdef CONFIG_TCP_PREQUEUE
if (!skb_queue_empty(&tp->ucopy.prequeue)) {
struct sk_buff *skb;
@@ -200,6 +203,7 @@ static void tcp_delack_timer(unsigned long data)
tp->ucopy.memory = 0;
}
+#endif
if (inet_csk_ack_scheduled(sk)) {
if (!icsk->icsk_ack.pingpong) {
^ permalink raw reply related
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