Re: [PATCH v2] iw_cxgb3: Support "iwarp-only" interfaces to avoid 4-tuple conflicts.
- From: Evgeniy Polyakov <johnpol@xxxxxxxxxxx>
- Date: Sun, 16 Sep 2007 18:22:41 +0400
Hi Steve.
On Sat, Sep 15, 2007 at 10:56:46AM -0500, Steve Wise (swise@xxxxxxxxxxxxxxxxxxxxx) wrote:
The iWARP driver must translate all listens on address 0.0.0.0 to the
set of rdma-only ip addresses for the device in question. This prevents
incoming connect requests to the TCP ipaddresses from going up the
rdma stack.
If the only solutions to solve a problem with hardware are to steal
packets or became a real device, then real device is much more
appropriate. Is that correct?
This is a real device. I don't understand your question? Packets
aren't being stolen.
I meant port from main network stack. Sorry for confusion.
+static void insert_ifa(struct iwch_dev *rnicp, struct in_ifaddr *ifa)
+{
+ struct iwch_addrlist *addr;
+
+ addr = kmalloc(sizeof *addr, GFP_KERNEL);
As a small nitpick: this wants to be sizeof(struct in_ifaddr)
No, insert_ifa() allocates a struct iwch_addrlist, which has 2 fields: a
list_head for linking, and a struct in_ifaddr pointer.
sizeof(struct iwch_addrlist) of course, not (*addr).
To simplify grep.
+ 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);
+}
What about providing error back to caller and fail to register?
There are two causes where this is called: 1) during module init to
populate the list of iwarp addresses. If we failed in that case then, I
_could_ then not register. 2) we get called via the notifier mechanism
when an address is added. If that fails, the caller doesn't care (since
we're on the notifier callout thread). But the code could perhaps
unregister the device. I prefer just logging an error in case 2. I'll
look into not registering if we cannot get any address due to lack of
memory. But there's another case: we load the module and the admin
hasn't yet created the ethX:iw interface.
Perhaps I should change the code to only register as a working rdma
device _when_ we get at least one ethX:iwY interface created? Whatchathink?
Does second case ends up with problem you described in the initial
e-mail not being fixed?
+static inline int is_iwarp_label(char *label)
+{
+ char *colon;
+
+ colon = strchr(label, ':');
+ if (colon && !strncmp(colon+1, "iw", 2))
+ return 1;
+ return 0;
+}
I.e. it is not allowed to create ':iw' alias for anyone else?
Well, looks crappy, but if it is the only solution...
It is kinda crappy. But I don't see a better solution. Any ideas?
Does creating the whole new netdevice is a too big overhead, or is it
considered bad idea?
+static struct iwch_listen_entry *alloc_listener(struct iwch_listen_ep
*ep,
+ __be32 addr)
Do you know, that cxgb3 function names suck? :)
Especially get_skb().
+{
+ struct iwch_dev *h = to_iwch_dev(ep->com.cm_id->device);
+ struct iwch_listen_entry *le;
+
+ le = kmalloc(sizeof *le, GFP_KERNEL);
Wants to be sizeof(struct iwch_listen_entry) and in other places too.
Do you mean I shouldn't use sizeof *le, but rather sizeof(struct
iwch_listen_entry)? Is that the preferred coding style?
Yes, exactly.
I skipped rdma internals of the patch, since I do not know it enough
to judge, but your approach looks good from core network point of view.
Maybe you should automatically create an alias each time new interface
is added so that admin would not care about proper aliases?
That would be much better IMO, but the problem is that I cannot create
an alias without an actual ip address. Unless we change the core
services to allow it.
Thanks for reviewing!
Steve.
--
Evgeniy Polyakov
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
- Follow-Ups:
- References:
- [PATCH v2] iw_cxgb3: Support "iwarp-only" interfaces to avoid 4-tuple conflicts.
- From: Steve Wise
- Re: [PATCH v2] iw_cxgb3: Support "iwarp-only" interfaces to avoid 4-tuple conflicts.
- From: Evgeniy Polyakov
- Re: [PATCH v2] iw_cxgb3: Support "iwarp-only" interfaces to avoid 4-tuple conflicts.
- From: Steve Wise
- [PATCH v2] iw_cxgb3: Support "iwarp-only" interfaces to avoid 4-tuple conflicts.
- Prev by Date: Re: Statement by SFLC (was Re: Wasting our Freedom)
- Next by Date: Re: Statement by SFLC (was Re: Wasting our Freedom)
- Previous by thread: Re: [PATCH v2] iw_cxgb3: Support "iwarp-only" interfaces to avoid 4-tuple conflicts.
- Next by thread: Re: [PATCH v2] iw_cxgb3: Support "iwarp-only" interfaces to avoid 4-tuple conflicts.
- Index(es):
Relevant Pages
|
