On 07 Dec 10:08, Tony Nguyen wrote:
From: Bartosz Staszewski <bartoszx.staszewski@xxxxxxxxx> Whenever trying to load XDP prog on downed interface, function i40e_xdp was passing vsi->rx_buf_len field to i40e_xdp_setup() which was equal 0. i40e_open() calls i40e_vsi_configure_rx() which configures that field, but that only happens when interface is up. When it is down, i40e_open() is not being called, thus vsi->rx_buf_len is not set. Solution for this is calculate buffer length in newly created function - i40e_calculate_vsi_rx_buf_len() that return actual buffer length. Buffer length is being calculated based on the same rules applied previously in i40e_vsi_configure_rx() function. Fixes: 613142b0bb88 ("i40e: Log error for oversized MTU on device") Fixes: 0c8493d90b6b ("i40e: add XDP support for pass and drop actions") Signed-off-by: Bartosz Staszewski <bartoszx.staszewski@xxxxxxxxx> Signed-off-by: Mateusz Palczewski <mateusz.palczewski@xxxxxxxxx> Tested-by: Shwetha Nagaraju <Shwetha.nagaraju@xxxxxxxxx> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@xxxxxxxxx> Signed-off-by: Tony Nguyen <anthony.l.nguyen@xxxxxxxxx> --- v2: - Change title and rework commit message - Dropped, previous, patch 1 v1: https://lore.kernel.org/netdev/20221115000324.3040207-1-anthony.l.nguyen@xxxxxxxxx/ drivers/net/ethernet/intel/i40e/i40e_main.c | 42 +++++++++++++++------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/drivers/net/ethernet/intel/i40e/i40e_main.c b/drivers/net/ethernet/intel/i40e/i40e_main.c index 6416322d7c18..b8a8098110eb 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_main.c +++ b/drivers/net/ethernet/intel/i40e/i40e_main.c @@ -3693,6 +3693,30 @@ static int i40e_vsi_configure_tx(struct i40e_vsi *vsi) return err; } +/** + * i40e_calculate_vsi_rx_buf_len - Calculates buffer length + * + * @vsi: VSI to calculate rx_buf_len from + */ +static u16 i40e_calculate_vsi_rx_buf_len(struct i40e_vsi *vsi) +{ + u16 ret; + + if (!vsi->netdev || (vsi->back->flags & I40E_FLAG_LEGACY_RX)) { + ret = I40E_RXBUFFER_2048; +#if (PAGE_SIZE < 8192) + } else if (!I40E_2K_TOO_SMALL_WITH_PADDING && + (vsi->netdev->mtu <= ETH_DATA_LEN)) { + ret = I40E_RXBUFFER_1536 - NET_IP_ALIGN; +#endif + } else { + ret = (PAGE_SIZE < 8192) ? I40E_RXBUFFER_3072 : + I40E_RXBUFFER_2048; + } + + return ret; +}
nit: linux coding style states: "Do not unnecessarily use braces where a single statement will do" I think this applies here. Also you could simplify this function to do early returns and drop the u16 ret variable and all the else statements. if (condition 1) return val1; if (condition 2) return val2; return val3; Other than that LGTM.