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_head list;
> /* iscsi iface record name */
> char name[ISCSI_MAX_IFACE_LEN];
> /* network layer iface name (eth0) */
> char netdev[IFNAMSIZ];
> char ipaddress[NI_MAXHOST];
> /*
> * TODO: we may have to make this bigger and interconnect
> * specific for infinniband
> */
> char hwaddress[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.
> */
> char alias[TARGET_NAME_MAXLEN + 1];
> char iname[TARGET_NAME_MAXLEN + 1];
> /*
> * Following fields are used to support UIO driver based
> * DHCP/ARP functionality for offload capable cards.
> */
> char nic_lib[PATH_MAX];
> char nic_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:<net_ifacename>. 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)(struct nic *, nic_interface_t *,
> struct packet *);
> int (*handle_irq) (struct nic *nic);
> } net_ops_t;
>
> int (*handle_irq) (struct nic *nic)
> Instead of using global nic_process_intr we should use SNIC specific
> user space interrupt handling.
>
> I have converted handle_iscsi_path_req callback into a direct call
> inside nic_nl.c, since its too generic.
>
> Regards
> Rakesh Ranjan
Thanks again.
-Ben
--~--~---------~--~----~------------~-------~--~----~
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
-~----------~----~----~----~------~----~------~--~---
