> -----Original Message-----
> From: Ferruh Yigit <ferruh.yi...@amd.com>
> Sent: Tuesday, March 7, 2023 11:03 PM
> To: Liu, Mingxia <mingxia....@intel.com>; Xing, Beilei
> <beilei.x...@intel.com>;
> Zhang, Yuying <yuying.zh...@intel.com>; Raslan Darawsheh
> <rasl...@nvidia.com>
> Cc: dev@dpdk.org; Stephen Hemminger <step...@networkplumber.org>;
> Richardson, Bruce <bruce.richard...@intel.com>; Zhang, Qi Z
> <qi.z.zh...@intel.com>
> Subject: Re: [PATCH v9 01/21] net/cpfl: support device initialization
>
> On 3/7/2023 2:11 PM, Ferruh Yigit wrote:
> > On 3/2/2023 9:20 PM, Mingxia Liu wrote:
> >> Support device init and add the following dev ops:
> >> - dev_configure
> >> - dev_close
> >> - dev_infos_get
> >> - link_update
> >> - dev_supported_ptypes_get
> >>
> >> Signed-off-by: Mingxia Liu <mingxia....@intel.com>
> >
> > <...>
> >
> >> +static void
> >> +cpfl_handle_virtchnl_msg(struct cpfl_adapter_ext *adapter) {
> >> + struct idpf_adapter *base = &adapter->base;
> >> + struct idpf_dma_mem *dma_mem = NULL;
> >> + struct idpf_hw *hw = &base->hw;
> >> + struct virtchnl2_event *vc_event;
> >> + struct idpf_ctlq_msg ctlq_msg;
> >> + enum idpf_mbx_opc mbx_op;
> >> + struct idpf_vport *vport;
> >> + enum virtchnl_ops vc_op;
> >> + uint16_t pending = 1;
> >> + int ret;
> >> +
> >> + while (pending) {
> >> + ret = idpf_vc_ctlq_recv(hw->arq, &pending, &ctlq_msg);
> >> + if (ret) {
> >> + PMD_DRV_LOG(INFO, "Failed to read msg from virtual
> channel, ret: %d", ret);
> >> + return;
> >> + }
> >> +
> >> + memcpy(base->mbx_resp, ctlq_msg.ctx.indirect.payload->va,
> >> + IDPF_DFLT_MBX_BUF_SIZE);
> >> +
> >> + mbx_op = rte_le_to_cpu_16(ctlq_msg.opcode);
> >> + vc_op = rte_le_to_cpu_32(ctlq_msg.cookie.mbx.chnl_opcode);
> >> + base->cmd_retval =
> >> +rte_le_to_cpu_32(ctlq_msg.cookie.mbx.chnl_retval);
> >> +
> >> + switch (mbx_op) {
> >> + case idpf_mbq_opc_send_msg_to_peer_pf:
> >> + if (vc_op == VIRTCHNL2_OP_EVENT) {
> >
> >
> > Raslan reported following build error [1], 'VIRTCHNL2_OP_EVENT' is not
> > an element of "enum virtchnl_ops", can you please check?
> >
> >
> > I guess there are a few options, have a new enum for virtchnl2, like
> > "enum virtchnl2_ops" which inlucde all 'VIRTCHNL2_OP_',
> >
> > OR
> >
> > use 'uint32_t' type (instead of "enum virtchnl_ops") when
> > 'VIRTCHNL2_OP_' opcodes can be used, this seems simpler.
> >
> >
> > BTW, this is same in the idfp driver.
> >
> >
> > [1]
> > drivers/libtmp_rte_net_cpfl.a.p/net_cpfl_cpfl_ethdev.c.o -c
> > ../../root/dpdk/drivers/net/cpfl/cpfl_ethdev.c
> > ../../root/dpdk/drivers/net/cpfl/cpfl_ethdev.c:1118:14: error:
> > comparison of constant 522 with expression of type 'enum virtchnl_ops'
> > is always false [-Werror,-Wtautological-constant-out-of-range-compare]
> > if (vc_op == VIRTCHNL2_OP_EVENT) {
> > ~~~~~ ^ ~~~~~~~~~~~~~~~~~~
> > 1 error generated.
> >
>
> Thinking twice, I am not sure if this a compiler issue or coding issue, many
> compilers doesn't complain about above issue.
>
> As far as I understand C allows assigning unlisted values to enums, because
> underneath it just uses an integer type.
>
> Only caveat I can see is, the integer type used is not fixed, technically
> compiler
> can select the type that fits all enum values, so for above enum compiler can
> select an char type to store the values, but fixed value is 522 out of the
> char
> limit may cause an issue. But in practice I am not sure if compilers are
> selecting
> char as underlying type, or if they all just use 'int'.
[Liu, Mingxia] By checking the code, we shouldn't compare an enum virtchnl_ops
variable with VIRTCHNL2_OP_EVENT,
as VIRTCHNL2_OP_EVENT is not included in enum virtchnl_ops. And the cpfl/idpf
pmd use virtual msg opcodes prefixed with virtchnl2 or
VIRTCHNL2. I'll send a fixed patch to fix this issue.
