On 6/8/21 3:06 PM, Laszlo Ersek wrote: > RFC 7143 explains that a single iSCSI session may use multiple TCP > connections. The first connection established is called the leading > connection. The login performed on the leading connection is called the > leading login. Before the session is considered full-featured, the leading > login must succeed. Further (non-leading) connections can be associated > with the session later. > > (It's unclear to me from RFC 7143 whether the non-leading connections > require individual (non-leading) logins as well, but that particular > question is irrelevant from the perspective of this patch; see below.) > > The data model in IScsiDxe exhibits some confusion, regarding connection / > session association: > > - On one hand, the "ISCSI_SESSION.Conns" field is a *set* (it has type > LIST_ENTRY), and accordingly, connections can be added to, and removed > from, a session, with the IScsiAttatchConnection() and > IScsiDetatchConnection() functions. > > - On the other hand, ISCSI_MAX_CONNS_PER_SESSION has value 1, therefore no > session will ever use more than 1 connection at a time (refer to > instances of "Session->MaxConnections" in > "NetworkPkg/IScsiDxe/IScsiProto.c"). > > This one-to-many confusion between ISCSI_SESSION and ISCSI_CONNECTION is > very visible in the CHAP logic, where the progress of the authentication > is maintained *per connection*, in the "ISCSI_CONNECTION.AuthStep" field > (with values such as ISCSI_AUTH_INITIAL, ISCSI_CHAP_STEP_ONE, etc), but > the *data* for the authentication are maintained *per session*, in the > "AuthType" and "AuthData" fields of ISCSI_SESSION. Clearly, this makes no > sense if multiple connections are eligible for logging in. > > Knowing that IScsiDxe uses only one connection per session (put > differently: knowing that any connection is a leading connection, and any > login is a leading login), there is no functionality bug. But the data > model is still broken: "AuthType", "AuthData", and "AuthStep" should be > maintained at the *same* level -- be it "session-level" or "(leading) > connection-level". > > Fixing this data model bug is more than what I'm signing up for. However, > I do need to add one function, in preparation for multi-hash support: > whenever a new login is attempted (put differently: whenever the leading > login is re-attempted), which always happens with a fresh connection, the > session-level authentication data needs to be rewound to a sane initial > state. > > Introduce the IScsiSessionResetAuthData() function. Call it from the > central -- session-level -- IScsiSessionLogin() function, just before the > latter calls the -- connection-level -- IScsiConnLogin() function. > > Right now, do nothing in IScsiSessionResetAuthData(); so functionally > speaking, the patch is a no-op. > > Cc: Jiaxin Wu <jiaxin...@intel.com> > Cc: Maciej Rabeda <maciej.rab...@linux.intel.com> > Cc: Philippe Mathieu-Daudé <phi...@redhat.com> > Cc: Siyuan Fu <siyuan...@intel.com> > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3355 > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > --- > NetworkPkg/IScsiDxe/IScsiProto.c | 18 ++++++++++++++++++ > 1 file changed, 18 insertions(+)
Reviewed-by: Philippe Mathieu-Daude <phi...@redhat.com> -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#76746): https://edk2.groups.io/g/devel/message/76746 Mute This Topic: https://groups.io/mt/83395022/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
