Hi Shyam, I have some observations/suggestions regarding your patch: On 11/06, Shyam Prasad N wrote:
Hi Steve, Please review this patch, and let me know what you think. Having this info in the workstation field of session setup helps server debugging in two ways. 1. It helps identify the client by node name. 2. It helps get the kernel release running on the client side. https://github.com/sprasad-microsoft/smb3-kernel-client/commit/d988e704dd9170c19ff94d9421c017e65dbbaac1.patch
- AFAICS it doesn't consider runtime hostname changes. Is it important to keep track of it? Would changing it mid-auth steps break it somehow? - I didn't understand the purpose of CIFS_DEFAULT_WORKSTATION_NAME. Why not simply use utsname()->nodename? Or even init_utsname()->nodename, which is supposed to be always valid. - Ditto for CIFS_MAX_WORKSTATION_LEN. utsname()->nodename has at most 65 bytes (__NEW_UTS_LEN + 1) anyway. Perhaps using MAXHOSTNAMELEN from <asm/param.h> would be a more generic approach. (btw this is because nodename is the unqualified hostname, sans-domain) - Instead of setting workstation_name to "nodename:release", why not implement the VERSION structure (MS-NLMP 2.2.2.10)? Then use LINUX_VERSION_* from <linux/version.h> or parse utsname()->release.
I ran some basic testing with the patch. Seems to serve the purpose. Please let me know if I'm missing something.
I hope I didn't miss anything. Cheers, Enzo
