Re: [net-next Patch v6 5/6] octeontx2-pf: Add support for HTB offload
From: Simon Horman
Date: Sat Apr 08 2023 - 12:51:22 EST
On Thu, Apr 06, 2023 at 03:51:02PM +0530, Hariprasad Kelam wrote:
> From: Naveen Mamindlapalli <naveenm@xxxxxxxxxxx>
>
> This patch registers callbacks to support HTB offload.
>
> Below are features supported,
>
> - supports traffic shaping on the given class by honoring rate and ceil
> configuration.
>
> - supports traffic scheduling, which prioritizes different types of
> traffic based on strict priority values.
>
> - supports the creation of leaf to inner classes such that parent node
> rate limits apply to all child nodes.
...
> +static int otx2_qos_leaf_alloc_queue(struct otx2_nic *pfvf, u16 classid,
> + u32 parent_classid, u64 rate, u64 ceil,
> + u64 prio, struct netlink_ext_ack *extack)
> +{
> + struct otx2_qos_cfg *old_cfg, *new_cfg;
> + struct otx2_qos_node *node, *parent;
> + int qid, ret, err;
> +
> + netdev_dbg(pfvf->netdev,
> + "TC_HTB_LEAF_ALLOC_QUEUE: classid=0x%x parent_classid=0x%x rate=%lld ceil=%lld prio=%lld\n",
> + classid, parent_classid, rate, ceil, prio);
> +
> + if (prio > OTX2_QOS_MAX_PRIO) {
> + NL_SET_ERR_MSG_MOD(extack, "Valid priority range 0 to 7");
> + ret = -EOPNOTSUPP;
> + goto out;
> + }
out dereferences parent, but it is not set until a few lines below.
reported by gcc-12 with W=1 EXTRA_CFLAGS=-Wmaybe-uninitialized as:
drivers/net/ethernet/marvell/octeontx2/nic/qos.c: In function 'otx2_qos_leaf_alloc_queue':
drivers/net/ethernet/marvell/octeontx2/nic/qos.c:1178:31: error: 'parent' may be used uninitialized [-Werror=maybe-uninitialized]
1178 | clear_bit(prio, parent->prio_bmap);
| ~~~~~~^~~~~~~~~~~
drivers/net/ethernet/marvell/octeontx2/nic/qos.c:1076:38: note: 'parent' was declared here
1076 | struct otx2_qos_node *node, *parent;
|
And by clang-16:
drivers/net/ethernet/marvell/octeontx2/nic/qos.c:1083:6: error: variable 'parent' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
if (prio > OTX2_QOS_MAX_PRIO) {
^~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/marvell/octeontx2/nic/qos.c:1178:18: note: uninitialized use occurs here
clear_bit(prio, parent->prio_bmap);
^~~~~~
drivers/net/ethernet/marvell/octeontx2/nic/qos.c:1083:2: note: remove the 'if' if its condition is always false
if (prio > OTX2_QOS_MAX_PRIO) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/marvell/octeontx2/nic/qos.c:1076:37: note: initialize the variable 'parent' to silence this warning
struct otx2_qos_node *node, *parent;
^
= NULL
> +
> + /* get parent node */
> + parent = otx2_sw_node_find(pfvf, parent_classid);
> + if (!parent) {
> + NL_SET_ERR_MSG_MOD(extack, "parent node not found");
> + ret = -ENOENT;
> + goto out;
> + }
...
> + return pfvf->hw.tx_queues + qid;
> +
> +free_node:
> + otx2_qos_sw_node_delete(pfvf, node);
> +free_old_cfg:
> + kfree(old_cfg);
> +out:
> + clear_bit(prio, parent->prio_bmap);
> + return ret;
> +}
> +
> +static int otx2_qos_leaf_to_inner(struct otx2_nic *pfvf, u16 classid,
> + u16 child_classid, u64 rate, u64 ceil, u64 prio,
> + struct netlink_ext_ack *extack)
> +{
> + struct otx2_qos_cfg *old_cfg, *new_cfg;
> + struct otx2_qos_node *node, *child;
> + int ret, err;
> + u16 qid;
> +
> + netdev_dbg(pfvf->netdev,
> + "TC_HTB_LEAF_TO_INNER classid %04x, child %04x, rate %llu, ceil %llu\n",
> + classid, child_classid, rate, ceil);
> +
> + if (prio > OTX2_QOS_MAX_PRIO) {
> + NL_SET_ERR_MSG_MOD(extack, "Valid priority range 0 to 7");
> + ret = -EOPNOTSUPP;
> + goto out;
Likewise, out dereferences node, but it is not set until a few lines below.
reported by gcc-12 with W=1 EXTRA_CFLAGS=-Wmaybe-uninitialized as:
drivers/net/ethernet/marvell/octeontx2/nic/qos.c: In function 'otx2_qos_leaf_to_inner':
drivers/net/ethernet/marvell/octeontx2/nic/qos.c:1288:29: error: 'node' may be used uninitialized [-Werror=maybe-uninitialized]
1288 | clear_bit(prio, node->prio_bmap);
| ~~~~^~~~~~~~~~~
drivers/net/ethernet/marvell/octeontx2/nic/qos.c:1187:31: note: 'node' was declared here
1187 | struct otx2_qos_node *node, *child;
| ^~~~
drivers/net/ethernet/marvell/octeontx2/nic/qos.c: In function 'otx2_qos_leaf_alloc_queue':
drivers/net/ethernet/marvell/octeontx2/nic/qos.c:1178:31: error: 'parent' may be used uninitialized [-Werror=maybe-uninitialized]
1178 | clear_bit(prio, parent->prio_bmap);
| ~~~~~~^~~~~~~~~~~
drivers/net/ethernet/marvell/octeontx2/nic/qos.c:1076:38: note: 'parent' was declared here
1076 | struct otx2_qos_node *node, *parent;
|
And by clang-16 as:
drivers/net/ethernet/marvell/octeontx2/nic/qos.c:1195:6: error: variable 'node' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
if (prio > OTX2_QOS_MAX_PRIO) {
^~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/marvell/octeontx2/nic/qos.c:1288:18: note: uninitialized use occurs here
clear_bit(prio, node->prio_bmap);
^~~~
drivers/net/ethernet/marvell/octeontx2/nic/qos.c:1195:2: note: remove the 'if' if its condition is always false
if (prio > OTX2_QOS_MAX_PRIO) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/marvell/octeontx2/nic/qos.c:1187:28: note: initialize the variable 'node' to silence this warning
struct otx2_qos_node *node, *child;
^
= NULL
> + }
> +
> + /* find node related to classid */
> + node = otx2_sw_node_find(pfvf, classid);
> + if (!node) {
> + NL_SET_ERR_MSG_MOD(extack, "HTB node not found");
> + ret = -ENOENT;
> + goto out;
> + }
...
> + return 0;
> +
> +free_node:
> + otx2_qos_sw_node_delete(pfvf, child);
> +free_old_cfg:
> + kfree(old_cfg);
> +out:
> + clear_bit(prio, node->prio_bmap);
> + return ret;
> +}
...