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 11ABD45BFE; Mon, 28 Oct 2024 17:15:06 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D1A6D410D0; Mon, 28 Oct 2024 17:15:04 +0100 (CET) Received: from mail-pg1-f170.google.com (mail-pg1-f170.google.com [209.85.215.170]) by mails.dpdk.org (Postfix) with ESMTP id 4A7E141060 for ; Mon, 28 Oct 2024 17:15:04 +0100 (CET) Received: by mail-pg1-f170.google.com with SMTP id 41be03b00d2f7-7edb6879196so2792622a12.3 for ; Mon, 28 Oct 2024 09:15:04 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20230601.gappssmtp.com; s=20230601; t=1730132103; x=1730736903; darn=dpdk.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=6BPe1o6wuUVMtj1bZ5oYYEmwVKXPftv1GoKJslOe/ag=; b=j3G0rVnJkXJ/AUqzGoHfvhZ2GlOANxSRsByT2uMnl1Bao8sMxr14ObtkLjKUgmCpR4 DXi4UnTKqU9q9oevCiVYUZU/4AV/m6+wOvdnFGfA2PUPgOQb9qKQAt7Y9sGTLMnYcS7L P0fOTFnu6m5C41iglHrDiRPmU3zHqYzycy+V6KNlipFCeUeu/Ca6qRD2aaL0P7qSY1Rh n8NlwTSvkdd8ilDT0dPbsP8IOuvIWUnBjK0IoYRaNVkia4d4dG2w3s+R4S99DuGuJEVF MD48dATIw1r5XSkyxoxL3yFZoPrUqjAidM7L7bi8+aWs65ITtbwAk/rBQRksgQ5nVQBl DoYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1730132103; x=1730736903; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=6BPe1o6wuUVMtj1bZ5oYYEmwVKXPftv1GoKJslOe/ag=; b=QG/iHHKl8lZ7kesfJ+vtrgA2P/4lv3GV/FLcvTuqhuTs/pQcbP5ouv8SgSMCTiIQzq PD25vmJT6XZAz/crZ0/ikJDkA5jOBLRcK+0gBcKNJPULsbQLlnrO5YL0s9leTNqcdg4p TP5GlSTYG0TMi1x8m5zFwBMhPB5oYpjFV33nRLOAz/U7GXB7nr7ivLfzFBTCl00Jz7LC VDoHakqmbhgfHQbVXV03+4kXMpB+W/gce4bWHHzL65urmb9J4wr/RSdwkdJRhbsIj2r+ Dw5W9MNzZKIM4qOsRMUD8k05JrBUOKrCXhyioDDGh2GUgZFsbtaWTY2lfWci3YCHv9gR /WaA== X-Gm-Message-State: AOJu0YzrTXTWk4wyuyJa/jF9PTAxG402G61uj2UkF+2r8RdriVd+lirk f3+Bd6qSv6p1ymZPnxb/Jx1Ey+58PAaKAuruJC/azOGQUvBZPflz2Q8MCsarZa4= X-Google-Smtp-Source: AGHT+IFEAOSfmA0/6jVKrQz9NO8xSp/+xKil40qB0Xyqxj8DsQTfk8weuyXxyTlbb+qw4Xq25vvibA== X-Received: by 2002:a05:6a20:8c22:b0:1d9:c6a6:61d8 with SMTP id adf61e73a8af0-1d9c6a661dbmr3753057637.7.1730132103123; Mon, 28 Oct 2024 09:15:03 -0700 (PDT) Received: from hermes.local (204-195-96-226.wavecable.com. [204.195.96.226]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72057921619sm5916483b3a.4.2024.10.28.09.15.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 28 Oct 2024 09:15:02 -0700 (PDT) Date: Mon, 28 Oct 2024 09:15:00 -0700 From: Stephen Hemminger To: Serhii Iliushyk Cc: dev@dpdk.org, mko-plv@napatech.com, ckm@napatech.com, andrew.rybchenko@oktetlabs.ru, ferruh.yigit@amd.com Subject: Re: [PATCH v3 53/73] net/ntnic: enable RSS feature Message-ID: <20241028091500.0383e499@hermes.local> In-Reply-To: <20241023170032.314155-54-sil-plv@napatech.com> References: <20241021210527.2075431-1-sil-plv@napatech.com> <20241023170032.314155-1-sil-plv@napatech.com> <20241023170032.314155-54-sil-plv@napatech.com> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit 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 Wed, 23 Oct 2024 19:00:01 +0200 Serhii Iliushyk wrote: > + > + rte_memcpy(&tmp_rss_conf.rss_key, rss_conf->rss_key, rss_conf->rss_key_len); Avoid use of rte_memcpy(), it has less checking that memcpy(). The only place it can make sense is in the hot data path where compiler is more conservative about alignment. > + rte_memcpy(&ndev->rss_conf, &tmp_rss_conf, sizeof(struct nt_eth_rss_conf)); > Use structure assignment instead of memcpy() to keep type checking. > + if (rss_conf->rss_key != NULL) { > + int key_len = rss_conf->rss_key_len < MAX_RSS_KEY_LEN ? rss_conf->rss_key_len > + : MAX_RSS_KEY_LEN; Use RTE_MIN() and the key_len variable should not be signed. > + memset(rss_conf->rss_key, 0, rss_conf->rss_key_len); > + rte_memcpy(rss_conf->rss_key, &ndev->rss_conf.rss_key, key_len); > +static int eth_dev_rss_hash_update(struct rte_eth_dev *eth_dev, struct rte_eth_rss_conf *rss_conf) > +{ > + const struct flow_filter_ops *flow_filter_ops = get_flow_filter_ops(); > + > + if (flow_filter_ops == NULL) { > + NT_LOG_DBGX(ERR, NTNIC, "flow_filter module uninitialized"); > + return -1; > + } > + > + struct pmd_internals *internals = (struct pmd_internals *)eth_dev->data->dev_private; Since dev_private is void *, no need for the cast here (in C code). > + > + struct flow_nic_dev *ndev = internals->flw_dev->ndev; > + struct nt_eth_rss_conf tmp_rss_conf = { 0 }; > + const int hsh_idx = 0; /* hsh index 0 means the default receipt in HSH module */ > + int res = 0; > + > + if (rss_conf->rss_key != NULL) { > + if (rss_conf->rss_key_len > MAX_RSS_KEY_LEN) { > + NT_LOG(ERR, NTNIC, > + "ERROR: - RSS hash key length %u exceeds maximum value %u", > + rss_conf->rss_key_len, MAX_RSS_KEY_LEN); > + return -1; > + } > + > + rte_memcpy(&tmp_rss_conf.rss_key, rss_conf->rss_key, rss_conf->rss_key_len); > + } > + > + tmp_rss_conf.algorithm = rss_conf->algorithm; > + > + tmp_rss_conf.rss_hf = rss_conf->rss_hf; > + res = flow_filter_ops->flow_nic_set_hasher_fields(ndev, hsh_idx, tmp_rss_conf); In general, this code is good about moving declarations next to first use. But here res is initialized to 0 but then set again from hasher_fields. Why not just move the declaration there.