[dpdk-dev] [PATCH] i40e: Unchecked return value
bruce.richardson at intel.com
Mon Jun 13 12:04:07 CEST 2016
On Mon, May 23, 2016 at 02:25:15PM +0200, Slawomir Mrozowicz wrote:
> Calling i40e_switch_tx_queue without checking return value.
> Fixed by add warning log information if return failed.
> Fixes: 71d35259ff67 ("i40e: tear down flow director")
> Coverity ID 13208
> Signed-off-by: Slawomir Mrozowicz <slawomirx.mrozowicz at intel.com>
> drivers/net/i40e/i40e_fdir.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
> diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
> index 8aa41e5..d0bdf2c 100644
> --- a/drivers/net/i40e/i40e_fdir.c
> +++ b/drivers/net/i40e/i40e_fdir.c
> @@ -288,11 +288,14 @@ i40e_fdir_teardown(struct i40e_pf *pf)
> struct i40e_hw *hw = I40E_PF_TO_HW(pf);
> struct i40e_vsi *vsi;
> + int err = I40E_SUCCESS;
> vsi = pf->fdir.fdir_vsi;
> if (!vsi)
> - i40e_switch_tx_queue(hw, vsi->base_queue, FALSE);
> + err = i40e_switch_tx_queue(hw, vsi->base_queue, FALSE);
> + if (err)
> + PMD_DRV_LOG(WARNING, "Failed to do FDIR TX switch off.");
> i40e_switch_rx_queue(hw, vsi->base_queue, FALSE);
So, we have a failure when we can't swtich off flow director in a queue. How
serious is this? Is it something that can be completely ignored, or is printing
a warning sufficient? What, if anything, should the user do about the warning?
I'm just concerned that this patch doesn't seem to help the overall usability
of DPDK much. We print a warning, which will probably be of absolutely no use
to the user at all. It doesn't tell the user what the failure will mean in
practical terms - will the failure mean that transmit won't work, that packets
may be corrupted, may go out on a wrong queue, etc., or how the user can prevent
the error from happening in the future.
Please review patch to ensure this is the best way to fix this error - if any
fix is needed. If the error doesn't cause any problematic user effects, then
just mark the coverity issue as a false positive (or does it work casting the
function to (void) as it is called?). If the error does have problematic effects,
please provide useful information to the user.
More information about the dev