On 11/4/19 12:46 PM, Hemant Agrawal wrote:
> Hi Andrzej
Hello Hemant,
Thank you for your comments.
>> -----Original Message-----
>> From: Andrzej Ostruszka <aostrus...@marvell.com>
[...]
>> a/drivers/net/dpaa2/mc/dpkg.c b/drivers/net/dpaa2/mc/dpkg.c index
>> 80f94f40e..7aa63ea12 100644
>> --- a/drivers/net/dpaa2/mc/dpkg.c
>> +++ b/drivers/net/dpaa2/mc/dpkg.c
>> @@ -63,7 +63,7 @@ dpkg_prepare_key_cfg(const struct dpkg_profile_cfg
>> *cfg, uint8_t *key_cfg_buf)
>> dpkg_set_field(extr->extract_type, EXTRACT_TYPE,
>> cfg->extracts[i].type);
>>
> [Hemant] Please add a check here to avoid array overrun
> + if (extr->num_of_byte_masks > DPKG_NUM_OF_MASKS)
> + return -EINVAL;
> +
Will do, thank you.
>> - for (j = 0; j < DPKG_NUM_OF_MASKS; j++) {
>> + for (j = 0; j < extr->num_of_byte_masks; j++) {
>> extr->masks[j].mask = cfg-
>>> extracts[i].masks[j].mask;
>> extr->masks[j].offset =
>> cfg->extracts[i].masks[j].offset;
>> diff --git a/drivers/net/dpaa2/mc/dpni.c b/drivers/net/dpaa2/mc/dpni.c
>> index 0950ee007..89a64b0ab 100644
>> --- a/drivers/net/dpaa2/mc/dpni.c
>> +++ b/drivers/net/dpaa2/mc/dpni.c
>> @@ -1839,10 +1839,13 @@ int dpni_set_congestion_notification(struct
>> fsl_mc_io *mc_io,
>> cmd_params->qtype = qtype;
>> cmd_params->tc = tc_id;
>> cmd_params->congestion_point = cfg->cg_point;
>> - cmd_params->cgid = (uint8_t)cfg->cgid;
>> - cmd_params->dest_id = cpu_to_le32(cfg->dest_cfg.dest_id);
>> + if (cfg->cg_point == DPNI_CP_CONGESTION_GROUP)
>> + cmd_params->cgid = (uint8_t)cfg->cgid;
>> + if (cfg->dest_cfg.dest_type != DPNI_DEST_NONE) {
>> + cmd_params->dest_id = cpu_to_le32(cfg-
>>> dest_cfg.dest_id);
>> + cmd_params->dest_priority = cfg->dest_cfg.priority;
>> + }
> [Hemant] What is the explicit error you got here?
> In the calling function, we are resetting the structure to 0. Which should
> have avoided
> Any un-initialized var errors?
> struct dpni_congestion_notification_cfg cong_notif_cfg = {0};
Indeed there is no warning from compiler here at the current version.
The original version was developed against 18.05 and this is what I used
to make compiler happy. I've kept this change (even though compiler
does not report anything here) since to me it looked like a good change
(use the cgid only in case of "congestion group" and use dest_* only
when not DEST_NONE).
You have obviously more authority here so I will remove these changes
(unless you prefer them to stay - in which case please indicate so).
Again thank you for the comments.
Regards
Andrzej
