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 <l...@intel.com>

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-...@lists.01.org

Attachment: .config.gz
Description: application/gzip

Reply via email to