iscsi: Move the maximum data segment limits into 'struct icl_conn'.

[jhb]

Summary

This fixes a few bugs in iSCSI backends where the backends were using
the limits they advertised initially during the login phase as the
final values instead of the values negotiated with the other end.

Reported by: Jithesh Arakkan @ Chelsio

[jhb]

sys/dev/cxgbe/cxgbei/icl_cxgbei.c

809

It turns out that this calculation below was just wrong as it could not possibly honor what the remote side had advertised as the max recv length. In addition, it didn't match the value returned in 'idl' for limits since the limits version unconditionally chops off the digest lengths. The actual failures we saw in QA were that when using ISO (a TSO-like feature where the NIC chops up a burst of PDUs into individual PDUs), we were sending too large PDUs to a Linux peer which complained. I think FreeBSD's peer might have failed to complain about the violation.

sys/dev/iscsi/icl_soft.c

1157

This previously relied on the fact that the driver was just setting ic_max_data_segment_length to the value returned in the 'idl' limits structure. It seems cleaner to let the backends just use the actual negotiated value instead of having to be careful to set the same value in two places.

sys/dev/iscsi/iscsi.c

1803

Despite the comment, these values aren't actually used.

[mav]

It mostly looks fine for me, except one comment inline.

sys/dev/iscsi/iscsi.c

1803

iSER uses kernel proxying during negotiation phase before handoff call pass the negotiated values. As I see iscsi_ioctl_daemon_send() uses is_max_send_data_segment_length. So some minimal defaults could be useful.

[jhb]

sys/dev/iscsi/iscsi.c

1803

Hmm, then I might have broken this a bit by making it depend on the values in is_conn. Is is_conn not valid yet during the negotiation phase? Hmm, looks like we allocate is_conn just a few lines below, so presumably I should be initializing these default values when the icl_conn is created? I guess there isn't a common "icl_init", so I will instead just set these here after we've setup the icl_conn.

One other question is if it makes sense to move max_burst_length and first_burst_length into the icl_conn as well?

[mav]

sys/dev/iscsi/iscsi.c

1803

Considering we ask the offload drivers for these limits, it may have sense to let them know the negotiation results, at least for interface symmetry. Though I don't think any offload drivers now have real use for them. I think we may need more parameters to be passed there in future if we implement support for non-immediate unsolicited data, multiple R2Ts, etc. , but so far I have no plans for that, since those are pretty rare cases. Or may be if we extend the target offload KPI somehow.

[jhb]

• Keep initial limits in iscsi.c.

[jhb]

sys/dev/iscsi/iscsi.c

1803

It is true there isn't much use for them in the driver. Part of my thinking was to reduce the fields we have to duplicate separately in 'cs' and 'is' that could otherwise be shared in 'icl_conn'. However, if we do that, it shouldn't be part of this commit but a separate followup IMO.

[mav]

sys/dev/iscsi/iscsi.c

1803

I just consider it a public KPI to not change it too often without real need, even though used by only 3 drivers I know of.