| Message ID | 20230310153211.10982-6-sprasad@microsoft.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [01/11] cifs: fix tcon status change after tree connect | expand |
Hi Shyam,
I love your patch! Yet something to improve:
[auto build test ERROR on cifs/for-next]
[also build test ERROR on linus/master v6.3-rc1 next-20230310]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Shyam-Prasad-N/cifs-generate-signkey-for-the-channel-that-s-reconnecting/20230310-234711
base: git://git.samba.org/sfrench/cifs-2.6.git for-next
patch link: https://lore.kernel.org/r/20230310153211.10982-6-sprasad%40microsoft.com
patch subject: [PATCH 06/11] cifs: fix sockaddr comparison in iface_cmp
config: x86_64-randconfig-a001 (https://download.01.org/0day-ci/archive/20230311/202303111213.VsaCo9Ff-lkp@intel.com/config)
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# https://github.com/intel-lab-lkp/linux/commit/799e15f02b7da48acdde0b568eef1deb23aa32ed
git remote add linux-review https://github.com/intel-lab-lkp/linux
git fetch --no-tags linux-review Shyam-Prasad-N/cifs-generate-signkey-for-the-channel-that-s-reconnecting/20230310-234711
git checkout 799e15f02b7da48acdde0b568eef1deb23aa32ed
# save the config file
mkdir build_dir && cp config build_dir/.config
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 olddefconfig
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash fs/cifs/
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303111213.VsaCo9Ff-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
>> fs/cifs/connect.c:1311:4: error: expected expression
struct sockaddr_in *saddr4 = (struct sockaddr_in *)srcaddr;
^
>> fs/cifs/connect.c:1313:19: error: use of undeclared identifier 'saddr4'; did you mean 'vaddr4'?
return memcmp(&saddr4->sin_addr.s_addr,
^~~~~~
vaddr4
fs/cifs/connect.c:1312:24: note: 'vaddr4' declared here
struct sockaddr_in *vaddr4 = (struct sockaddr_in *)rhs;
^
>> fs/cifs/connect.c:1312:24: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
struct sockaddr_in *vaddr4 = (struct sockaddr_in *)rhs;
^
fs/cifs/connect.c:1328:4: error: expected expression
struct sockaddr_in6 *saddr6 = (struct sockaddr_in6 *)srcaddr;
^
>> fs/cifs/connect.c:1330:19: error: use of undeclared identifier 'saddr6'; did you mean 'vaddr6'?
return memcmp(&saddr6->sin6_addr,
^~~~~~
vaddr6
fs/cifs/connect.c:1329:25: note: 'vaddr6' declared here
struct sockaddr_in6 *vaddr6 = (struct sockaddr_in6 *)rhs;
^
fs/cifs/connect.c:1329:25: warning: mixing declarations and code is incompatible with standards before C99 [-Wdeclaration-after-statement]
struct sockaddr_in6 *vaddr6 = (struct sockaddr_in6 *)rhs;
^
2 warnings and 4 errors generated.
vim +1311 fs/cifs/connect.c
1291
1292 int
1293 cifs_ipaddr_cmp(struct sockaddr *srcaddr, struct sockaddr *rhs)
1294 {
1295 switch (srcaddr->sa_family) {
1296 case AF_UNSPEC:
1297 switch (rhs->sa_family) {
1298 case AF_UNSPEC:
1299 return 0;
1300 case AF_INET:
1301 case AF_INET6:
1302 return 1;
1303 default:
1304 return -1;
1305 }
1306 case AF_INET: {
1307 switch (rhs->sa_family) {
1308 case AF_UNSPEC:
1309 return -1;
1310 case AF_INET:
> 1311 struct sockaddr_in *saddr4 = (struct sockaddr_in *)srcaddr;
> 1312 struct sockaddr_in *vaddr4 = (struct sockaddr_in *)rhs;
> 1313 return memcmp(&saddr4->sin_addr.s_addr,
1314 &vaddr4->sin_addr.s_addr,
1315 sizeof(struct sockaddr_in));
1316 case AF_INET6:
1317 return 1;
1318 default:
1319 return -1;
1320 }
1321 }
1322 case AF_INET6: {
1323 switch (rhs->sa_family) {
1324 case AF_UNSPEC:
1325 case AF_INET:
1326 return -1;
1327 case AF_INET6:
1328 struct sockaddr_in6 *saddr6 = (struct sockaddr_in6 *)srcaddr;
1329 struct sockaddr_in6 *vaddr6 = (struct sockaddr_in6 *)rhs;
> 1330 return memcmp(&saddr6->sin6_addr,
1331 &vaddr6->sin6_addr,
1332 sizeof(struct sockaddr_in6));
1333 default:
1334 return -1;
1335 }
1336 }
1337 default:
1338 return -1; /* don't expect to be here */
1339 }
1340 }
1341
diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h index 81ff13e41f97..a11e7b10f607 100644 --- a/fs/cifs/cifsglob.h +++ b/fs/cifs/cifsglob.h @@ -965,43 +965,6 @@ release_iface(struct kref *ref) kfree(iface); } -/* - * compare two interfaces a and b - * return 0 if everything matches. - * return 1 if a has higher link speed, or rdma capable, or rss capable - * return -1 otherwise. - */ -static inline int -iface_cmp(struct cifs_server_iface *a, struct cifs_server_iface *b) -{ - int cmp_ret = 0; - - WARN_ON(!a || !b); - if (a->speed == b->speed) { - if (a->rdma_capable == b->rdma_capable) { - if (a->rss_capable == b->rss_capable) { - cmp_ret = memcmp(&a->sockaddr, &b->sockaddr, - sizeof(a->sockaddr)); - if (!cmp_ret) - return 0; - else if (cmp_ret > 0) - return 1; - else - return -1; - } else if (a->rss_capable > b->rss_capable) - return 1; - else - return -1; - } else if (a->rdma_capable > b->rdma_capable) - return 1; - else - return -1; - } else if (a->speed > b->speed) - return 1; - else - return -1; -} - struct cifs_chan { struct TCP_Server_Info *server; struct cifs_server_iface *iface; /* interface in use */ diff --git a/fs/cifs/cifsproto.h b/fs/cifs/cifsproto.h index e2eff66eefab..30fd81268eb7 100644 --- a/fs/cifs/cifsproto.h +++ b/fs/cifs/cifsproto.h @@ -86,6 +86,7 @@ extern int cifs_handle_standard(struct TCP_Server_Info *server, struct mid_q_entry *mid); extern int smb3_parse_devname(const char *devname, struct smb3_fs_context *ctx); extern int smb3_parse_opt(const char *options, const char *key, char **val); +extern int cifs_ipaddr_cmp(struct sockaddr *srcaddr, struct sockaddr *rhs); extern bool cifs_match_ipaddr(struct sockaddr *srcaddr, struct sockaddr *rhs); extern int cifs_discard_remaining_data(struct TCP_Server_Info *server); extern int cifs_call_async(struct TCP_Server_Info *server, diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c index fb9d9994df09..b9af60417194 100644 --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -1289,6 +1289,56 @@ cifs_demultiplex_thread(void *p) module_put_and_kthread_exit(0); } +int +cifs_ipaddr_cmp(struct sockaddr *srcaddr, struct sockaddr *rhs) +{ + switch (srcaddr->sa_family) { + case AF_UNSPEC: + switch (rhs->sa_family) { + case AF_UNSPEC: + return 0; + case AF_INET: + case AF_INET6: + return 1; + default: + return -1; + } + case AF_INET: { + switch (rhs->sa_family) { + case AF_UNSPEC: + return -1; + case AF_INET: + struct sockaddr_in *saddr4 = (struct sockaddr_in *)srcaddr; + struct sockaddr_in *vaddr4 = (struct sockaddr_in *)rhs; + return memcmp(&saddr4->sin_addr.s_addr, + &vaddr4->sin_addr.s_addr, + sizeof(struct sockaddr_in)); + case AF_INET6: + return 1; + default: + return -1; + } + } + case AF_INET6: { + switch (rhs->sa_family) { + case AF_UNSPEC: + case AF_INET: + return -1; + case AF_INET6: + struct sockaddr_in6 *saddr6 = (struct sockaddr_in6 *)srcaddr; + struct sockaddr_in6 *vaddr6 = (struct sockaddr_in6 *)rhs; + return memcmp(&saddr6->sin6_addr, + &vaddr6->sin6_addr, + sizeof(struct sockaddr_in6)); + default: + return -1; + } + } + default: + return -1; /* don't expect to be here */ + } +} + /* * Returns true if srcaddr isn't specified and rhs isn't specified, or * if srcaddr is specified and matches the IP address of the rhs argument diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c index 6dfb865ee9d7..0627d5e38236 100644 --- a/fs/cifs/smb2ops.c +++ b/fs/cifs/smb2ops.c @@ -510,6 +510,43 @@ smb3_negotiate_rsize(struct cifs_tcon *tcon, struct smb3_fs_context *ctx) return rsize; } +/* + * compare two interfaces a and b + * return 0 if everything matches. + * return 1 if a is rdma capable, or rss capable, or has higher link speed + * return -1 otherwise. + */ +static int +iface_cmp(struct cifs_server_iface *a, struct cifs_server_iface *b) +{ + int cmp_ret = 0; + + WARN_ON(!a || !b); + if (a->rdma_capable == b->rdma_capable) { + if (a->rss_capable == b->rss_capable) { + if (a->speed == b->speed) { + cmp_ret = cifs_ipaddr_cmp((struct sockaddr *) &a->sockaddr, + (struct sockaddr *) &b->sockaddr); + if (!cmp_ret) + return 0; + else if (cmp_ret > 0) + return 1; + else + return -1; + } else if (a->speed > b->speed) + return 1; + else + return -1; + } else if (a->rss_capable > b->rss_capable) + return 1; + else + return -1; + } else if (a->rdma_capable > b->rdma_capable) + return 1; + else + return -1; +} + static int parse_server_interfaces(struct network_interface_info_ioctl_rsp *buf, size_t buf_len, struct cifs_ses *ses, bool in_mount)
iface_cmp used to simply do a memcmp of the two provided struct sockaddrs. The comparison needs to do more based on the address family. Similar logic was already present in cifs_match_ipaddr. Doing something similar now. Signed-off-by: Shyam Prasad N <sprasad@microsoft.com> --- fs/cifs/cifsglob.h | 37 --------------------------------- fs/cifs/cifsproto.h | 1 + fs/cifs/connect.c | 50 +++++++++++++++++++++++++++++++++++++++++++++ fs/cifs/smb2ops.c | 37 +++++++++++++++++++++++++++++++++ 4 files changed, 88 insertions(+), 37 deletions(-)