Re: [PATCH net,stable] net: cdc_ncm: correct overhead in delayed_ndp_size

From: kernel test robot
Date: Sun Jan 03 2021 - 12:04:36 EST

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 <lkp@xxxxxxxxx>

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@xxxxxxxxxxxx

Attachment: .config.gz
Description: application/gzip