From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 2316441B8D; Tue, 31 Jan 2023 10:34:21 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id F192840E28; Tue, 31 Jan 2023 10:34:20 +0100 (CET) Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by mails.dpdk.org (Postfix) with ESMTP id 8657040DFB; Tue, 31 Jan 2023 10:34:18 +0100 (CET) Received: from kwepemi500017.china.huawei.com (unknown [172.30.72.57]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4P5fs810CrzJqnL; Tue, 31 Jan 2023 17:29:48 +0800 (CST) Received: from [10.67.103.235] (10.67.103.235) by kwepemi500017.china.huawei.com (7.221.188.110) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2375.34; Tue, 31 Jan 2023 17:34:16 +0800 Subject: Re: [PATCH V2 09/10] net/hns3: fix bad memory structure conversion To: , , , , References: <20230129105140.29921-1-liudongdong3@huawei.com> <20230130093129.18529-1-liudongdong3@huawei.com> <20230130093129.18529-10-liudongdong3@huawei.com> CC: , From: Dongdong Liu Message-ID: <5e1bc8d5-84ac-95ab-8d1b-6589dc5b5c82@huawei.com> Date: Tue, 31 Jan 2023 17:34:15 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.7.1 MIME-Version: 1.0 In-Reply-To: <20230130093129.18529-10-liudongdong3@huawei.com> Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.103.235] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To kwepemi500017.china.huawei.com (7.221.188.110) X-CFilter-Loop: Reflected X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On 2023/1/30 17:31, Dongdong Liu wrote: > From: Huisong Li > > When the type in 'struct rte_flow_action' is RTE_FLOW_ACTION_TYPE_RSS, the > 'conf' pointer references the 'struct rte_flow_action_rss' instead of the > 'struct hns3_rss_conf' in driver. But driver uses 'struct hns3_rss_conf' to > convert this 'conf' pointer to get RSS action configuration. > > In addition, RSS filter configuration is directly cloned to RSS filter node > instead of coping it after successfully setting to hardware. > > Fixes: c37ca66f2b27 ("net/hns3: support RSS") > Cc: stable@dpdk.org > > Signed-off-by: Huisong Li > Signed-off-by: Dongdong Liu > --- > drivers/net/hns3/hns3_flow.c | 57 +++++++++++++----------------------- > 1 file changed, 20 insertions(+), 37 deletions(-) > > diff --git a/drivers/net/hns3/hns3_flow.c b/drivers/net/hns3/hns3_flow.c > index fbc38dd3d4..307aba75a7 100644 > --- a/drivers/net/hns3/hns3_flow.c > +++ b/drivers/net/hns3/hns3_flow.c > @@ -95,8 +95,8 @@ static const struct rte_flow_action * > hns3_find_rss_general_action(const struct rte_flow_item pattern[], > const struct rte_flow_action actions[]) > { > + const struct rte_flow_action_rss *rss_act; > const struct rte_flow_action *act = NULL; > - const struct hns3_rss_conf *rss; > bool have_eth = false; > > for (; actions->type != RTE_FLOW_ACTION_TYPE_END; actions++) { > @@ -115,8 +115,8 @@ hns3_find_rss_general_action(const struct rte_flow_item pattern[], > } > } > > - rss = act->conf; > - if (have_eth && rss->conf.queue_num) { > + rss_act = act->conf; > + if (have_eth && rss_act->queue_num) { > /* > * Pattern have ETH and action's queue_num > 0, indicate this is > * queue region configuration. > @@ -1296,30 +1296,6 @@ hns3_action_rss_same(const struct rte_flow_action_rss *comp, > sizeof(*with->queue) * with->queue_num)); > } > > -static int > -hns3_rss_conf_copy(struct hns3_rss_conf *out, > - const struct rte_flow_action_rss *in) > -{ > - if (in->key_len > RTE_DIM(out->key) || > - in->queue_num > RTE_DIM(out->queue)) > - return -EINVAL; > - if (in->key == NULL && in->key_len) > - return -EINVAL; > - out->conf = (struct rte_flow_action_rss) { > - .func = in->func, > - .level = in->level, > - .types = in->types, > - .key_len = in->key_len, > - .queue_num = in->queue_num, > - }; > - out->conf.queue = memcpy(out->queue, in->queue, > - sizeof(*in->queue) * in->queue_num); > - if (in->key) > - out->conf.key = memcpy(out->key, in->key, in->key_len); > - > - return 0; > -} > - > static bool > hns3_rss_input_tuple_supported(struct hns3_hw *hw, > const struct rte_flow_action_rss *rss) > @@ -1733,9 +1709,10 @@ hns3_flow_create_rss_rule(struct rte_eth_dev *dev, > struct rte_flow *flow) > { > struct hns3_hw *hw = HNS3_DEV_PRIVATE_TO_HW(dev->data->dev_private); > + const struct rte_flow_action_rss *rss_act; > struct hns3_rss_conf_ele *rss_filter_ptr; > struct hns3_rss_conf_ele *filter_ptr; > - const struct hns3_rss_conf *rss_conf; > + struct hns3_rss_conf *new_conf; > int ret; > > rss_filter_ptr = rte_zmalloc("hns3 rss filter", > @@ -1745,19 +1722,25 @@ hns3_flow_create_rss_rule(struct rte_eth_dev *dev, > return -ENOMEM; > } > > - /* > - * After all the preceding tasks are successfully configured, configure > - * rules to the hardware to simplify the rollback of rules in the > - * hardware. > - */ > - rss_conf = (const struct hns3_rss_conf *)act->conf; > - ret = hns3_flow_parse_rss(dev, rss_conf, true); > + rss_act = (const struct rte_flow_action_rss *)act->conf; > + new_conf = &rss_filter_ptr->filter_info; > + memcpy(&new_conf->conf, rss_act, sizeof(*rss_act)); > + if (rss_act->queue_num > 0) { > + memcpy(new_conf->queue, rss_act->queue, > + rss_act->queue_num * sizeof(new_conf->queue[0])); > + new_conf->conf.queue = new_conf->queue; > + } > + if (rss_act->key_len > 0) { When do the below test, Segmentation fault occurred. testpmd> flow create 0 ingress pattern end actions rss key_len 40 / end Segmentation fault (core dumped) It should make sure new_conf->key is not NULL before doing memcpy. Will fix this in V3. Thanks, Dongdong > + memcpy(new_conf->key, rss_act->key, > + rss_act->key_len * sizeof(new_conf->key[0])); > + new_conf->conf.key = new_conf->key; > + } > + > + ret = hns3_flow_parse_rss(dev, new_conf, true); > if (ret != 0) { > rte_free(rss_filter_ptr); > return ret; > } > - > - hns3_rss_conf_copy(&rss_filter_ptr->filter_info, &rss_conf->conf); > rss_filter_ptr->filter_info.valid = true; > > /* >