On 2026-01-08 at 21:37:44, Kalesh Anakkur Purayil (kalesh-anakkur.purayil@broadcom.com) wrote:

On Wed, Jan 7, 2026 at 7:23 PM Ratheesh Kannoth [off-list ref] wrote:

quoted

+               return 0;
+       }
+       mutex_unlock(&sw_fl_stats_lock);
+
+       snode = kcalloc(1, sizeof(*snode), GFP_KERNEL);

Why not kzalloc() instead of kcalloc with size 1?

All fields are assigned with values. why to initialize to zero ?

quoted

+       if (!snode)
+
+       return 0;
+}
+
+int rvu_sw_fl_stats_sync2db(struct rvu *rvu, struct fl_info *fl, int cnt)
+{
+       struct npc_mcam_get_mul_stats_req *req = NULL;
+       struct npc_mcam_get_mul_stats_rsp *rsp = NULL;

there is no need to initialize these two variables

This is initialized so that (after fail lablel) free() does not act
on garbage value.

quoted

+       int tot = 0;
+       u16 i2idx_map[256];

follow RCT order

ACK.

quoted

+       int rc = 0;
+       u64 pkts;
+       int idx;
+
+       cnt = min(cnt, 64);
+
+       for (int i = 0; i < cnt; i++) {

I think you can move the declaration of i at the beginning of the
function. it is repeated in the for loops below as well

I think, better to keep the scope local. Does kernel coding guidlines
mandate it ?

quoted

+               tot++;
+               if (fl[i].uni_di)
+                       continue;
+
+               tot++;
+       }
+
+       req = kcalloc(1, sizeof(*req), GFP_KERNEL);

I think you can use kzalloc kere

ACK.

quoted

+       if (!req) {
+               rc = -ENOMEM;

You can return directly here

ACK.

quoted

+               goto fail;
+       }
+
+       rsp = kcalloc(1, sizeof(*rsp), GFP_KERNEL);
+       if (!rsp) {
+               rc = -ENOMEM;
+               goto fail;

better do individual cleanup by adding a label and use goto free_req

free: lablel is enough , right ?

quoted

+       }
+
+       req->cnt = tot;
+       idx = 0;
+       for (int i = 0; i < tot; idx++) {
+               i2idx_map[i] = idx;
+               req->entry[i++] = fl[idx].mcam_idx[0];
+               if (fl[idx].uni_di)
+                       continue;
+
+               i2idx_map[i] = idx;
+               req->entry[i++] = fl[idx].mcam_idx[1];
+       }
+
+       if (rvu_mbox_handler_npc_mcam_mul_stats(rvu, req, rsp)) {
+               dev_err(rvu->dev, "Error to get multiple stats\n");
+               rc = -EFAULT;

You can add a new label and use goto free_resp

same comment as above.

quoted

+               goto fail;
+       }
+
+
+fail:
+       kfree(req);
+       kfree(rsp);
+       return rc;
+}
+
+       fl_entry->features = req->features;
+
+       mutex_lock(&fl_offl_llock);
+       list_add_tail(&fl_entry->list, &fl_offl_lh);
+       mutex_unlock(&fl_offl_llock);
+
+       if (!fl_offl_work_running) {
+               sw_fl_offl_wq = alloc_workqueue("sw_af_fl_wq", 0, 0);
+               if (!sw_fl_offl_wq)

free fl_entry here? also, do you want to move list_add_tail() after
this if() condition?

ACK.

quoted

+                       return -ENOMEM;
+
                  void *type_data)
 {
+       struct otx2_nic *nic = netdev_priv(netdev);
+
        switch (type) {
        case TC_SETUP_BLOCK:
+               if (netif_is_ovs_port(netdev)) {
+                       return flow_block_cb_setup_simple(type_data,
+                                                         &otx2_block_cb_list,
+                                                         sw_fl_setup_ft_block_ingress_cb,
+                                                         nic, nic, true);
+               }

braces are not required here

ACK.

quoted

+
                return otx2_setup_tc_block(netdev, type_data);
+}
+
+static int sw_fl_parse_actions(struct otx2_nic *nic,
+                              struct flow_action *flow_action,
+                              struct flow_cls_offload *f,
+                              struct fl_tuple *tuple, u64 *op)
+{
+       struct flow_action_entry *act;
+       struct otx2_nic *out_nic;
+       int err;
+       int used = 0;

RCT order here

ACK.

quoted

+       int i;
+                       return rc;
+       }
+
+       sw_fl_add_to_list(nic, &tuple, f->cookie, true);
+       return 0;
+}
+
+static int sw_fl_del(struct otx2_nic *nic, struct flow_cls_offload *f)

function prototype can be changed to void?

ACK.

quoted

+{