[v4,03/34] common/sfc_efx/base: add API to list HW tables
Checks
Commit Message
From: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
New MCDI Table Access API allows management of
the HW tables' content.
This part of API helps to list all supported tables.
In the near future, only the CT table is planned
to be used, so only one identifier for this table
was added to the efx.
New table IDs will be added as needed.
Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
Reviewed-by: Andy Moreton <amoreton@xilinx.com>
---
drivers/common/sfc_efx/base/efx.h | 15 ++++
drivers/common/sfc_efx/base/efx_table.c | 94 +++++++++++++++++++++++++
drivers/common/sfc_efx/base/meson.build | 1 +
drivers/common/sfc_efx/version.map | 2 +
4 files changed, 112 insertions(+)
create mode 100644 drivers/common/sfc_efx/base/efx_table.c
Comments
On 6/7/2023 2:02 PM, Ivan Malov wrote:
> From: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
>
> New MCDI Table Access API allows management of
> the HW tables' content.
> This part of API helps to list all supported tables.
> In the near future, only the CT table is planned
> to be used, so only one identifier for this table
> was added to the efx.
> New table IDs will be added as needed.
>
> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
>
This patch adds a function to the base code, but it is disconnected from
the context.
In the future if someone looks this function in git log, there is no
easy way to see why this function added and where/how it is used at time
it is added etc..
So, instead of making commit per function, can you please split commits
based on functionality/logic?
Please combine the commit that new function and commit where new
function is used to single commit, making a commit per feature?
If you are concerned about checkpatch warnings related to the component
(like common/sfc_efx/base), please ignore it for the case when a feature
is distributed into multiple components, and feel free to use most
appropriate component name, I assume it will be driver component
(net/sfc) most of the times.
There is apply errors on CI, which prevents CI checks, can you please
rebase set on top of latest head?
Btw, we call 'API' to end-user facing functions, that user directly
call, for this context better to call it 'function', but after patches
merged probably you won't need it at all.
Thanks,
Ferruh
Hi Ferruh,
Thank you so much for your review notes. You suggested to squash
some pairs of "common/sfc_base/efx" and "net/sfc" patches, so
that logical changes would be unified. I see your point.
On the other hand, however, doing so would be rather unusual
from our internal process standpoint. Typically, we stick
with the idea that splitting patches into smaller ones is
better, as it is much easier to reason about and debug
smaller changesets rather than bigger ones.
The thing is, the DPDK driver is not the only one based on
the common code ("libefx"), that is, we got used to
keeping things separate, even despite some of them
beging logically connected. I apologise in case
my explanation is still vague.
If the reader comes across a libefx patch in one of the
other libefx-based projects and wants to search for it
in the other projects (DPDK), it is much easier for
them to find what they need provided that the patch
exists in DPDK in the same format, as a separate
change set with (almost) the same commit summary.
In other words, there are pros and cons to squashing
things, well, at least in this particular series,
which is rather big and complicated.
How about we retain the series as it is, in its current state
this time? We hope to adopt the suggested ("bigger logical
patches") next time, in our future work. What do you think?
We would discuss this internally, with our team, and come
up with the new approach for us all to structure future
patch sets, for some other features yet to be supported.
Thank you.
On Mon, 19 Jun 2023, Ferruh Yigit wrote:
> On 6/7/2023 2:02 PM, Ivan Malov wrote:
>> From: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
>>
>> New MCDI Table Access API allows management of
>> the HW tables' content.
>> This part of API helps to list all supported tables.
>> In the near future, only the CT table is planned
>> to be used, so only one identifier for this table
>> was added to the efx.
>> New table IDs will be added as needed.
>>
>> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
>> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
>>
>
> This patch adds a function to the base code, but it is disconnected from
> the context.
> In the future if someone looks this function in git log, there is no
> easy way to see why this function added and where/how it is used at time
> it is added etc..
>
> So, instead of making commit per function, can you please split commits
> based on functionality/logic?
>
> Please combine the commit that new function and commit where new
> function is used to single commit, making a commit per feature?
>
>
> If you are concerned about checkpatch warnings related to the component
> (like common/sfc_efx/base), please ignore it for the case when a feature
> is distributed into multiple components, and feel free to use most
> appropriate component name, I assume it will be driver component
> (net/sfc) most of the times.
>
>
> There is apply errors on CI, which prevents CI checks, can you please
> rebase set on top of latest head?
>
>
> Btw, we call 'API' to end-user facing functions, that user directly
> call, for this context better to call it 'function', but after patches
> merged probably you won't need it at all.
>
> Thanks,
> Ferruh
>
On 6/21/2023 12:09 PM, Ivan Malov wrote:
> Hi Ferruh,
>
> Thank you so much for your review notes. You suggested to squash
> some pairs of "common/sfc_base/efx" and "net/sfc" patches, so
> that logical changes would be unified. I see your point.
>
> On the other hand, however, doing so would be rather unusual
> from our internal process standpoint. Typically, we stick
> with the idea that splitting patches into smaller ones is
> better, as it is much easier to reason about and debug
> smaller changesets rather than bigger ones.
>
> The thing is, the DPDK driver is not the only one based on
> the common code ("libefx"), that is, we got used to
> keeping things separate, even despite some of them
> beging logically connected. I apologise in case
> my explanation is still vague.
>
> If the reader comes across a libefx patch in one of the
> other libefx-based projects and wants to search for it
> in the other projects (DPDK), it is much easier for
> them to find what they need provided that the patch
> exists in DPDK in the same format, as a separate
> change set with (almost) the same commit summary.
>
> In other words, there are pros and cons to squashing
> things, well, at least in this particular series,
> which is rather big and complicated.
>
> How about we retain the series as it is, in its current state
> this time? We hope to adopt the suggested ("bigger logical
> patches") next time, in our future work. What do you think?
>
> We would discuss this internally, with our team, and come
> up with the new approach for us all to structure future
> patch sets, for some other features yet to be supported.
>
Ack, thanks.
Let me continue with this set as it is taking into account that -rc2 is
so close, we can sync more for future patches.
> Thank you.
>
> On Mon, 19 Jun 2023, Ferruh Yigit wrote:
>
>> On 6/7/2023 2:02 PM, Ivan Malov wrote:
>>> From: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
>>>
>>> New MCDI Table Access API allows management of
>>> the HW tables' content.
>>> This part of API helps to list all supported tables.
>>> In the near future, only the CT table is planned
>>> to be used, so only one identifier for this table
>>> was added to the efx.
>>> New table IDs will be added as needed.
>>>
>>> Signed-off-by: Denis Pryazhennikov <denis.pryazhennikov@arknetworks.am>
>>> Reviewed-by: Andy Moreton <amoreton@xilinx.com>
>>>
>>
>> This patch adds a function to the base code, but it is disconnected from
>> the context.
>> In the future if someone looks this function in git log, there is no
>> easy way to see why this function added and where/how it is used at time
>> it is added etc..
>>
>> So, instead of making commit per function, can you please split commits
>> based on functionality/logic?
>>
>> Please combine the commit that new function and commit where new
>> function is used to single commit, making a commit per feature?
>>
>>
>> If you are concerned about checkpatch warnings related to the component
>> (like common/sfc_efx/base), please ignore it for the case when a feature
>> is distributed into multiple components, and feel free to use most
>> appropriate component name, I assume it will be driver component
>> (net/sfc) most of the times.
>>
>>
>> There is apply errors on CI, which prevents CI checks, can you please
>> rebase set on top of latest head?
>>
>>
>> Btw, we call 'API' to end-user facing functions, that user directly
>> call, for this context better to call it 'function', but after patches
>> merged probably you won't need it at all.
>>
>> Thanks,
>> Ferruh
>>
@@ -5070,6 +5070,21 @@ efx_nic_dma_map(
__in size_t len,
__out efsys_dma_addr_t *nic_addrp);
+/* Unique IDs for HW tables */
+typedef enum efx_table_id_e {
+ EFX_TABLE_ID_CONNTRACK = 0x10300,
+} efx_table_id_t;
+
+LIBEFX_API
+extern __checkReturn efx_rc_t
+efx_table_list(
+ __in efx_nic_t *enp,
+ __in uint32_t entry_ofst,
+ __out_opt unsigned int *total_n_tablesp,
+ __out_ecount_opt(n_table_ids) efx_table_id_t *table_ids,
+ __in unsigned int n_table_ids,
+ __out_opt unsigned int *n_table_ids_writtenp);
+
#ifdef __cplusplus
}
#endif
new file mode 100644
@@ -0,0 +1,94 @@
+/* SPDX-License-Identifier: BSD-3-Clause
+ *
+ * Copyright (c) 2023 Advanced Micro Devices, Inc.
+ */
+
+#include "efx.h"
+#include "efx_impl.h"
+
+ __checkReturn efx_rc_t
+efx_table_list(
+ __in efx_nic_t *enp,
+ __in uint32_t entry_ofst,
+ __out_opt unsigned int *total_n_tablesp,
+ __out_ecount_opt(n_table_ids) efx_table_id_t *table_ids,
+ __in unsigned int n_table_ids,
+ __out_opt unsigned int *n_table_ids_writtenp)
+{
+ const efx_nic_cfg_t *encp = efx_nic_cfg_get(enp);
+ unsigned int n_entries;
+ efx_mcdi_req_t req;
+ unsigned int i;
+ efx_rc_t rc;
+ EFX_MCDI_DECLARE_BUF(payload,
+ MC_CMD_TABLE_LIST_IN_LEN,
+ MC_CMD_TABLE_LIST_OUT_LENMAX_MCDI2);
+
+ /* Ensure EFX and MCDI use same values for table IDs */
+ EFX_STATIC_ASSERT(EFX_TABLE_ID_CONNTRACK == TABLE_ID_CONNTRACK_TABLE);
+
+ if (encp->enc_table_api_supported == B_FALSE) {
+ rc = ENOTSUP;
+ goto fail1;
+ }
+
+ if ((n_table_ids != 0) &&
+ ((table_ids == NULL) || (n_table_ids_writtenp == NULL))) {
+ rc = EINVAL;
+ goto fail2;
+ }
+
+ req.emr_cmd = MC_CMD_TABLE_LIST;
+ req.emr_in_buf = payload;
+ req.emr_in_length = MC_CMD_TABLE_LIST_IN_LEN;
+ req.emr_out_buf = payload;
+ req.emr_out_length = MC_CMD_TABLE_LIST_OUT_LENMAX_MCDI2;
+
+ MCDI_IN_SET_DWORD(req, TABLE_LIST_IN_FIRST_TABLE_ID_INDEX, entry_ofst);
+
+ efx_mcdi_execute(enp, &req);
+
+ if (req.emr_rc != 0) {
+ rc = req.emr_rc;
+ goto fail3;
+ }
+
+ if (req.emr_out_length_used < MC_CMD_TABLE_LIST_OUT_LENMIN) {
+ rc = EMSGSIZE;
+ goto fail4;
+ }
+
+ if (total_n_tablesp != NULL)
+ *total_n_tablesp = MCDI_OUT_DWORD(req, TABLE_LIST_OUT_N_TABLES);
+
+ n_entries = MC_CMD_TABLE_LIST_OUT_TABLE_ID_NUM(req.emr_out_length_used);
+
+ if (table_ids != NULL) {
+ if (n_entries > n_table_ids) {
+ rc = ENOMEM;
+ goto fail5;
+ }
+
+ for (i = 0; i < n_entries; i++) {
+ table_ids[i] = MCDI_OUT_INDEXED_DWORD(req,
+ TABLE_LIST_OUT_TABLE_ID, i);
+ }
+ }
+
+ if (n_table_ids_writtenp != NULL)
+ *n_table_ids_writtenp = n_entries;
+
+ return (0);
+
+fail5:
+ EFSYS_PROBE(fail5);
+fail4:
+ EFSYS_PROBE(fail4);
+fail3:
+ EFSYS_PROBE(fail3);
+fail2:
+ EFSYS_PROBE(fail2);
+fail1:
+ EFSYS_PROBE1(fail1, efx_rc_t, rc);
+ return (rc);
+}
@@ -16,6 +16,7 @@ sources = [
'efx_lic.c',
'efx_mac.c',
'efx_mae.c',
+ 'efx_table.c',
'efx_mcdi.c',
'efx_mon.c',
'efx_nic.c',
@@ -232,6 +232,8 @@ INTERNAL {
efx_sram_buf_tbl_clear;
efx_sram_buf_tbl_set;
+ efx_table_list;
+
efx_tunnel_config_clear;
efx_tunnel_config_udp_add;
efx_tunnel_config_udp_remove;