Re: [RFC]iscsid to run in conjunction with uIP stack
Hi Mike, No problems, I will look over the source and submit fixes and integrate the needed Broadcom user-space library code into libuip. Thanks Rakesh integrating the uIP code into the open-iscsi tree! -Ben On Sat, 2009-08-15 at 19:53 -0700, Mike Christie wrote: > On 07/24/2009 08:06 AM, Rakesh Ranjan wrote: > > Mike Christie wrote: > >> Sorry for the really late response on this. > >> > >> On 06/25/2009 08:25 AM, Rakesh Ranjan wrote: > >>> Li, I have attached the diff against git master, please go through it. I > >>> have merged the contents of uip directory and nic related files into one > >>> common libuip. So now offload dir looks like > >>> > >>> offload/drivers > >>> offload/libuip > >>> > >>> Mike, what are your thoughts about linking uip core code int iscsid. > >>> Weather it gonna be static or dynamic ? Also it would be very helpful if > >> I think we would want to do it static, because it will make the distro > >> boot and installer stuff easier. > >> > >>> you could help on adding additional iface options. > >>> > >>> Regards > >>> Rakesh Ranjan > >> Was the attached patch ok for a starting point? I will cleanup the > >> open-iscsi/user and include stuff and add the uip stuff as is and then > >> put it in a new branch for us to work off of. > > > > Hi Mike, > > > > Herein attached the latest patch against current master. I have made > > libuip part to be static, but userspace driver part is still dynamic. I > > think this way vendor can drop in their driver as shared library. > > > > I will merge it this way, but we have to set some rules about where the > modules will live so distros do not have to search the ifaces. > > > I did some clean up and added a fixed I had for initiator.c. > > Ben, this code does not support bnx2i. Could you add that and add fixes > you have in your standalone code? > > Oh yeah, this is in the open-iscsi git tree's offload branch so clone > the tree like normal then do > > git branch -a > git checkout origin/offload > git checkout -b my_offload > > > Rakesh thanks again for doing this. And Ben, thanks for your work on > bcrm uip and for adding bnx2i to this. > --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "open-iscsi" group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: [RFC]iscsid to run in conjunction with uIP stack
On 07/24/2009 08:06 AM, Rakesh Ranjan wrote: > Mike Christie wrote: >> Sorry for the really late response on this. >> >> On 06/25/2009 08:25 AM, Rakesh Ranjan wrote: >>> Li, I have attached the diff against git master, please go through it. I >>> have merged the contents of uip directory and nic related files into one >>> common libuip. So now offload dir looks like >>> >>> offload/drivers >>> offload/libuip >>> >>> Mike, what are your thoughts about linking uip core code int iscsid. >>> Weather it gonna be static or dynamic ? Also it would be very helpful if >> I think we would want to do it static, because it will make the distro >> boot and installer stuff easier. >> >>> you could help on adding additional iface options. >>> >>> Regards >>> Rakesh Ranjan >> Was the attached patch ok for a starting point? I will cleanup the >> open-iscsi/user and include stuff and add the uip stuff as is and then >> put it in a new branch for us to work off of. > > Hi Mike, > > Herein attached the latest patch against current master. I have made > libuip part to be static, but userspace driver part is still dynamic. I > think this way vendor can drop in their driver as shared library. > I will merge it this way, but we have to set some rules about where the modules will live so distros do not have to search the ifaces. I did some clean up and added a fixed I had for initiator.c. Ben, this code does not support bnx2i. Could you add that and add fixes you have in your standalone code? Oh yeah, this is in the open-iscsi git tree's offload branch so clone the tree like normal then do git branch -a git checkout origin/offload git checkout -b my_offload Rakesh thanks again for doing this. And Ben, thanks for your work on bcrm uip and for adding bnx2i to this. --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "open-iscsi" group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: [RFC]iscsid to run in conjunction with uIP stack
Mike Christie wrote: > Sorry for the really late response on this. > > On 06/25/2009 08:25 AM, Rakesh Ranjan wrote: >> >> Li, I have attached the diff against git master, please go through it. I >> have merged the contents of uip directory and nic related files into one >> common libuip. So now offload dir looks like >> >> offload/drivers >> offload/libuip >> >> Mike, what are your thoughts about linking uip core code int iscsid. >> Weather it gonna be static or dynamic ? Also it would be very helpful if > > I think we would want to do it static, because it will make the distro > boot and installer stuff easier. > >> you could help on adding additional iface options. >> >> Regards >> Rakesh Ranjan > > Was the attached patch ok for a starting point? I will cleanup the > open-iscsi/user and include stuff and add the uip stuff as is and then > put it in a new branch for us to work off of. Hi Mike, I have done some more changes to the basic design, I will send the changes to you by tomorrow. I think those changeset can be used as a starting point. Regards Rakesh Ranjan --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "open-iscsi" group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: [RFC]iscsid to run in conjunction with uIP stack
Sorry for the really late response on this. On 06/25/2009 08:25 AM, Rakesh Ranjan wrote: > > Li, I have attached the diff against git master, please go through it. I > have merged the contents of uip directory and nic related files into one > common libuip. So now offload dir looks like > > offload/drivers > offload/libuip > > Mike, what are your thoughts about linking uip core code int iscsid. > Weather it gonna be static or dynamic ? Also it would be very helpful if I think we would want to do it static, because it will make the distro boot and installer stuff easier. > you could help on adding additional iface options. > > Regards > Rakesh Ranjan Was the attached patch ok for a starting point? I will cleanup the open-iscsi/user and include stuff and add the uip stuff as is and then put it in a new branch for us to work off of. --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "open-iscsi" group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: [RFC]iscsid to run in conjunction with uIP stack
On Tue, 2009-06-23 at 09:34 -0700, Mike Christie wrote: > Michael Chan wrote: > > > > On Mon, 2009-06-22 at 16:51 -0700, Benjamin Li wrote: > >> On Mon, 2009-06-22 at 05:59 -0700, Rakesh Ranjan wrote: > >>> 4. Get rid of ISCSI_NL_GRP_UIP netlink message and use > >>> ISCSI_NL_GRP_ISCSID instead. Move the netlink processing in nic_nl.c > >>> into netlink. > >> Correct, it is safe to remove ISCSI_NL_GRP_UIP because we could then > >> rely on ISCSI_NL_GRP_ISCSID instead. netlink.c::ctldev_handle() would > >> need to be modified to handle 'ISCSI_KEVENT_PATH_REQ' and > >> 'ISCSI_KEVENT_IF_DOWN' > > > > This will be an ABI change. The 2.6.31 kernel today is using the 2 > > multicast groups. If we decide a single multicast group is better, I > > think we need to change the kernel code now. > > > > Mike, how do we do this? > > > > The patch would just be something like the attached, right? If that is > what you agree on, then we can send it now. > > If you guys think we need to wait and make sure it is the right path, I > think we can wait and break ABI here though. bnx2i/open-iscsi is a odd > driver where a good chunk is in userspace and userspace is just not done > in my opinion (there is no code in the upstream open-iscsi package and > distros that add code that is not upstream take on this type of risk). I > think the scsi maintainer might allow the change. I am not sure about > Dave Miller though. He seems more strict. > > Let me know what you want to do and I will ping James to make sure it is ok. > I think we should wait and keep the 2 separate groups for now. This is how it will work until uIP is combined with iscsid. Even when they are combined, it's not a big problem to have to listen to 2 multicast groups. For performance or other reasons, we may still want to have two threads listening for these 2 message types. --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "open-iscsi" group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: [RFC]iscsid to run in conjunction with uIP stack
Michael Chan wrote:
>
> On Mon, 2009-06-22 at 16:51 -0700, Benjamin Li wrote:
>> On Mon, 2009-06-22 at 05:59 -0700, Rakesh Ranjan wrote:
>>> 4. Get rid of ISCSI_NL_GRP_UIP netlink message and use
>>> ISCSI_NL_GRP_ISCSID instead. Move the netlink processing in nic_nl.c
>>> into netlink.
>> Correct, it is safe to remove ISCSI_NL_GRP_UIP because we could then
>> rely on ISCSI_NL_GRP_ISCSID instead. netlink.c::ctldev_handle() would
>> need to be modified to handle 'ISCSI_KEVENT_PATH_REQ' and
>> 'ISCSI_KEVENT_IF_DOWN'
>
> This will be an ABI change. The 2.6.31 kernel today is using the 2
> multicast groups. If we decide a single multicast group is better, I
> think we need to change the kernel code now.
>
> Mike, how do we do this?
>
The patch would just be something like the attached, right? If that is
what you agree on, then we can send it now.
If you guys think we need to wait and make sure it is the right path, I
think we can wait and break ABI here though. bnx2i/open-iscsi is a odd
driver where a good chunk is in userspace and userspace is just not done
in my opinion (there is no code in the upstream open-iscsi package and
distros that add code that is not upstream take on this type of risk). I
think the scsi maintainer might allow the change. I am not sure about
Dave Miller though. He seems more strict.
Let me know what you want to do and I will ping James to make sure it is ok.
--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to
open-iscsi+unsubscr...@googlegroups.com
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---
diff --git a/drivers/scsi/scsi_transport_iscsi.c b/drivers/scsi/scsi_transport_iscsi.c
index f3e6646..45b08f1 100644
--- a/drivers/scsi/scsi_transport_iscsi.c
+++ b/drivers/scsi/scsi_transport_iscsi.c
@@ -1011,7 +1011,7 @@ int iscsi_offload_mesg(struct Scsi_Host *shost,
memcpy((char *)ev + sizeof(*ev), data, data_size);
- return iscsi_multicast_skb(skb, ISCSI_NL_GRP_UIP, GFP_NOIO);
+ return iscsi_multicast_skb(skb, ISCSI_NL_GRP_ISCSID, GFP_NOIO);
}
EXPORT_SYMBOL_GPL(iscsi_offload_mesg);
@@ -1438,7 +1438,7 @@ iscsi_set_path(struct iscsi_transport *transport, struct iscsi_uevent *ev)
}
static int
-iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
+iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
{
int err = 0;
struct iscsi_uevent *ev = NLMSG_DATA(nlh);
@@ -1448,11 +1448,6 @@ iscsi_if_recv_msg(struct sk_buff *skb, struct nlmsghdr *nlh, uint32_t *group)
struct iscsi_cls_conn *conn;
struct iscsi_endpoint *ep = NULL;
- if (nlh->nlmsg_type == ISCSI_UEVENT_PATH_UPDATE)
- *group = ISCSI_NL_GRP_UIP;
- else
- *group = ISCSI_NL_GRP_ISCSID;
-
priv = iscsi_if_transport_lookup(iscsi_ptr(ev->transport_handle));
if (!priv)
return -EINVAL;
@@ -1579,7 +1574,6 @@ iscsi_if_rx(struct sk_buff *skb)
uint32_t rlen;
struct nlmsghdr *nlh;
struct iscsi_uevent *ev;
- uint32_t group;
nlh = nlmsg_hdr(skb);
if (nlh->nlmsg_len < sizeof(*nlh) ||
@@ -1592,7 +1586,7 @@ iscsi_if_rx(struct sk_buff *skb)
if (rlen > skb->len)
rlen = skb->len;
- err = iscsi_if_recv_msg(skb, nlh, &group);
+ err = iscsi_if_recv_msg(skb, nlh);
if (err) {
ev->type = ISCSI_KEVENT_IF_ERROR;
ev->iferror = err;
@@ -1606,8 +1600,9 @@ iscsi_if_rx(struct sk_buff *skb)
*/
if (ev->type == ISCSI_UEVENT_GET_STATS && !err)
break;
- err = iscsi_if_send_reply(group, nlh->nlmsg_seq,
-nlh->nlmsg_type, 0, 0, ev, sizeof(*ev));
+ err = iscsi_if_send_reply(ISCSI_NL_GRP_ISCSID,
+ nlh->nlmsg_seq, nlh->nlmsg_type,
+ 0, 0, ev, sizeof(*ev));
} while (err < 0 && err != -ECONNREFUSED);
skb_pull(skb, rlen);
}
diff --git a/include/scsi/iscsi_if.h b/include/scsi/iscsi_if.h
index 4426f00..da4fdff 100644
--- a/include/scsi/iscsi_if.h
+++ b/include/scsi/iscsi_if.h
@@ -26,7 +26,6 @@
#include
#define ISCSI_NL_GRP_ISCSID 1
-#define ISCSI_NL_GRP_UIP 2
#define UEVENT_BASE 10
#define KEVENT_BASE 100
Re: [RFC]iscsid to run in conjunction with uIP stack
Hi Li, Thanks for comments. Benjamin Li wrote: >> 2. Use set_net_config callback to configure SNIC using vendor provided >> library. >> 3. Do the necessary code changes in various module to integrate the >> whole functionality without need of any extra user space daemon. >> 4. Get rid of ISCSI_NL_GRP_UIP netlink message and use >> ISCSI_NL_GRP_ISCSID instead. Move the netlink processing in nic_nl.c >> into netlink. > > Correct, it is safe to remove ISCSI_NL_GRP_UIP because we could then > rely on ISCSI_NL_GRP_ISCSID instead. netlink.c::ctldev_handle() would > need to be modified to handle 'ISCSI_KEVENT_PATH_REQ' and > 'ISCSI_KEVENT_IF_DOWN' I have already modified netlink.c::ctldev_handle() to acknowledge these messages and provided a wrapper nic_nl.c::nic_process_iscsi_kevent_req() to do further processing, but it need to be cleaned more. > > Could you also send all the .git directory containing all the metadata? > This way I can see all the commits that you have made to your tree? No problem, will do that. >> B. Extend iface files to include following options >> "iface.nic_lib = vendor_lib.so" >> "iface.nic_dev = /dev/uioX" >> "iface.nic_args = additional SNIC specific args" >> > > Could we remove the option 'iface.nic_dev = /dev/uioX'? The option > 'iface.net_ifacename' should be enough to determine the uio char path. > 'iface.net_ifacename' will tell iscsid what network interface to use. > Then iscsid could scan all the entries /sys/class/uio/uio*/device/ for > the file entry net:. If there is a filename match then > iscsid has found the correct uio char path used to open the uio device. > This should simplify the configuration file or prevent conflicts with > the 'iface.net_ifacename' parameter. Yep that will be fine. >> D. I have introduced new directory offload under open-iscsi for >> following purposes. >> >> open-iscsi-head/offload >> open-iscsi-head/offload/vendor_lib => holds vendor specfic UIO library >> open-iscsi-head/offload/niclib => holds generic nic module >> open-iscsi-head/offload/uip => the uIP stack module >> open-iscsi-head/offload/uip/apps => holds various uip based module to >> provide dhcp and other functionality. >> > > Should these directories be placed under the 'usr' directory since they > will be linked with iscsid? I don't have preference over this, I used the initial layout because it seems more clean to me. But yep related code is going to be linked with iscsid anyway. Going through the cnic lib I found that to serve the iscsi_path_req, there is cnic_nl.c::cnic_arp_send, IMHO wouldn't it be more nice to have ARPOP handling in uip, which in turn can queue the packet for TX through nic indirection ? Regards Rakesh Ranjan --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "open-iscsi" group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: [RFC]iscsid to run in conjunction with uIP stack
On Mon, 2009-06-22 at 16:51 -0700, Benjamin Li wrote: > > On Mon, 2009-06-22 at 05:59 -0700, Rakesh Ranjan wrote: > > 4. Get rid of ISCSI_NL_GRP_UIP netlink message and use > > ISCSI_NL_GRP_ISCSID instead. Move the netlink processing in nic_nl.c > > into netlink. > > Correct, it is safe to remove ISCSI_NL_GRP_UIP because we could then > rely on ISCSI_NL_GRP_ISCSID instead. netlink.c::ctldev_handle() would > need to be modified to handle 'ISCSI_KEVENT_PATH_REQ' and > 'ISCSI_KEVENT_IF_DOWN' This will be an ABI change. The 2.6.31 kernel today is using the 2 multicast groups. If we decide a single multicast group is better, I think we need to change the kernel code now. Mike, how do we do this? > > > > B. Extend iface files to include following options > > "iface.nic_lib = vendor_lib.so" > > "iface.nic_dev = /dev/uioX" > > "iface.nic_args = additional SNIC specific args" > > > > Could we remove the option 'iface.nic_dev = /dev/uioX'? The option > 'iface.net_ifacename' should be enough to determine the uio char path. > 'iface.net_ifacename' will tell iscsid what network interface to use. > Then iscsid could scan all the entries /sys/class/uio/uio*/device/ for > the file entry net:. If there is a filename match then > iscsid has found the correct uio char path used to open the uio device. > This should simplify the configuration file or prevent conflicts with > the 'iface.net_ifacename' parameter. Yes, we should not count on the uio device being the same after every reboot. Thanks. --~--~-~--~~~---~--~~ You received this message because you are subscribed to the Google Groups "open-iscsi" group. To post to this group, send email to open-iscsi@googlegroups.com To unsubscribe from this group, send email to open-iscsi+unsubscr...@googlegroups.com For more options, visit this group at http://groups.google.com/group/open-iscsi -~--~~~~--~~--~--~---
Re: [RFC]iscsid to run in conjunction with uIP stack
Hi Rakesh,
Thanks for starting on this, I agree with you that for simplicity that
the uIP daemon should be integrated into the iscsid daemon. My comments
are below:
On Mon, 2009-06-22 at 05:59 -0700, Rakesh Ranjan wrote:
> Hi Guys,
>
> Following are my suggestions to get rid of vendor provided daemon for
> SNIC configuration and use iscsid for DHCP configuration and
> provisioning.
>
> 1. Remove ISCSID_UIP_IPC_GET_IFACE IPC message and related handlers.
You are correct, this is not necessary because the uIP application is
going away.
> 2. Use set_net_config callback to configure SNIC using vendor provided
> library.
> 3. Do the necessary code changes in various module to integrate the
> whole functionality without need of any extra user space daemon.
> 4. Get rid of ISCSI_NL_GRP_UIP netlink message and use
> ISCSI_NL_GRP_ISCSID instead. Move the netlink processing in nic_nl.c
> into netlink.
Correct, it is safe to remove ISCSI_NL_GRP_UIP because we could then
rely on ISCSI_NL_GRP_ISCSID instead. netlink.c::ctldev_handle() would
need to be modified to handle 'ISCSI_KEVENT_PATH_REQ' and
'ISCSI_KEVENT_IF_DOWN'
>
> I have done some initial modification in my private tree and I am
> attaching whole tree. It won't even compile for now since related build
> infra is missing. First I would like to have feedback/suggestions from
> you guys before going ahead.
Could you also send all the .git directory containing all the metadata?
This way I can see all the commits that you have made to your tree?
>
>
> A. I have modified iface_rec to include vendor library path, UIO dev
> node path.
>
> typedef struct iface_rec {
> struct list_headlist;
> /* iscsi iface record name */
> charname[ISCSI_MAX_IFACE_LEN];
> /* network layer iface name (eth0) */
> charnetdev[IFNAMSIZ];
> charipaddress[NI_MAXHOST];
> /*
>* TODO: we may have to make this bigger and interconnect
>* specific for infinniband
>*/
> charhwaddress[ISCSI_MAX_IFACE_LEN];
> char
> transport_name[ISCSI_TRANSPORT_NAME_MAXLEN];
> /*
>* This is only used for boot now, but the iser guys
>* can use this for their virtualization idea.
>*/
> charalias[TARGET_NAME_MAXLEN + 1];
> chariname[TARGET_NAME_MAXLEN + 1];
> /*
>* Following fields are used to support UIO driver based
>* DHCP/ARP functionality for offload capable cards.
>*/
> charnic_lib[PATH_MAX];
> charnic_dev[PATH_MAX];
> } iface_rec_t;
>
> B. Extend iface files to include following options
> "iface.nic_lib = vendor_lib.so"
> "iface.nic_dev = /dev/uioX"
> "iface.nic_args = additional SNIC specific args"
>
Could we remove the option 'iface.nic_dev = /dev/uioX'? The option
'iface.net_ifacename' should be enough to determine the uio char path.
'iface.net_ifacename' will tell iscsid what network interface to use.
Then iscsid could scan all the entries /sys/class/uio/uio*/device/ for
the file entry net:. If there is a filename match then
iscsid has found the correct uio char path used to open the uio device.
This should simplify the configuration file or prevent conflicts with
the 'iface.net_ifacename' parameter.
> C. I have also modified set_net_config callback parameter in
> iscsi_transport_template as following, since we only need to have
> DHCP/STATIC and nic lib information while processing the nic config.
>
> struct iscsi_transport_template {
> const char *name;
> uint8_t rdma;
> int (*ep_connect) (struct iscsi_conn *conn, int non_blocking);
> int (*ep_poll) (struct iscsi_conn *conn, int timeout_ms);
> void (*ep_disconnect) (struct iscsi_conn *conn);
> void (*create_conn) (struct iscsi_conn *conn);
> int (*set_net_config) (struct iface_rec *iface);
> };
Agreed, we can remove the session parameter.
>
> D. I have introduced new directory offload under open-iscsi for
> following purposes.
>
> open-iscsi-head/offload
> open-iscsi-head/offload/vendor_lib => holds vendor specfic UIO library
> open-iscsi-head/offload/niclib => holds generic nic module
> open-iscsi-head/offload/uip => the uIP stack module
> open-iscsi-head/offload/uip/apps => holds various uip based module to
> provide dhcp and other functionality.
>
Should these directories be placed under the 'usr' directory since they
will be linked with iscsid?
> E. nic_ops structure has been modified as following
>
> typedef struct nic_ops {
> struct nic_lib_ops lib_ops;
>
> int (*open)(struct nic *);
> int (*close)(struct nic *);
> int (*read)(struct nic *, struct packet *);
> int (*write)(str
