| Message ID | 1364994415-8330-1-git-send-email-jsquyres@cisco.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Headers | show |
> Per off-list conversation with Roland, add some new enums for the > Cisco Ethernet Virtual NIC (it's not an RNIC/iWARP device, so it > doesn't fit in the same category as RDMA_NODE_RNIC / RDMA_TRANSPORT_IWARP). > > "USNIC" = "Userspace NIC". Can we get a better patch description? Maybe mention something about the NIC? Does it support all verbs? Is it for kernel users or just user space? Does this simply export a raw ethernet interface? > --- > examples/devinfo.c | 1 + > include/infiniband/verbs.h | 6 ++++-- > src/enum_strs.c | 5 +++-- > src/init.c | 5 ++++- > 4 files changed, 12 insertions(+), 5 deletions(-) > > diff --git a/examples/devinfo.c b/examples/devinfo.c > index 7dc0463..98a6b4b 100644 > --- a/examples/devinfo.c > +++ b/examples/devinfo.c > @@ -72,6 +72,7 @@ static const char *transport_str(enum ibv_transport_type > transport) > switch (transport) { > case IBV_TRANSPORT_IB: return "InfiniBand"; > case IBV_TRANSPORT_IWARP: return "iWARP"; > + case IBV_TRANSPORT_USNIC: return "USNIC"; > default: return "invalid transport"; > } > } > diff --git a/include/infiniband/verbs.h b/include/infiniband/verbs.h > index 6acfc81..6a6944c 100644 > --- a/include/infiniband/verbs.h > +++ b/include/infiniband/verbs.h > @@ -68,13 +68,15 @@ enum ibv_node_type { > IBV_NODE_CA = 1, > IBV_NODE_SWITCH, > IBV_NODE_ROUTER, > - IBV_NODE_RNIC > + IBV_NODE_RNIC, > + IBV_NODE_USNIC > }; > > enum ibv_transport_type { > IBV_TRANSPORT_UNKNOWN = -1, > IBV_TRANSPORT_IB = 0, > - IBV_TRANSPORT_IWARP > + IBV_TRANSPORT_IWARP, > + IBV_TRANSPORT_USNIC > }; > > enum ibv_device_cap_flags { > diff --git a/src/enum_strs.c b/src/enum_strs.c > index 54d71a6..0d68c75 100644 > --- a/src/enum_strs.c > +++ b/src/enum_strs.c > @@ -38,10 +38,11 @@ const char *ibv_node_type_str(enum ibv_node_type node_type) > [IBV_NODE_CA] = "InfiniBand channel adapter", > [IBV_NODE_SWITCH] = "InfiniBand switch", > [IBV_NODE_ROUTER] = "InfiniBand router", > - [IBV_NODE_RNIC] = "iWARP NIC" > + [IBV_NODE_RNIC] = "iWARP NIC", > + [IBV_NODE_USNIC] = "Ethernet USNIC" > }; > > - if (node_type < IBV_NODE_CA || node_type > IBV_NODE_RNIC) > + if (node_type < IBV_NODE_CA || node_type > IBV_NODE_USNIC) > return "unknown"; > > return node_type_str[node_type]; > diff --git a/src/init.c b/src/init.c > index 8d6786e..e4ef001 100644 > --- a/src/init.c > +++ b/src/init.c > @@ -346,7 +346,7 @@ static struct ibv_device *try_driver(struct ibv_driver > *driver, > dev->node_type = IBV_NODE_UNKNOWN; > } else { > dev->node_type = strtol(value, NULL, 10); > - if (dev->node_type < IBV_NODE_CA || dev->node_type > IBV_NODE_RNIC) > + if (dev->node_type < IBV_NODE_CA || dev->node_type > > IBV_NODE_USNIC) > dev->node_type = IBV_NODE_UNKNOWN; > } > > @@ -359,6 +359,9 @@ static struct ibv_device *try_driver(struct ibv_driver > *driver, > case IBV_NODE_RNIC: > dev->transport_type = IBV_TRANSPORT_IWARP; > break; > + case IBV_NODE_USNIC: > + dev->transport_type = IBV_TRANSPORT_USNIC; > + break; > default: > dev->transport_type = IBV_TRANSPORT_UNKNOWN; > break; > -- > 1.8.1.1 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Apr 3, 2013 at 5:49 PM, Hefty, Sean <sean.hefty@intel.com> wrote: >> Per off-list conversation with Roland, add some new enums for the >> Cisco Ethernet Virtual NIC (it's not an RNIC/iWARP device, so it >> doesn't fit in the same category as RDMA_NODE_RNIC / >> RDMA_TRANSPORT_IWARP). "USNIC" = "Userspace NIC". > Can we get a better patch description? > Maybe mention something about the NIC? Does it support all verbs? Is it for kernel > users or just user space? Does this simply export a raw ethernet interface? Jeff, I agree with Sean, there's not much point to review/discuss these general/pre-step patches without seeing some actual device specific kernel (if there are such or user space code if there aren't any kernel ones) code. e.g you can submit the two kernel pre-step patches as the two first pieces in a series that has the driver code. Or. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Apr 3, 2013, at 10:49 AM, "Hefty, Sean" <sean.hefty@intel.com> wrote: > Can we get a better patch description? > > Maybe mention something about the NIC? Does it support all verbs? Is it for kernel users or just user space? Does this simply export a raw ethernet interface? Sure. For a little background, the 2nd-generation Cisco VIC has been available since last year (IIRC): http://www.cisco.com/en/US/products/ps10277/prod_module_series_home.html. It's a converged 10G Ethernet adapter available in a variety of form factors (e.g., 2x10G on PCIe and Mezz). We'll be providing a UD verbs kernel driver and libibverbs plugin for OS bypass. It is currently going through QA and debugging; it'll probably take a bit more time before we can submit good patches. The main intended use for this driver is userspace/libibverbs applications, but I suppose it could be used by kernel applications, too. The wire protocol transport that is uses underneath will initially be a very simple L2-Ethernet based frame (DMAC, SMAC, ET, QP num, etc.). We are not exposing a RAW interface at this time; the libibverbs plugin will provide UD QP functionality. After some off-list discussion with Roland, we chose to create new IBV_*_USNIC enums because none of the current enums were accurate for our device. It's an Ethernet NIC, but it's not an RNIC. It's an Ethernet-based transport, but it's not iWARP. The reason we're asking for these IBV_*_USNIC enums now -- before we've submitted the driver -- is because we're targeting getting our driver included in RHEL 6.5. There's a bit of a chicken-and-egg issue here: they'll accept our patches for a new hardware driver while that driver is being worked upstream. But they (rightfully) won't accept patches to IB core and libibverbs until they've been vetted by the community. Hence, even though our driver is slowly working its way through QA and not available yet, we wanted to submit these new enums upstream for community approval so that they can be included in RHEL 6.5. Does that help?
On Apr 3, 2013, at 2:45 PM, Or Gerlitz <or.gerlitz@gmail.com> wrote: > Jeff, I agree with Sean, there's not much point to review/discuss > these general/pre-step patches without seeing some actual device > specific kernel (if there are such or user space code if there aren't > any kernel ones) code. e.g you can submit the two kernel pre-step > patches as the two first pieces in a series that has the driver code. Unfortunately, not yet. I just sent another mail that explained our rationale: our kernel driver and libibverbs plugin code are working their way through QA. It'll take a little time before we can submit good patches for these. The main driving factor for submitting these new enums is so that they can be included in RHEL 6.5.
> The reason we're asking for these IBV_*_USNIC enums now -- before we've > submitted the driver -- is because we're targeting getting our driver included > in RHEL 6.5. There's a bit of a chicken-and-egg issue here: they'll accept our > patches for a new hardware driver while that driver is being worked upstream. > But they (rightfully) won't accept patches to IB core and libibverbs until > they've been vetted by the community. Hence, even though our driver is slowly > working its way through QA and not available yet, we wanted to submit these new > enums upstream for community approval so that they can be included in RHEL 6.5. I understand the issue. In the end, these are kernel changes with no actual users of those changes... But then they are also just small changes to a framework... Just thinking aloud here, but what if we added 'RDMA_NODE_VENDOR' instead? Then other fields, such as transport, become vendor specific. - Sean -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Jeff Squyres (jsquyres) <jsquyres@cisco.com> wrote: > Sure. For a little background, the 2nd-generation Cisco VIC has been available > since last year (IIRC): http://www.cisco.com/en/US/products/ps10277 > /prod_module_series_home.html. It's a converged 10G Ethernet adapter available > in a variety of form factors (e.g., 2x10G on PCIe and Mezz). > After some off-list discussion with Roland, we chose to create new IBV_*_USNIC > enums because none of the current enums were accurate for our device. It's an > Ethernet NIC, but it's not an RNIC. It's an Ethernet-based transport, but it's not > iWARP. > > The reason we're asking for these IBV_*_USNIC enums now -- before we've submitted the driver -- is because we're targeting getting our driver included in RHEL 6.5. There's a bit of a chicken-and-egg issue here: they'll accept our patches for a new hardware driver while that driver is being worked upstream. But they (rightfully) won't accept patches to IB core and libibverbs until they've been vetted by the community. Hence, even though our driver is slowly working its way through QA and not available yet, we wanted to submit these new enums upstream for community approval so that they can be included in RHEL 6.5. > Does that help? yes it does, but I still think we need to see the driver code in order to conduct proper /better review and maybe even accept the proposed changes to the IB core. You can submit it as RFC which means "you can look on it, and give me comments, but don't pick it up yet" Or. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Forgive the top reply; I'm actually on vacation this week and currently only have email access on my phone... I'm not sure what you're asking me to do. Are you asking us to submit our known-buggy-and-not-yet-complete driver just to get two enums approved? Sent from my phone. No type good. On Apr 4, 2013, at 5:27 PM, "Or Gerlitz" <or.gerlitz@gmail.com> wrote: > Jeff Squyres (jsquyres) <jsquyres@cisco.com> wrote: > >> Sure. For a little background, the 2nd-generation Cisco VIC has been available >> since last year (IIRC): http://www.cisco.com/en/US/products/ps10277 >> /prod_module_series_home.html. It's a converged 10G Ethernet adapter available > in a variety of form factors (e.g., 2x10G on PCIe and Mezz). > >> After some off-list discussion with Roland, we chose to create new IBV_*_USNIC >> enums because none of the current enums were accurate for our device. It's an >> Ethernet NIC, but it's not an RNIC. It's an Ethernet-based transport, but it's not >> iWARP. > >> >> The reason we're asking for these IBV_*_USNIC enums now -- before we've submitted the driver -- is because we're targeting getting our driver included in RHEL 6.5. There's a bit of a chicken-and-egg issue here: they'll accept our patches for a new hardware driver while that driver is being worked upstream. But they (rightfully) won't accept patches to IB core and libibverbs until they've been vetted by the community. Hence, even though our driver is slowly working its way through QA and not available yet, we wanted to submit these new enums upstream for community approval so that they can be included in RHEL 6.5. > >> Does that help? > > yes it does, but I still think we need to see the driver code in order > to conduct proper /better review and maybe even accept the proposed > changes to the IB core. You can submit it as RFC which means "you can > look on it, and give me comments, but don't pick it up yet" > > Or. > -- > To unsubscribe from this list: send the line "unsubscribe linux-rdma" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Per my previous email, forgive my top reply... RDMA_NODE_VENDOR would be great, actually. Should I work up a patch for that? Sent from my phone. No type good. On Apr 4, 2013, at 10:32 AM, "Hefty, Sean" <sean.hefty@intel.com> wrote: >> The reason we're asking for these IBV_*_USNIC enums now -- before we've >> submitted the driver -- is because we're targeting getting our driver included >> in RHEL 6.5. There's a bit of a chicken-and-egg issue here: they'll accept our >> patches for a new hardware driver while that driver is being worked upstream. >> But they (rightfully) won't accept patches to IB core and libibverbs until >> they've been vetted by the community. Hence, even though our driver is slowly >> working its way through QA and not available yet, we wanted to submit these new >> enums upstream for community approval so that they can be included in RHEL 6.5. > > I understand the issue. > > In the end, these are kernel changes with no actual users of those changes... But then they are also just small changes to a framework... > > Just thinking aloud here, but what if we added 'RDMA_NODE_VENDOR' instead? Then other fields, such as transport, become vendor specific. > > - Sean -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> RDMA_NODE_VENDOR would be great, actually. Should I work up a patch for that?
I would prefer taking this approach and would be fine accepting such a change.
Roland, do you have an opinion on this?
- Sean
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Apr 5, 2013 at 11:19 AM, Jeff Squyres (jsquyres)
<jsquyres@cisco.com> wrote:
> I'm not sure what you're asking me to do. Are you asking us to submit our known-buggy-and-not-yet-complete driver just to get two enums approved?
I think the idea is that without context, it's hard to know if adding
these enums makes sense or not. And I'm sorry but I'm not that
sympathetic to "my code isn't ready but you have to take this
out-of-context patch so I can meet Red Hat's arbitrary schedule."
- R.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
On Apr 5, 2013, at 4:40 PM, Roland Dreier <roland@purestorage.com> wrote: > I think the idea is that without context, it's hard to know if adding > these enums makes sense or not. And I'm sorry but I'm not that > sympathetic to "my code isn't ready but you have to take this > out-of-context patch so I can meet Red Hat's arbitrary schedule." Ok, fair enough. It'll be a few weeks before we can submit usnic.ko, so I'll re-bring up the IBV_NODE_VENDOR/related patches then. I think the MTU discussion is still relevant, however -- there seems to be a larger design issue there. I'll go reply separately on that thread.
diff --git a/examples/devinfo.c b/examples/devinfo.c index 7dc0463..98a6b4b 100644 --- a/examples/devinfo.c +++ b/examples/devinfo.c @@ -72,6 +72,7 @@ static const char *transport_str(enum ibv_transport_type transport) switch (transport) { case IBV_TRANSPORT_IB: return "InfiniBand"; case IBV_TRANSPORT_IWARP: return "iWARP"; + case IBV_TRANSPORT_USNIC: return "USNIC"; default: return "invalid transport"; } } diff --git a/include/infiniband/verbs.h b/include/infiniband/verbs.h index 6acfc81..6a6944c 100644 --- a/include/infiniband/verbs.h +++ b/include/infiniband/verbs.h @@ -68,13 +68,15 @@ enum ibv_node_type { IBV_NODE_CA = 1, IBV_NODE_SWITCH, IBV_NODE_ROUTER, - IBV_NODE_RNIC + IBV_NODE_RNIC, + IBV_NODE_USNIC }; enum ibv_transport_type { IBV_TRANSPORT_UNKNOWN = -1, IBV_TRANSPORT_IB = 0, - IBV_TRANSPORT_IWARP + IBV_TRANSPORT_IWARP, + IBV_TRANSPORT_USNIC }; enum ibv_device_cap_flags { diff --git a/src/enum_strs.c b/src/enum_strs.c index 54d71a6..0d68c75 100644 --- a/src/enum_strs.c +++ b/src/enum_strs.c @@ -38,10 +38,11 @@ const char *ibv_node_type_str(enum ibv_node_type node_type) [IBV_NODE_CA] = "InfiniBand channel adapter", [IBV_NODE_SWITCH] = "InfiniBand switch", [IBV_NODE_ROUTER] = "InfiniBand router", - [IBV_NODE_RNIC] = "iWARP NIC" + [IBV_NODE_RNIC] = "iWARP NIC", + [IBV_NODE_USNIC] = "Ethernet USNIC" }; - if (node_type < IBV_NODE_CA || node_type > IBV_NODE_RNIC) + if (node_type < IBV_NODE_CA || node_type > IBV_NODE_USNIC) return "unknown"; return node_type_str[node_type]; diff --git a/src/init.c b/src/init.c index 8d6786e..e4ef001 100644 --- a/src/init.c +++ b/src/init.c @@ -346,7 +346,7 @@ static struct ibv_device *try_driver(struct ibv_driver *driver, dev->node_type = IBV_NODE_UNKNOWN; } else { dev->node_type = strtol(value, NULL, 10); - if (dev->node_type < IBV_NODE_CA || dev->node_type > IBV_NODE_RNIC) + if (dev->node_type < IBV_NODE_CA || dev->node_type > IBV_NODE_USNIC) dev->node_type = IBV_NODE_UNKNOWN; } @@ -359,6 +359,9 @@ static struct ibv_device *try_driver(struct ibv_driver *driver, case IBV_NODE_RNIC: dev->transport_type = IBV_TRANSPORT_IWARP; break; + case IBV_NODE_USNIC: + dev->transport_type = IBV_TRANSPORT_USNIC; + break; default: dev->transport_type = IBV_TRANSPORT_UNKNOWN; break;