<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style> </head> <body dir="ltr"> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> On 5/03/2022 9:05 PM, Ferruh Yigit wrote: > On 4/20/2022 2:46 PM, Walter Heymans wrote: > > The 'max_rx_pktlen' value was previously read from hardware, which was > > set by the running firmware. This caused confusion due to different > > meanings of 'MAX_MTU'. This patch updates the 'max_rx_pktlen' to the > > maximum value that the NFP NIC can support. The 'max_mtu' value that is > > read from hardware, is assigned to the 'dev_info->max_mtu' variable. > > > > If more layer 2 metadata must be used, the firmware can be updated to > > report a smaller 'max_mtu' value. > > > > The constant defined for NFP_FRAME_SIZE_MAX is derived for the maximum > > supported buffer size of 10240, minus 136 bytes that is reserved by the > > hardware and another 56 bytes reserved for expansion in firmware. This > > results in a usable maximum packet length of 10048 bytes. > > > > Signed-off-by: Walter Heymans <walter.heymans@corigine.com> > > Signed-off-by: Niklas Söderlund <niklas.soderlund@corigine.com> > > Reviewed-by: Louis Peens <louis.peens@corigine.com> > > Reviewed-by: Chaoyong He <chaoyong.he@corigine.com> > > Reviewed-by: Richard Donkin <richard.donkin@corigine.com> > > --- > > drivers/net/nfp/nfp_common.c | 11 ++++++++++- > > drivers/net/nfp/nfp_common.h | 3 +++ > > 2 files changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c > > index b26770dbfb..52fbda1a79 100644 > > --- a/drivers/net/nfp/nfp_common.c > > +++ b/drivers/net/nfp/nfp_common.c > > @@ -692,7 +692,16 @@ nfp_net_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) > > dev_info->max_rx_queues = (uint16_t)hw->max_rx_queues; > > dev_info->max_tx_queues = (uint16_t)hw->max_tx_queues; > > dev_info->min_rx_bufsize = RTE_ETHER_MIN_MTU; > > - dev_info->max_rx_pktlen = hw->max_mtu; > > + /* > > + * The maximum rx packet length (max_rx_pktlen) is set to the > > + * maximum supported frame size that the NFP can handle. This > > + * includes layer 2 headers, CRC and other metadata that can > > + * optionally be used. > > + * The maximum layer 3 MTU (max_mtu) is read from hardware, > > + * which was set by the firmware loaded onto the card. > > + */ > > + dev_info->max_rx_pktlen = NFP_FRAME_SIZE_MAX; > > + dev_info->max_mtu = hw->max_mtu; > > Hi Walter, > > ethdev uses 'max_mtu' and 'max_rx_pktlen' to calculate the frame > overhead, like: > 'overhead_len = max_rx_pktlen - max_mtu;' > > In above change 'max_rx_pktlen' is hardcoded but 'max_mtu' is 'hw' > depended, can you confirm 'hw->max_mtu' will always have the frame > overhead as according above calculation?</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> Hi Ferruh,</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> The 'max_rx_pktlen' is hardcoded to the maximum packet length that </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> the hardware can support. The 'hw->max_mtu' is read from hardware,</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> but it is a variable value, depending on the firmware running on the </div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> card. The firmware sets this value to account for the maximum overhead</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> required. To answer you question, yes, this value should always be</div> <div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);"> available. </div> <div id="appendonsend"></div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Ferruh Yigit <ferruh.yigit@xilinx.com> Sent: 03 May 2022 21:04 To: Walter Heymans <walter.heymans@corigine.com>; dev@dpdk.org <dev@dpdk.org> Cc: Niklas Soderlund <niklas.soderlund@corigine.com>; Louis Peens <louis.peens@corigine.com>; Chaoyong He <chaoyong.he@corigine.com>; Richard Donkin <richard.donkin@corigine.com> Subject: Re: [PATCH v2] net/nfp: update how MAX MTU is read <div> </div> </div> <div class="BodyFragment"> <div class="PlainText">On 4/20/2022 2:46 PM, Walter Heymans wrote: > The 'max_rx_pktlen' value was previously read from hardware, which was > set by the running firmware. This caused confusion due to different > meanings of 'MAX_MTU'. This patch updates the 'max_rx_pktlen' to the > maximum value that the NFP NIC can support. The 'max_mtu' value that is > read from hardware, is assigned to the 'dev_info->max_mtu' variable. > > If more layer 2 metadata must be used, the firmware can be updated to > report a smaller 'max_mtu' value. > > The constant defined for NFP_FRAME_SIZE_MAX is derived for the maximum > supported buffer size of 10240, minus 136 bytes that is reserved by the > hardware and another 56 bytes reserved for expansion in firmware. This > results in a usable maximum packet length of 10048 bytes. > > Signed-off-by: Walter Heymans <walter.heymans@corigine.com> > Signed-off-by: Niklas Söderlund <niklas.soderlund@corigine.com> > Reviewed-by: Louis Peens <louis.peens@corigine.com> > Reviewed-by: Chaoyong He <chaoyong.he@corigine.com> > Reviewed-by: Richard Donkin <richard.donkin@corigine.com> > --- > drivers/net/nfp/nfp_common.c | 11 ++++++++++- > drivers/net/nfp/nfp_common.h | 3 +++ > 2 files changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/nfp/nfp_common.c b/drivers/net/nfp/nfp_common.c > index b26770dbfb..52fbda1a79 100644 > --- a/drivers/net/nfp/nfp_common.c > +++ b/drivers/net/nfp/nfp_common.c > @@ -692,7 +692,16 @@ nfp_net_infos_get(struct rte_eth_dev *dev, struct rte_eth_dev_info *dev_info) > dev_info->max_rx_queues = (uint16_t)hw->max_rx_queues; > dev_info->max_tx_queues = (uint16_t)hw->max_tx_queues; > dev_info->min_rx_bufsize = RTE_ETHER_MIN_MTU; > - dev_info->max_rx_pktlen = hw->max_mtu; > + /* > + * The maximum rx packet length (max_rx_pktlen) is set to the > + * maximum supported frame size that the NFP can handle. This > + * includes layer 2 headers, CRC and other metadata that can > + * optionally be used. > + * The maximum layer 3 MTU (max_mtu) is read from hardware, > + * which was set by the firmware loaded onto the card. > + */ > + dev_info->max_rx_pktlen = NFP_FRAME_SIZE_MAX; > + dev_info->max_mtu = hw->max_mtu; Hi Walter, ethdev uses 'max_mtu' and 'max_rx_pktlen' to calculate the frame overhead, like: 'overhead_len = max_rx_pktlen - max_mtu;' In above change 'max_rx_pktlen' is hardcoded but 'max_mtu' is 'hw' depended, can you confirm 'hw->max_mtu' will always have the frame overhead as according above calculation? > /* Next should change when PF support is implemented */ > dev_info->max_mac_addrs = 1; > > diff --git a/drivers/net/nfp/nfp_common.h b/drivers/net/nfp/nfp_common.h > index 8b35fa119c..8db5ec23f8 100644 > --- a/drivers/net/nfp/nfp_common.h > +++ b/drivers/net/nfp/nfp_common.h > @@ -98,6 +98,9 @@ struct nfp_net_adapter; > /* Number of supported physical ports */ > #define NFP_MAX_PHYPORTS 12 > > +/* Maximum supported NFP frame size (MTU + layer 2 headers) */ > +#define NFP_FRAME_SIZE_MAX 10048 > + > #include <linux/types.h> > #include <rte_io.h> > </div> </div> </body> </html>