Re: [PATCH 0/7] Enable iSCSI offload drivers to use information from iface.
On Wed, Jun 14, 2017 at 10:47 AM, Robert LeBlanc wrote: > On Wed, Jun 14, 2017 at 3:20 AM, Rangankar, Manish > wrote: >> >> On 13/06/17 10:19 PM, "Robert LeBlanc" wrote: >> >>>On Wed, Jun 7, 2017 at 12:30 PM, Robert LeBlanc >>>wrote: On Wed, Jun 7, 2017 at 10:28 AM, Chris Leech wrote: > On Tue, Jun 06, 2017 at 12:07:10PM -0600, Robert LeBlanc wrote: >> This patchset enables iSCSI offload drivers to have access to the >>iface >> information provided by iscsid. This allows users to have more control >> of how the driver connects to the iSCSI target. iSER is updated to use >> iface.ipaddress to set the source IP address if configured. This >>allows >> iSER to use multiple ports on the same network or in more complicated >> routed configurations. >> >> Since there is already a change to the function parameters, dst_addr >> is upgraded to sockaddr_storage so that it is more future proof and >>makes >> the size of the struct static and not dependent on checking the >>SA_FAMILY. >> >> This is dependent on updates to Open-iSCSI. > > Hi Robert, > > I don't think that passing the iface_rec structure directly from the > iscsid internals into a netlink message is a good way to go about this. > It's really big, there's an embedded list_head with user address > pointers that needs to be left out, and there are 32/64-bit layout > differences. > > Let me take a look at how you're proposing using this info for iSER, if > it makes sense I think we should come up with a better designed > structure for passing the information. > > Thanks, > Chris > Chris, Thank you for your feedback. I agree that the entire iface is probably overkill, it was more of a proof of concept. We are only using the ipaddress in the iface for iSER (in my patch), but I could see other drivers benefiting from some of the other data in the iface (mac, interface_name, vlan, etc) so I didn't want to be too restrictive so that it wouldn't have to be extended later. I've not worked on userspace/kernel interaction before so I need some guidance to make the transition between userspace and kernel versions smoother. This patchset works for what we need and it is very important for us (and I'm sure others once the feature is available) and I'm happy to put in the time to get it accepted upstream, I'm just new to kernel development and need some guidance. >>> >>>Are there other comments/ideas/suggestions specifically from the >>>iSCSI/iSER guys? I'd like to keep this patch moving. >> >> Considering partial iSCSI offload solution (like bnx2i and qedi) point of >> view, we liked the idea from Hannes to create TAP interface to associate >> with each iSCSI offload interface, which will allow us to use userspace >> tools for configuration. I haven't dig into its details yet, but at higher >> level it looks like this will help us to move away from our dependency >> over iscsiuio. > > I'm having a hard time wrapping my head around this idea. How can > configuring a TAP (separate Ethernet device) affect the offload NIC. I > don't see how you can bind to the right interface with the IP address > or how that is passed to rdma_connect() using a TAP. Is TAP used > instead of netlink for communicating between userspace and the kernel? > Obviously by my questions, at the moment I'm not sure how to approach > this at all and would be the wrong person to make this happen. > > If someone can explain how this would work and point to some code that > does something like this (examples are good for me), I can try to > create a patch with TAPs. As I mentioned before, this is important for > us and I'm willing to put in the time to learn and code, but I'm > really lost at this point. > > Thank you, > > > Robert LeBlanc > PGP Fingerprint 79A2 9CA4 6CC4 45DD A904 C70E E654 3BB2 FA62 B9F1 Can we get some more discussion/enlightenment on this? It would be really nice to have this in 4.14 and time is getting short. Thanks. Robert LeBlanc PGP Fingerprint 79A2 9CA4 6CC4 45DD A904 C70E E654 3BB2 FA62 B9F1
Re: [PATCH 0/7] Enable iSCSI offload drivers to use information from iface.
On Wed, Jun 14, 2017 at 3:20 AM, Rangankar, Manish wrote: > > On 13/06/17 10:19 PM, "Robert LeBlanc" wrote: > >>On Wed, Jun 7, 2017 at 12:30 PM, Robert LeBlanc >>wrote: >>> On Wed, Jun 7, 2017 at 10:28 AM, Chris Leech wrote: On Tue, Jun 06, 2017 at 12:07:10PM -0600, Robert LeBlanc wrote: > This patchset enables iSCSI offload drivers to have access to the >iface > information provided by iscsid. This allows users to have more control > of how the driver connects to the iSCSI target. iSER is updated to use > iface.ipaddress to set the source IP address if configured. This >allows > iSER to use multiple ports on the same network or in more complicated > routed configurations. > > Since there is already a change to the function parameters, dst_addr > is upgraded to sockaddr_storage so that it is more future proof and >makes > the size of the struct static and not dependent on checking the >SA_FAMILY. > > This is dependent on updates to Open-iSCSI. Hi Robert, I don't think that passing the iface_rec structure directly from the iscsid internals into a netlink message is a good way to go about this. It's really big, there's an embedded list_head with user address pointers that needs to be left out, and there are 32/64-bit layout differences. Let me take a look at how you're proposing using this info for iSER, if it makes sense I think we should come up with a better designed structure for passing the information. Thanks, Chris >>> >>> Chris, >>> >>> Thank you for your feedback. I agree that the entire iface is probably >>> overkill, it was more of a proof of concept. We are only using the >>> ipaddress in the iface for iSER (in my patch), but I could see other >>> drivers benefiting from some of the other data in the iface (mac, >>> interface_name, vlan, etc) so I didn't want to be too restrictive so >>> that it wouldn't have to be extended later. I've not worked on >>> userspace/kernel interaction before so I need some guidance to make >>> the transition between userspace and kernel versions smoother. >>> >>> This patchset works for what we need and it is very important for us >>> (and I'm sure others once the feature is available) and I'm happy to >>> put in the time to get it accepted upstream, I'm just new to kernel >>> development and need some guidance. >> >>Are there other comments/ideas/suggestions specifically from the >>iSCSI/iSER guys? I'd like to keep this patch moving. > > Considering partial iSCSI offload solution (like bnx2i and qedi) point of > view, we liked the idea from Hannes to create TAP interface to associate > with each iSCSI offload interface, which will allow us to use userspace > tools for configuration. I haven't dig into its details yet, but at higher > level it looks like this will help us to move away from our dependency > over iscsiuio. I'm having a hard time wrapping my head around this idea. How can configuring a TAP (separate Ethernet device) affect the offload NIC. I don't see how you can bind to the right interface with the IP address or how that is passed to rdma_connect() using a TAP. Is TAP used instead of netlink for communicating between userspace and the kernel? Obviously by my questions, at the moment I'm not sure how to approach this at all and would be the wrong person to make this happen. If someone can explain how this would work and point to some code that does something like this (examples are good for me), I can try to create a patch with TAPs. As I mentioned before, this is important for us and I'm willing to put in the time to learn and code, but I'm really lost at this point. Thank you, Robert LeBlanc PGP Fingerprint 79A2 9CA4 6CC4 45DD A904 C70E E654 3BB2 FA62 B9F1
Re: [PATCH 0/7] Enable iSCSI offload drivers to use information from iface.
On 13/06/17 10:19 PM, "Robert LeBlanc" wrote: >On Wed, Jun 7, 2017 at 12:30 PM, Robert LeBlanc >wrote: >> On Wed, Jun 7, 2017 at 10:28 AM, Chris Leech wrote: >>> On Tue, Jun 06, 2017 at 12:07:10PM -0600, Robert LeBlanc wrote: This patchset enables iSCSI offload drivers to have access to the iface information provided by iscsid. This allows users to have more control of how the driver connects to the iSCSI target. iSER is updated to use iface.ipaddress to set the source IP address if configured. This allows iSER to use multiple ports on the same network or in more complicated routed configurations. Since there is already a change to the function parameters, dst_addr is upgraded to sockaddr_storage so that it is more future proof and makes the size of the struct static and not dependent on checking the SA_FAMILY. This is dependent on updates to Open-iSCSI. >>> >>> Hi Robert, >>> >>> I don't think that passing the iface_rec structure directly from the >>> iscsid internals into a netlink message is a good way to go about this. >>> It's really big, there's an embedded list_head with user address >>> pointers that needs to be left out, and there are 32/64-bit layout >>> differences. >>> >>> Let me take a look at how you're proposing using this info for iSER, if >>> it makes sense I think we should come up with a better designed >>> structure for passing the information. >>> >>> Thanks, >>> Chris >>> >> >> Chris, >> >> Thank you for your feedback. I agree that the entire iface is probably >> overkill, it was more of a proof of concept. We are only using the >> ipaddress in the iface for iSER (in my patch), but I could see other >> drivers benefiting from some of the other data in the iface (mac, >> interface_name, vlan, etc) so I didn't want to be too restrictive so >> that it wouldn't have to be extended later. I've not worked on >> userspace/kernel interaction before so I need some guidance to make >> the transition between userspace and kernel versions smoother. >> >> This patchset works for what we need and it is very important for us >> (and I'm sure others once the feature is available) and I'm happy to >> put in the time to get it accepted upstream, I'm just new to kernel >> development and need some guidance. > >Are there other comments/ideas/suggestions specifically from the >iSCSI/iSER guys? I'd like to keep this patch moving. Considering partial iSCSI offload solution (like bnx2i and qedi) point of view, we liked the idea from Hannes to create TAP interface to associate with each iSCSI offload interface, which will allow us to use userspace tools for configuration. I haven't dig into its details yet, but at higher level it looks like this will help us to move away from our dependency over iscsiuio. Thanks, Manish R.
Re: [PATCH 0/7] Enable iSCSI offload drivers to use information from iface.
On Wed, Jun 7, 2017 at 12:30 PM, Robert LeBlanc wrote: > On Wed, Jun 7, 2017 at 10:28 AM, Chris Leech wrote: >> On Tue, Jun 06, 2017 at 12:07:10PM -0600, Robert LeBlanc wrote: >>> This patchset enables iSCSI offload drivers to have access to the iface >>> information provided by iscsid. This allows users to have more control >>> of how the driver connects to the iSCSI target. iSER is updated to use >>> iface.ipaddress to set the source IP address if configured. This allows >>> iSER to use multiple ports on the same network or in more complicated >>> routed configurations. >>> >>> Since there is already a change to the function parameters, dst_addr >>> is upgraded to sockaddr_storage so that it is more future proof and makes >>> the size of the struct static and not dependent on checking the SA_FAMILY. >>> >>> This is dependent on updates to Open-iSCSI. >> >> Hi Robert, >> >> I don't think that passing the iface_rec structure directly from the >> iscsid internals into a netlink message is a good way to go about this. >> It's really big, there's an embedded list_head with user address >> pointers that needs to be left out, and there are 32/64-bit layout >> differences. >> >> Let me take a look at how you're proposing using this info for iSER, if >> it makes sense I think we should come up with a better designed >> structure for passing the information. >> >> Thanks, >> Chris >> > > Chris, > > Thank you for your feedback. I agree that the entire iface is probably > overkill, it was more of a proof of concept. We are only using the > ipaddress in the iface for iSER (in my patch), but I could see other > drivers benefiting from some of the other data in the iface (mac, > interface_name, vlan, etc) so I didn't want to be too restrictive so > that it wouldn't have to be extended later. I've not worked on > userspace/kernel interaction before so I need some guidance to make > the transition between userspace and kernel versions smoother. > > This patchset works for what we need and it is very important for us > (and I'm sure others once the feature is available) and I'm happy to > put in the time to get it accepted upstream, I'm just new to kernel > development and need some guidance. Are there other comments/ideas/suggestions specifically from the iSCSI/iSER guys? I'd like to keep this patch moving. Thanks. Robert LeBlanc PGP Fingerprint 79A2 9CA4 6CC4 45DD A904 C70E E654 3BB2 FA62 B9F1
Re: [PATCH 0/7] Enable iSCSI offload drivers to use information from iface.
On Wed, Jun 7, 2017 at 10:28 AM, Chris Leech wrote: > On Tue, Jun 06, 2017 at 12:07:10PM -0600, Robert LeBlanc wrote: >> This patchset enables iSCSI offload drivers to have access to the iface >> information provided by iscsid. This allows users to have more control >> of how the driver connects to the iSCSI target. iSER is updated to use >> iface.ipaddress to set the source IP address if configured. This allows >> iSER to use multiple ports on the same network or in more complicated >> routed configurations. >> >> Since there is already a change to the function parameters, dst_addr >> is upgraded to sockaddr_storage so that it is more future proof and makes >> the size of the struct static and not dependent on checking the SA_FAMILY. >> >> This is dependent on updates to Open-iSCSI. > > Hi Robert, > > I don't think that passing the iface_rec structure directly from the > iscsid internals into a netlink message is a good way to go about this. > It's really big, there's an embedded list_head with user address > pointers that needs to be left out, and there are 32/64-bit layout > differences. > > Let me take a look at how you're proposing using this info for iSER, if > it makes sense I think we should come up with a better designed > structure for passing the information. > > Thanks, > Chris > Chris, Thank you for your feedback. I agree that the entire iface is probably overkill, it was more of a proof of concept. We are only using the ipaddress in the iface for iSER (in my patch), but I could see other drivers benefiting from some of the other data in the iface (mac, interface_name, vlan, etc) so I didn't want to be too restrictive so that it wouldn't have to be extended later. I've not worked on userspace/kernel interaction before so I need some guidance to make the transition between userspace and kernel versions smoother. This patchset works for what we need and it is very important for us (and I'm sure others once the feature is available) and I'm happy to put in the time to get it accepted upstream, I'm just new to kernel development and need some guidance. Thanks, Robert LeBlanc PGP Fingerprint 79A2 9CA4 6CC4 45DD A904 C70E E654 3BB2 FA62 B9F1
Re: [PATCH 0/7] Enable iSCSI offload drivers to use information from iface.
On Tue, Jun 06, 2017 at 12:07:10PM -0600, Robert LeBlanc wrote: > This patchset enables iSCSI offload drivers to have access to the iface > information provided by iscsid. This allows users to have more control > of how the driver connects to the iSCSI target. iSER is updated to use > iface.ipaddress to set the source IP address if configured. This allows > iSER to use multiple ports on the same network or in more complicated > routed configurations. > > Since there is already a change to the function parameters, dst_addr > is upgraded to sockaddr_storage so that it is more future proof and makes > the size of the struct static and not dependent on checking the SA_FAMILY. > > This is dependent on updates to Open-iSCSI. Hi Robert, I don't think that passing the iface_rec structure directly from the iscsid internals into a netlink message is a good way to go about this. It's really big, there's an embedded list_head with user address pointers that needs to be left out, and there are 32/64-bit layout differences. Let me take a look at how you're proposing using this info for iSER, if it makes sense I think we should come up with a better designed structure for passing the information. Thanks, Chris > Robert LeBlanc (7): > scsi/scsi_transport_iscsi: Add iface struct to kernel. > scsi/scsi_transport_iscsi: Update ep_connect to include iface. > ib/iSER: Add binding to source IP address. > scsi/be2iscsi: Update beiscsi_ep_connect to accept iface and > sockaddr_storage. > scsi/bnx2i: Update bnx2i_ep_connect to accept iface and > sockaddr_storage. > scsi/cxgbi: Update cxgbi_ep_connect to accept iface and > sockaddr_storage. > scsi/qla4xxx: Update qla4xxx_ep_connect to accept iface and > sockaddr_storage. > > drivers/infiniband/ulp/iser/iscsi_iser.c | 33 +++-- > drivers/infiniband/ulp/iser/iscsi_iser.h | 4 +- > drivers/infiniband/ulp/iser/iser_initiator.c | 1 + > drivers/infiniband/ulp/iser/iser_memory.c| 1 + > drivers/infiniband/ulp/iser/iser_verbs.c | 8 ++- > drivers/scsi/be2iscsi/be_cmds.c | 1 + > drivers/scsi/be2iscsi/be_iscsi.c | 8 ++- > drivers/scsi/be2iscsi/be_iscsi.h | 5 +- > drivers/scsi/be2iscsi/be_main.c | 1 + > drivers/scsi/be2iscsi/be_mgmt.c | 1 + > drivers/scsi/bnx2i/bnx2i_hwi.c | 1 + > drivers/scsi/bnx2i/bnx2i_iscsi.c | 13 ++-- > drivers/scsi/cxgbi/libcxgbi.c| 15 ++-- > drivers/scsi/cxgbi/libcxgbi.h| 2 +- > drivers/scsi/qla4xxx/ql4_os.c| 15 ++-- > drivers/scsi/scsi_transport_iscsi.c | 9 ++- > include/scsi/scsi_transport_iscsi.h | 100 > ++- > 17 files changed, 179 insertions(+), 39 deletions(-) > > -- > 2.11.0 >
Re: [PATCH 0/7] Enable iSCSI offload drivers to use information from iface.
On 06/06/2017 08:07 PM, Robert LeBlanc wrote: > This patchset enables iSCSI offload drivers to have access to the iface > information provided by iscsid. This allows users to have more control > of how the driver connects to the iSCSI target. iSER is updated to use > iface.ipaddress to set the source IP address if configured. This allows > iSER to use multiple ports on the same network or in more complicated > routed configurations. > > Since there is already a change to the function parameters, dst_addr > is upgraded to sockaddr_storage so that it is more future proof and makes > the size of the struct static and not dependent on checking the SA_FAMILY. > > This is dependent on updates to Open-iSCSI. > > Robert LeBlanc (7): > scsi/scsi_transport_iscsi: Add iface struct to kernel. > scsi/scsi_transport_iscsi: Update ep_connect to include iface. > ib/iSER: Add binding to source IP address. > scsi/be2iscsi: Update beiscsi_ep_connect to accept iface and > sockaddr_storage. > scsi/bnx2i: Update bnx2i_ep_connect to accept iface and > sockaddr_storage. > scsi/cxgbi: Update cxgbi_ep_connect to accept iface and > sockaddr_storage. > scsi/qla4xxx: Update qla4xxx_ep_connect to accept iface and > sockaddr_storage. > > drivers/infiniband/ulp/iser/iscsi_iser.c | 33 +++-- > drivers/infiniband/ulp/iser/iscsi_iser.h | 4 +- > drivers/infiniband/ulp/iser/iser_initiator.c | 1 + > drivers/infiniband/ulp/iser/iser_memory.c| 1 + > drivers/infiniband/ulp/iser/iser_verbs.c | 8 ++- > drivers/scsi/be2iscsi/be_cmds.c | 1 + > drivers/scsi/be2iscsi/be_iscsi.c | 8 ++- > drivers/scsi/be2iscsi/be_iscsi.h | 5 +- > drivers/scsi/be2iscsi/be_main.c | 1 + > drivers/scsi/be2iscsi/be_mgmt.c | 1 + > drivers/scsi/bnx2i/bnx2i_hwi.c | 1 + > drivers/scsi/bnx2i/bnx2i_iscsi.c | 13 ++-- > drivers/scsi/cxgbi/libcxgbi.c| 15 ++-- > drivers/scsi/cxgbi/libcxgbi.h| 2 +- > drivers/scsi/qla4xxx/ql4_os.c| 15 ++-- > drivers/scsi/scsi_transport_iscsi.c | 9 ++- > include/scsi/scsi_transport_iscsi.h | 100 > ++- > 17 files changed, 179 insertions(+), 39 deletions(-) > Hmm. That it rather large, just for passing information from userspace into the kernel. What is the actual benefit here? Personally, I would rather see iscsid creating tap interfaces associated with each iSCSI offload interface, and having the kernel accessing the information from _that_. Then each connection would have a network interface attached to it, and could retrieve the information from there. Plus we could use 'normal' userspace tools like 'ip' to configure the iSCSI offload network. Cheers, Hannes -- Dr. Hannes ReineckeTeamlead Storage & Networking h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg)
[PATCH 0/7] Enable iSCSI offload drivers to use information from iface.
This patchset enables iSCSI offload drivers to have access to the iface information provided by iscsid. This allows users to have more control of how the driver connects to the iSCSI target. iSER is updated to use iface.ipaddress to set the source IP address if configured. This allows iSER to use multiple ports on the same network or in more complicated routed configurations. Since there is already a change to the function parameters, dst_addr is upgraded to sockaddr_storage so that it is more future proof and makes the size of the struct static and not dependent on checking the SA_FAMILY. This is dependent on updates to Open-iSCSI. Robert LeBlanc (7): scsi/scsi_transport_iscsi: Add iface struct to kernel. scsi/scsi_transport_iscsi: Update ep_connect to include iface. ib/iSER: Add binding to source IP address. scsi/be2iscsi: Update beiscsi_ep_connect to accept iface and sockaddr_storage. scsi/bnx2i: Update bnx2i_ep_connect to accept iface and sockaddr_storage. scsi/cxgbi: Update cxgbi_ep_connect to accept iface and sockaddr_storage. scsi/qla4xxx: Update qla4xxx_ep_connect to accept iface and sockaddr_storage. drivers/infiniband/ulp/iser/iscsi_iser.c | 33 +++-- drivers/infiniband/ulp/iser/iscsi_iser.h | 4 +- drivers/infiniband/ulp/iser/iser_initiator.c | 1 + drivers/infiniband/ulp/iser/iser_memory.c| 1 + drivers/infiniband/ulp/iser/iser_verbs.c | 8 ++- drivers/scsi/be2iscsi/be_cmds.c | 1 + drivers/scsi/be2iscsi/be_iscsi.c | 8 ++- drivers/scsi/be2iscsi/be_iscsi.h | 5 +- drivers/scsi/be2iscsi/be_main.c | 1 + drivers/scsi/be2iscsi/be_mgmt.c | 1 + drivers/scsi/bnx2i/bnx2i_hwi.c | 1 + drivers/scsi/bnx2i/bnx2i_iscsi.c | 13 ++-- drivers/scsi/cxgbi/libcxgbi.c| 15 ++-- drivers/scsi/cxgbi/libcxgbi.h| 2 +- drivers/scsi/qla4xxx/ql4_os.c| 15 ++-- drivers/scsi/scsi_transport_iscsi.c | 9 ++- include/scsi/scsi_transport_iscsi.h | 100 ++- 17 files changed, 179 insertions(+), 39 deletions(-) -- 2.11.0
