Hi "Jouni, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on v5.11-rc1 next-20201223] [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] url: https://github.com/0day-ci/linux/commits/Jouni-Sepp-nen/net-cdc_ncm-correct-overhead-in-delayed_ndp_size/20210103-224538 base: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 3516bd729358a2a9b090c1905bd2a3fa926e24c6 config: x86_64-randconfig-a003-20210103 (attached as .config) compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 7af6a134508cd1c7f75c6e3441ce436f220f30a4) 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 # install x86_64 cross compiling tool for clang build # apt-get install binutils-x86-64-linux-gnu # https://github.com/0day-ci/linux/commit/3d8cc665ef1cf4705135a5a96893a6fdc6dcd398 git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Jouni-Sepp-nen/net-cdc_ncm-correct-overhead-in-delayed_ndp_size/20210103-224538 git checkout 3d8cc665ef1cf4705135a5a96893a6fdc6dcd398 # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot All warnings (new ones prefixed by >>): >> drivers/net/usb/cdc_ncm.c:1203:4: warning: comparison of distinct pointer types ('typeof (ctx->tx_ndp_modulus) *' (aka 'unsigned short *') and 'typeof (ctx->tx_modulus + ctx->tx_remainder) *' (aka 'int *')) [-Wcompare-distinct-pointer-types] max(ctx->tx_ndp_modulus, ^~~~~~~~~~~~~~~~~~~~~~~~ include/linux/minmax.h:58:19: note: expanded from macro 'max' #define max(x, y) __careful_cmp(x, y, >) ^~~~~~~~~~~~~~~~~~~~~~ include/linux/minmax.h:42:24: note: expanded from macro '__careful_cmp' __builtin_choose_expr(__safe_cmp(x, y), \ ^~~~~~~~~~~~~~~~ include/linux/minmax.h:32:4: note: expanded from macro '__safe_cmp' (__typecheck(x, y) && __no_side_effects(x, y)) ^~~~~~~~~~~~~~~~~ include/linux/minmax.h:18:28: note: expanded from macro '__typecheck' (!!(sizeof((typeof(x) *)1 == (typeof(y) *)1))) ~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~ 1 warning generated. vim +1203 drivers/net/usb/cdc_ncm.c 1179 1180 struct sk_buff * 1181 cdc_ncm_fill_tx_frame(struct usbnet *dev, struct sk_buff *skb, __le32 sign) 1182 { 1183 struct cdc_ncm_ctx *ctx = (struct cdc_ncm_ctx *)dev->data[0]; 1184 union { 1185 struct usb_cdc_ncm_nth16 *nth16; 1186 struct usb_cdc_ncm_nth32 *nth32; 1187 } nth; 1188 union { 1189 struct usb_cdc_ncm_ndp16 *ndp16; 1190 struct usb_cdc_ncm_ndp32 *ndp32; 1191 } ndp; 1192 struct sk_buff *skb_out; 1193 u16 n = 0, index, ndplen; 1194 u8 ready2send = 0; 1195 u32 delayed_ndp_size; 1196 size_t padding_count; 1197 1198 /* When our NDP gets written in cdc_ncm_ndp(), then skb_out->len gets updated 1199 * accordingly. Otherwise, we should check here. 1200 */ 1201 if (ctx->drvflags & CDC_NCM_FLAG_NDP_TO_END) 1202 delayed_ndp_size = ctx->max_ndp_size + > 1203 max(ctx->tx_ndp_modulus, 1204 ctx->tx_modulus + ctx->tx_remainder) - 1; 1205 else 1206 delayed_ndp_size = 0; 1207 1208 /* if there is a remaining skb, it gets priority */ 1209 if (skb != NULL) { 1210 swap(skb, ctx->tx_rem_skb); 1211 swap(sign, ctx->tx_rem_sign); 1212 } else { 1213 ready2send = 1; 1214 } 1215 1216 /* check if we are resuming an OUT skb */ 1217 skb_out = ctx->tx_curr_skb; 1218 1219 /* allocate a new OUT skb */ 1220 if (!skb_out) { 1221 if (ctx->tx_low_mem_val == 0) { 1222 ctx->tx_curr_size = ctx->tx_max; 1223 skb_out = alloc_skb(ctx->tx_curr_size, GFP_ATOMIC); 1224 /* If the memory allocation fails we will wait longer 1225 * each time before attempting another full size 1226 * allocation again to not overload the system 1227 * further. 1228 */ 1229 if (skb_out == NULL) { 1230 ctx->tx_low_mem_max_cnt = min(ctx->tx_low_mem_max_cnt + 1, 1231 (unsigned)CDC_NCM_LOW_MEM_MAX_CNT); 1232 ctx->tx_low_mem_val = ctx->tx_low_mem_max_cnt; 1233 } 1234 } 1235 if (skb_out == NULL) { 1236 /* See if a very small allocation is possible. 1237 * We will send this packet immediately and hope 1238 * that there is more memory available later. 1239 */ 1240 if (skb) 1241 ctx->tx_curr_size = max(skb->len, 1242 (u32)USB_CDC_NCM_NTB_MIN_OUT_SIZE); 1243 else 1244 ctx->tx_curr_size = USB_CDC_NCM_NTB_MIN_OUT_SIZE; 1245 skb_out = alloc_skb(ctx->tx_curr_size, GFP_ATOMIC); 1246 1247 /* No allocation possible so we will abort */ 1248 if (skb_out == NULL) { 1249 if (skb != NULL) { 1250 dev_kfree_skb_any(skb); 1251 dev->net->stats.tx_dropped++; 1252 } 1253 goto exit_no_skb; 1254 } 1255 ctx->tx_low_mem_val--; 1256 } 1257 if (ctx->is_ndp16) { 1258 /* fill out the initial 16-bit NTB header */ 1259 nth.nth16 = skb_put_zero(skb_out, sizeof(struct usb_cdc_ncm_nth16)); 1260 nth.nth16->dwSignature = cpu_to_le32(USB_CDC_NCM_NTH16_SIGN); 1261 nth.nth16->wHeaderLength = cpu_to_le16(sizeof(struct usb_cdc_ncm_nth16)); 1262 nth.nth16->wSequence = cpu_to_le16(ctx->tx_seq++); 1263 } else { 1264 /* fill out the initial 32-bit NTB header */ 1265 nth.nth32 = skb_put_zero(skb_out, sizeof(struct usb_cdc_ncm_nth32)); 1266 nth.nth32->dwSignature = cpu_to_le32(USB_CDC_NCM_NTH32_SIGN); 1267 nth.nth32->wHeaderLength = cpu_to_le16(sizeof(struct usb_cdc_ncm_nth32)); 1268 nth.nth32->wSequence = cpu_to_le16(ctx->tx_seq++); 1269 } 1270 1271 /* count total number of frames in this NTB */ 1272 ctx->tx_curr_frame_num = 0; 1273 1274 /* recent payload counter for this skb_out */ 1275 ctx->tx_curr_frame_payload = 0; 1276 } 1277 1278 for (n = ctx->tx_curr_frame_num; n < ctx->tx_max_datagrams; n++) { 1279 /* send any remaining skb first */ 1280 if (skb == NULL) { 1281 skb = ctx->tx_rem_skb; 1282 sign = ctx->tx_rem_sign; 1283 ctx->tx_rem_skb = NULL; 1284 1285 /* check for end of skb */ 1286 if (skb == NULL) 1287 break; 1288 } 1289 1290 /* get the appropriate NDP for this skb */ 1291 if (ctx->is_ndp16) 1292 ndp.ndp16 = cdc_ncm_ndp16(ctx, skb_out, sign, skb->len + ctx->tx_modulus + ctx->tx_remainder); 1293 else 1294 ndp.ndp32 = cdc_ncm_ndp32(ctx, skb_out, sign, skb->len + ctx->tx_modulus + ctx->tx_remainder); 1295 1296 /* align beginning of next frame */ 1297 cdc_ncm_align_tail(skb_out, ctx->tx_modulus, ctx->tx_remainder, ctx->tx_curr_size); 1298 1299 /* check if we had enough room left for both NDP and frame */ 1300 if ((ctx->is_ndp16 && !ndp.ndp16) || (!ctx->is_ndp16 && !ndp.ndp32) || 1301 skb_out->len + skb->len + delayed_ndp_size > ctx->tx_curr_size) { 1302 if (n == 0) { 1303 /* won't fit, MTU problem? */ 1304 dev_kfree_skb_any(skb); 1305 skb = NULL; 1306 dev->net->stats.tx_dropped++; 1307 } else { 1308 /* no room for skb - store for later */ 1309 if (ctx->tx_rem_skb != NULL) { 1310 dev_kfree_skb_any(ctx->tx_rem_skb); 1311 dev->net->stats.tx_dropped++; 1312 } 1313 ctx->tx_rem_skb = skb; 1314 ctx->tx_rem_sign = sign; 1315 skb = NULL; 1316 ready2send = 1; 1317 ctx->tx_reason_ntb_full++; /* count reason for transmitting */ 1318 } 1319 break; 1320 } 1321 1322 /* calculate frame number within this NDP */ 1323 if (ctx->is_ndp16) { 1324 ndplen = le16_to_cpu(ndp.ndp16->wLength); 1325 index = (ndplen - sizeof(struct usb_cdc_ncm_ndp16)) / sizeof(struct usb_cdc_ncm_dpe16) - 1; 1326 1327 /* OK, add this skb */ 1328 ndp.ndp16->dpe16[index].wDatagramLength = cpu_to_le16(skb->len); 1329 ndp.ndp16->dpe16[index].wDatagramIndex = cpu_to_le16(skb_out->len); 1330 ndp.ndp16->wLength = cpu_to_le16(ndplen + sizeof(struct usb_cdc_ncm_dpe16)); 1331 } else { 1332 ndplen = le16_to_cpu(ndp.ndp32->wLength); 1333 index = (ndplen - sizeof(struct usb_cdc_ncm_ndp32)) / sizeof(struct usb_cdc_ncm_dpe32) - 1; 1334 1335 ndp.ndp32->dpe32[index].dwDatagramLength = cpu_to_le32(skb->len); 1336 ndp.ndp32->dpe32[index].dwDatagramIndex = cpu_to_le32(skb_out->len); 1337 ndp.ndp32->wLength = cpu_to_le16(ndplen + sizeof(struct usb_cdc_ncm_dpe32)); 1338 } 1339 skb_put_data(skb_out, skb->data, skb->len); 1340 ctx->tx_curr_frame_payload += skb->len; /* count real tx payload data */ 1341 dev_kfree_skb_any(skb); 1342 skb = NULL; 1343 1344 /* send now if this NDP is full */ 1345 if (index >= CDC_NCM_DPT_DATAGRAMS_MAX) { 1346 ready2send = 1; 1347 ctx->tx_reason_ndp_full++; /* count reason for transmitting */ 1348 break; 1349 } 1350 } 1351 1352 /* free up any dangling skb */ 1353 if (skb != NULL) { 1354 dev_kfree_skb_any(skb); 1355 skb = NULL; 1356 dev->net->stats.tx_dropped++; 1357 } 1358 1359 ctx->tx_curr_frame_num = n; 1360 1361 if (n == 0) { 1362 /* wait for more frames */ 1363 /* push variables */ 1364 ctx->tx_curr_skb = skb_out; 1365 goto exit_no_skb; 1366 1367 } else if ((n < ctx->tx_max_datagrams) && (ready2send == 0) && (ctx->timer_interval > 0)) { 1368 /* wait for more frames */ 1369 /* push variables */ 1370 ctx->tx_curr_skb = skb_out; 1371 /* set the pending count */ 1372 if (n < CDC_NCM_RESTART_TIMER_DATAGRAM_CNT) 1373 ctx->tx_timer_pending = CDC_NCM_TIMER_PENDING_CNT; 1374 goto exit_no_skb; 1375 1376 } else { 1377 if (n == ctx->tx_max_datagrams) 1378 ctx->tx_reason_max_datagram++; /* count reason for transmitting */ 1379 /* frame goes out */ 1380 /* variables will be reset at next call */ 1381 } 1382 1383 /* If requested, put NDP at end of frame. */ 1384 if (ctx->drvflags & CDC_NCM_FLAG_NDP_TO_END) { 1385 if (ctx->is_ndp16) { 1386 nth.nth16 = (struct usb_cdc_ncm_nth16 *)skb_out->data; 1387 cdc_ncm_align_tail(skb_out, ctx->tx_ndp_modulus, 0, ctx->tx_curr_size - ctx->max_ndp_size); 1388 nth.nth16->wNdpIndex = cpu_to_le16(skb_out->len); 1389 skb_put_data(skb_out, ctx->delayed_ndp16, ctx->max_ndp_size); 1390 1391 /* Zero out delayed NDP - signature checking will naturally fail. */ 1392 ndp.ndp16 = memset(ctx->delayed_ndp16, 0, ctx->max_ndp_size); 1393 } else { 1394 nth.nth32 = (struct usb_cdc_ncm_nth32 *)skb_out->data; 1395 cdc_ncm_align_tail(skb_out, ctx->tx_ndp_modulus, 0, ctx->tx_curr_size - ctx->max_ndp_size); 1396 nth.nth32->dwNdpIndex = cpu_to_le32(skb_out->len); 1397 skb_put_data(skb_out, ctx->delayed_ndp32, ctx->max_ndp_size); 1398 1399 ndp.ndp32 = memset(ctx->delayed_ndp32, 0, ctx->max_ndp_size); 1400 } 1401 } 1402 1403 /* If collected data size is less or equal ctx->min_tx_pkt 1404 * bytes, we send buffers as it is. If we get more data, it 1405 * would be more efficient for USB HS mobile device with DMA 1406 * engine to receive a full size NTB, than canceling DMA 1407 * transfer and receiving a short packet. 1408 * 1409 * This optimization support is pointless if we end up sending 1410 * a ZLP after full sized NTBs. 1411 */ 1412 if (!(dev->driver_info->flags & FLAG_SEND_ZLP) && 1413 skb_out->len > ctx->min_tx_pkt) { 1414 padding_count = ctx->tx_curr_size - skb_out->len; 1415 if (!WARN_ON(padding_count > ctx->tx_curr_size)) 1416 skb_put_zero(skb_out, padding_count); 1417 } else if (skb_out->len < ctx->tx_curr_size && 1418 (skb_out->len % dev->maxpacket) == 0) { 1419 skb_put_u8(skb_out, 0); /* force short packet */ 1420 } 1421 1422 /* set final frame length */ 1423 if (ctx->is_ndp16) { 1424 nth.nth16 = (struct usb_cdc_ncm_nth16 *)skb_out->data; 1425 nth.nth16->wBlockLength = cpu_to_le16(skb_out->len); 1426 } else { 1427 nth.nth32 = (struct usb_cdc_ncm_nth32 *)skb_out->data; 1428 nth.nth32->dwBlockLength = cpu_to_le32(skb_out->len); 1429 } 1430 1431 /* return skb */ 1432 ctx->tx_curr_skb = NULL; 1433 1434 /* keep private stats: framing overhead and number of NTBs */ 1435 ctx->tx_overhead += skb_out->len - ctx->tx_curr_frame_payload; 1436 ctx->tx_ntbs++; 1437 1438 /* usbnet will count all the framing overhead by default. 1439 * Adjust the stats so that the tx_bytes counter show real 1440 * payload data instead. 1441 */ 1442 usbnet_set_skb_tx_stats(skb_out, n, 1443 (long)ctx->tx_curr_frame_payload - skb_out->len); 1444 1445 return skb_out; 1446 1447 exit_no_skb: 1448 /* Start timer, if there is a remaining non-empty skb */ 1449 if (ctx->tx_curr_skb != NULL && n > 0) 1450 cdc_ncm_tx_timeout_start(ctx); 1451 return NULL; 1452 } 1453 EXPORT_SYMBOL_GPL(cdc_ncm_fill_tx_frame); 1454 --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org