* [net-next PATCH] octeontx2-pf: Change receive buffer size using ethtool
@ 2022-01-12 17:02 Subbaraya Sundeep
2022-01-12 19:03 ` Jakub Kicinski
0 siblings, 1 reply; 5+ messages in thread
From: Subbaraya Sundeep @ 2022-01-12 17:02 UTC (permalink / raw)
To: davem, kuba, netdev; +Cc: hkelam, gakula, sgoutham, Subbaraya Sundeep
ethtool rx-buf-len is for setting receive buffer size,
support setting it via ethtool -G parameter and getting
it via ethtool -g parameter.
Signed-off-by: Subbaraya Sundeep <sbhatta@marvell.com>
Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
---
Hi,
This patch is reworked to use ethtool instead
of devlink paramter. Initial patch with devlink
was rejected and Jakub suggested to use ethtool
which was already work in progress by Huawei at
that moment.
https://lore.kernel.org/all/1633454136-14679-1-git-send-email-sbhatta@marvell.com/t/#me83fcb2ce41a2c054370b1ec75172c2f839f57e2
Thanks,
Sundeep
.../net/ethernet/marvell/octeontx2/nic/otx2_common.c | 7 +++++++
.../net/ethernet/marvell/octeontx2/nic/otx2_common.h | 1 +
.../net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c | 18 +++++++++++++++++-
drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c | 5 +++++
drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c | 3 +++
5 files changed, 33 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
index 66da31f..92c0ddb 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.c
@@ -222,8 +222,11 @@ EXPORT_SYMBOL(otx2_set_mac_address);
int otx2_hw_set_mtu(struct otx2_nic *pfvf, int mtu)
{
struct nix_frs_cfg *req;
+ u16 maxlen;
int err;
+ maxlen = otx2_get_max_mtu(pfvf) + OTX2_ETH_HLEN + OTX2_HW_TIMESTAMP_LEN;
+
mutex_lock(&pfvf->mbox.lock);
req = otx2_mbox_alloc_msg_nix_set_hw_frs(&pfvf->mbox);
if (!req) {
@@ -233,6 +236,10 @@ int otx2_hw_set_mtu(struct otx2_nic *pfvf, int mtu)
req->maxlen = pfvf->netdev->mtu + OTX2_ETH_HLEN + OTX2_HW_TIMESTAMP_LEN;
+ /* Use max receive length supported by hardware for loopback devices */
+ if (is_otx2_lbkvf(pfvf->pdev))
+ req->maxlen = maxlen;
+
err = otx2_sync_mbox_msg(&pfvf->mbox);
mutex_unlock(&pfvf->mbox.lock);
return err;
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
index 61e5281..6d11cb2 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
@@ -177,6 +177,7 @@ struct otx2_hw {
u16 pool_cnt;
u16 rqpool_cnt;
u16 sqpool_cnt;
+ u16 rbuf_len;
/* NPA */
u32 stack_pg_ptrs; /* No of ptrs per stack page */
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
index d85db90..a100296 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
@@ -371,6 +371,7 @@ static void otx2_get_ringparam(struct net_device *netdev,
ring->rx_pending = qs->rqe_cnt ? qs->rqe_cnt : Q_COUNT(Q_SIZE_256);
ring->tx_max_pending = Q_COUNT(Q_SIZE_MAX);
ring->tx_pending = qs->sqe_cnt ? qs->sqe_cnt : Q_COUNT(Q_SIZE_4K);
+ kernel_ring->rx_buf_len = pfvf->hw.rbuf_len;
}
static int otx2_set_ringparam(struct net_device *netdev,
@@ -379,6 +380,7 @@ static int otx2_set_ringparam(struct net_device *netdev,
struct netlink_ext_ack *extack)
{
struct otx2_nic *pfvf = netdev_priv(netdev);
+ u32 rx_buf_len = kernel_ring->rx_buf_len;
bool if_up = netif_running(netdev);
struct otx2_qset *qs = &pfvf->qset;
u32 rx_count, tx_count;
@@ -386,6 +388,15 @@ static int otx2_set_ringparam(struct net_device *netdev,
if (ring->rx_mini_pending || ring->rx_jumbo_pending)
return -EINVAL;
+ /* Hardware supports max size of 32k for a receive buffer
+ * and 1536 is typical ethernet frame size.
+ */
+ if (rx_buf_len && (rx_buf_len < 1536 || rx_buf_len > 32768)) {
+ netdev_err(netdev,
+ "Receive buffer range is 1536 - 32768");
+ return -EINVAL;
+ }
+
/* Permitted lengths are 16 64 256 1K 4K 16K 64K 256K 1M */
rx_count = ring->rx_pending;
/* On some silicon variants a skid or reserved CQEs are
@@ -403,7 +414,7 @@ static int otx2_set_ringparam(struct net_device *netdev,
Q_COUNT(Q_SIZE_4K), Q_COUNT(Q_SIZE_MAX));
tx_count = Q_COUNT(Q_SIZE(tx_count, 3));
- if (tx_count == qs->sqe_cnt && rx_count == qs->rqe_cnt)
+ if (tx_count == qs->sqe_cnt && rx_count == qs->rqe_cnt && !rx_buf_len)
return 0;
if (if_up)
@@ -413,6 +424,10 @@ static int otx2_set_ringparam(struct net_device *netdev,
qs->sqe_cnt = tx_count;
qs->rqe_cnt = rx_count;
+ if (rx_buf_len)
+ pfvf->hw.rbuf_len = ALIGN(rx_buf_len, OTX2_ALIGN) +
+ OTX2_HEAD_ROOM;
+
if (if_up)
return netdev->netdev_ops->ndo_open(netdev);
@@ -1207,6 +1222,7 @@ static int otx2_set_link_ksettings(struct net_device *netdev,
static const struct ethtool_ops otx2_ethtool_ops = {
.supported_coalesce_params = ETHTOOL_COALESCE_USECS |
ETHTOOL_COALESCE_MAX_FRAMES,
+ .supported_ring_params = ETHTOOL_RING_USE_RX_BUF_LEN,
.get_link = otx2_get_link,
.get_drvinfo = otx2_get_drvinfo,
.get_strings = otx2_get_strings,
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
index 6080ebd..37afffa 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
@@ -66,6 +66,8 @@ static int otx2_change_mtu(struct net_device *netdev, int new_mtu)
netdev->mtu, new_mtu);
netdev->mtu = new_mtu;
+ pf->hw.rbuf_len = 0;
+
if (if_up)
err = otx2_open(netdev);
@@ -1306,6 +1308,9 @@ static int otx2_get_rbuf_size(struct otx2_nic *pf, int mtu)
int total_size;
int rbuf_size;
+ if (pf->hw.rbuf_len)
+ return pf->hw.rbuf_len;
+
/* The data transferred by NIX to memory consists of actual packet
* plus additional data which has timestamp and/or EDSA/HIGIG2
* headers if interface is configured in corresponding modes.
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
index 925b74e..3fe4c0a 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_vf.c
@@ -438,6 +438,7 @@ static void otx2vf_do_set_rx_mode(struct work_struct *work)
static int otx2vf_change_mtu(struct net_device *netdev, int new_mtu)
{
+ struct otx2_nic *vf = netdev_priv(netdev);
bool if_up = netif_running(netdev);
int err = 0;
@@ -448,6 +449,8 @@ static int otx2vf_change_mtu(struct net_device *netdev, int new_mtu)
netdev->mtu, new_mtu);
netdev->mtu = new_mtu;
+ vf->hw.rbuf_len = 0;
+
if (if_up)
err = otx2vf_open(netdev);
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [net-next PATCH] octeontx2-pf: Change receive buffer size using ethtool
2022-01-12 17:02 [net-next PATCH] octeontx2-pf: Change receive buffer size using ethtool Subbaraya Sundeep
@ 2022-01-12 19:03 ` Jakub Kicinski
2022-01-13 6:37 ` sundeep subbaraya
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2022-01-12 19:03 UTC (permalink / raw)
To: Subbaraya Sundeep; +Cc: davem, netdev, hkelam, gakula, sgoutham
On Wed, 12 Jan 2022 22:32:55 +0530 Subbaraya Sundeep wrote:
> ethtool rx-buf-len is for setting receive buffer size,
> support setting it via ethtool -G parameter and getting
> it via ethtool -g parameter.
I don't see a check against current MTU, in case MTU is larger than
the buffer length the device will scatter?
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> index 61e5281..6d11cb2 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> @@ -177,6 +177,7 @@ struct otx2_hw {
> u16 pool_cnt;
> u16 rqpool_cnt;
> u16 sqpool_cnt;
> + u16 rbuf_len;
>
> /* NPA */
> u32 stack_pg_ptrs; /* No of ptrs per stack page */
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> index d85db90..a100296 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> @@ -371,6 +371,7 @@ static void otx2_get_ringparam(struct net_device *netdev,
> ring->rx_pending = qs->rqe_cnt ? qs->rqe_cnt : Q_COUNT(Q_SIZE_256);
> ring->tx_max_pending = Q_COUNT(Q_SIZE_MAX);
> ring->tx_pending = qs->sqe_cnt ? qs->sqe_cnt : Q_COUNT(Q_SIZE_4K);
> + kernel_ring->rx_buf_len = pfvf->hw.rbuf_len;
> }
>
> static int otx2_set_ringparam(struct net_device *netdev,
> @@ -379,6 +380,7 @@ static int otx2_set_ringparam(struct net_device *netdev,
> struct netlink_ext_ack *extack)
> {
> struct otx2_nic *pfvf = netdev_priv(netdev);
> + u32 rx_buf_len = kernel_ring->rx_buf_len;
> bool if_up = netif_running(netdev);
> struct otx2_qset *qs = &pfvf->qset;
> u32 rx_count, tx_count;
> @@ -386,6 +388,15 @@ static int otx2_set_ringparam(struct net_device *netdev,
> if (ring->rx_mini_pending || ring->rx_jumbo_pending)
> return -EINVAL;
>
> + /* Hardware supports max size of 32k for a receive buffer
> + * and 1536 is typical ethernet frame size.
> + */
> + if (rx_buf_len && (rx_buf_len < 1536 || rx_buf_len > 32768)) {
> + netdev_err(netdev,
> + "Receive buffer range is 1536 - 32768");
> + return -EINVAL;
> + }
> +
> /* Permitted lengths are 16 64 256 1K 4K 16K 64K 256K 1M */
> rx_count = ring->rx_pending;
> /* On some silicon variants a skid or reserved CQEs are
> @@ -403,7 +414,7 @@ static int otx2_set_ringparam(struct net_device *netdev,
> Q_COUNT(Q_SIZE_4K), Q_COUNT(Q_SIZE_MAX));
> tx_count = Q_COUNT(Q_SIZE(tx_count, 3));
>
> - if (tx_count == qs->sqe_cnt && rx_count == qs->rqe_cnt)
> + if (tx_count == qs->sqe_cnt && rx_count == qs->rqe_cnt && !rx_buf_len)
Should we use rx_buf_len = 0 as a way for users to reset the rxbuf len
to the default value? I think that would be handy.
> if (if_up)
> @@ -413,6 +424,10 @@ static int otx2_set_ringparam(struct net_device *netdev,
> qs->sqe_cnt = tx_count;
> qs->rqe_cnt = rx_count;
>
> + if (rx_buf_len)
> + pfvf->hw.rbuf_len = ALIGN(rx_buf_len, OTX2_ALIGN) +
> + OTX2_HEAD_ROOM;
> +
> if (if_up)
> return netdev->netdev_ops->ndo_open(netdev);
>
> @@ -1207,6 +1222,7 @@ static int otx2_set_link_ksettings(struct net_device *netdev,
> static const struct ethtool_ops otx2_ethtool_ops = {
> .supported_coalesce_params = ETHTOOL_COALESCE_USECS |
> ETHTOOL_COALESCE_MAX_FRAMES,
> + .supported_ring_params = ETHTOOL_RING_USE_RX_BUF_LEN,
> .get_link = otx2_get_link,
> .get_drvinfo = otx2_get_drvinfo,
> .get_strings = otx2_get_strings,
> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> index 6080ebd..37afffa 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> @@ -66,6 +66,8 @@ static int otx2_change_mtu(struct net_device *netdev, int new_mtu)
> netdev->mtu, new_mtu);
> netdev->mtu = new_mtu;
>
> + pf->hw.rbuf_len = 0;
Why reset the buf len on mtu change?
> if (if_up)
> err = otx2_open(netdev);
>
> @@ -1306,6 +1308,9 @@ static int otx2_get_rbuf_size(struct otx2_nic *pf, int mtu)
> int total_size;
> int rbuf_size;
>
> + if (pf->hw.rbuf_len)
> + return pf->hw.rbuf_len;
> +
> /* The data transferred by NIX to memory consists of actual packet
> * plus additional data which has timestamp and/or EDSA/HIGIG2
> * headers if interface is configured in corresponding modes.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [net-next PATCH] octeontx2-pf: Change receive buffer size using ethtool
2022-01-12 19:03 ` Jakub Kicinski
@ 2022-01-13 6:37 ` sundeep subbaraya
2022-01-13 18:31 ` Jakub Kicinski
0 siblings, 1 reply; 5+ messages in thread
From: sundeep subbaraya @ 2022-01-13 6:37 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Subbaraya Sundeep, David Miller, netdev, hariprasad,
Geetha sowjanya, Sunil Kovvuri Goutham
Hi Jakub,
On Thu, Jan 13, 2022 at 5:24 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Wed, 12 Jan 2022 22:32:55 +0530 Subbaraya Sundeep wrote:
> > ethtool rx-buf-len is for setting receive buffer size,
> > support setting it via ethtool -G parameter and getting
> > it via ethtool -g parameter.
>
> I don't see a check against current MTU, in case MTU is larger than
> the buffer length the device will scatter?
>
Yes correct. The idea is to have an option for user to choose either one big
frame or multiple fragments for a frame.
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> > index 61e5281..6d11cb2 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_common.h
> > @@ -177,6 +177,7 @@ struct otx2_hw {
> > u16 pool_cnt;
> > u16 rqpool_cnt;
> > u16 sqpool_cnt;
> > + u16 rbuf_len;
> >
> > /* NPA */
> > u32 stack_pg_ptrs; /* No of ptrs per stack page */
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> > index d85db90..a100296 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_ethtool.c
> > @@ -371,6 +371,7 @@ static void otx2_get_ringparam(struct net_device *netdev,
> > ring->rx_pending = qs->rqe_cnt ? qs->rqe_cnt : Q_COUNT(Q_SIZE_256);
> > ring->tx_max_pending = Q_COUNT(Q_SIZE_MAX);
> > ring->tx_pending = qs->sqe_cnt ? qs->sqe_cnt : Q_COUNT(Q_SIZE_4K);
> > + kernel_ring->rx_buf_len = pfvf->hw.rbuf_len;
> > }
> >
> > static int otx2_set_ringparam(struct net_device *netdev,
> > @@ -379,6 +380,7 @@ static int otx2_set_ringparam(struct net_device *netdev,
> > struct netlink_ext_ack *extack)
> > {
> > struct otx2_nic *pfvf = netdev_priv(netdev);
> > + u32 rx_buf_len = kernel_ring->rx_buf_len;
> > bool if_up = netif_running(netdev);
> > struct otx2_qset *qs = &pfvf->qset;
> > u32 rx_count, tx_count;
> > @@ -386,6 +388,15 @@ static int otx2_set_ringparam(struct net_device *netdev,
> > if (ring->rx_mini_pending || ring->rx_jumbo_pending)
> > return -EINVAL;
> >
> > + /* Hardware supports max size of 32k for a receive buffer
> > + * and 1536 is typical ethernet frame size.
> > + */
> > + if (rx_buf_len && (rx_buf_len < 1536 || rx_buf_len > 32768)) {
> > + netdev_err(netdev,
> > + "Receive buffer range is 1536 - 32768");
> > + return -EINVAL;
> > + }
> > +
> > /* Permitted lengths are 16 64 256 1K 4K 16K 64K 256K 1M */
> > rx_count = ring->rx_pending;
> > /* On some silicon variants a skid or reserved CQEs are
> > @@ -403,7 +414,7 @@ static int otx2_set_ringparam(struct net_device *netdev,
> > Q_COUNT(Q_SIZE_4K), Q_COUNT(Q_SIZE_MAX));
> > tx_count = Q_COUNT(Q_SIZE(tx_count, 3));
> >
> > - if (tx_count == qs->sqe_cnt && rx_count == qs->rqe_cnt)
> > + if (tx_count == qs->sqe_cnt && rx_count == qs->rqe_cnt && !rx_buf_len)
>
> Should we use rx_buf_len = 0 as a way for users to reset the rxbuf len
> to the default value? I think that would be handy.
>
Before this patch we calculate each receive buffer based on mtu set by user.
Now user can use rx-buf-len but the old mtu based calculation is also there.
Here I am using rx_buf_len == 0 as a switch to calculate buffer length
using mtu or
just use length set by user. So here I am not setting rx_buf_len to some
default value.
> > if (if_up)
> > @@ -413,6 +424,10 @@ static int otx2_set_ringparam(struct net_device *netdev,
> > qs->sqe_cnt = tx_count;
> > qs->rqe_cnt = rx_count;
> >
> > + if (rx_buf_len)
> > + pfvf->hw.rbuf_len = ALIGN(rx_buf_len, OTX2_ALIGN) +
> > + OTX2_HEAD_ROOM;
> > +
> > if (if_up)
> > return netdev->netdev_ops->ndo_open(netdev);
> >
> > @@ -1207,6 +1222,7 @@ static int otx2_set_link_ksettings(struct net_device *netdev,
> > static const struct ethtool_ops otx2_ethtool_ops = {
> > .supported_coalesce_params = ETHTOOL_COALESCE_USECS |
> > ETHTOOL_COALESCE_MAX_FRAMES,
> > + .supported_ring_params = ETHTOOL_RING_USE_RX_BUF_LEN,
> > .get_link = otx2_get_link,
> > .get_drvinfo = otx2_get_drvinfo,
> > .get_strings = otx2_get_strings,
> > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> > index 6080ebd..37afffa 100644
> > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> > @@ -66,6 +66,8 @@ static int otx2_change_mtu(struct net_device *netdev, int new_mtu)
> > netdev->mtu, new_mtu);
> > netdev->mtu = new_mtu;
> >
> > + pf->hw.rbuf_len = 0;
>
> Why reset the buf len on mtu change?
>
As explained above buffer size will be calculated using mtu
now instead of rx-buf-len from ethtool.
Thanks,
Sundeep
> > if (if_up)
> > err = otx2_open(netdev);
> >
> > @@ -1306,6 +1308,9 @@ static int otx2_get_rbuf_size(struct otx2_nic *pf, int mtu)
> > int total_size;
> > int rbuf_size;
> >
> > + if (pf->hw.rbuf_len)
> > + return pf->hw.rbuf_len;
> > +
> > /* The data transferred by NIX to memory consists of actual packet
> > * plus additional data which has timestamp and/or EDSA/HIGIG2
> > * headers if interface is configured in corresponding modes.
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [net-next PATCH] octeontx2-pf: Change receive buffer size using ethtool
2022-01-13 6:37 ` sundeep subbaraya
@ 2022-01-13 18:31 ` Jakub Kicinski
2022-01-14 5:58 ` sundeep subbaraya
0 siblings, 1 reply; 5+ messages in thread
From: Jakub Kicinski @ 2022-01-13 18:31 UTC (permalink / raw)
To: sundeep subbaraya
Cc: Subbaraya Sundeep, David Miller, netdev, hariprasad,
Geetha sowjanya, Sunil Kovvuri Goutham
On Thu, 13 Jan 2022 12:07:42 +0530 sundeep subbaraya wrote:
> > Should we use rx_buf_len = 0 as a way for users to reset the rxbuf len
> > to the default value? I think that would be handy.
> >
> Before this patch we calculate each receive buffer based on mtu set by user.
> Now user can use rx-buf-len but the old mtu based calculation is also there.
> Here I am using rx_buf_len == 0 as a switch to calculate buffer length
> using mtu or
> just use length set by user. So here I am not setting rx_buf_len to some
> default value.
>
> > > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> > > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> > > @@ -66,6 +66,8 @@ static int otx2_change_mtu(struct net_device *netdev, int new_mtu)
> > > netdev->mtu, new_mtu);
> > > netdev->mtu = new_mtu;
> > >
> > > + pf->hw.rbuf_len = 0;
> >
> > Why reset the buf len on mtu change?
> >
> As explained above buffer size will be calculated using mtu
> now instead of rx-buf-len from ethtool.
IIUC you're saying that the way to get back to the default buffer size
is to change the MTU?
It was discussed on the list in the past in the context of RSS
indirection tables and the conclusion was that we should not reset
explicit user configuration (in case of RSS indir table this means
user-set table survives change in the number of queues).
Having one config reset another is pretty painful to deal with for
the users. I had to fix many cases of this sort of problem in the
production env I'm working with. Turning one knob resets other knobs
so it takes multiple runs of the config daemon (chef or alike) to
get to the target configuration.
In my mind if user has set the rx-buf-len it should survive all other
config changes. User can reset to default by setting rx-buf-len to 0.
It should be possible to set rx-buf-len and mtu in any order and have
the same result.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [net-next PATCH] octeontx2-pf: Change receive buffer size using ethtool
2022-01-13 18:31 ` Jakub Kicinski
@ 2022-01-14 5:58 ` sundeep subbaraya
0 siblings, 0 replies; 5+ messages in thread
From: sundeep subbaraya @ 2022-01-14 5:58 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Subbaraya Sundeep, David Miller, netdev, hariprasad,
Geetha sowjanya, Sunil Kovvuri Goutham
Hi Jakub,
On Fri, Jan 14, 2022 at 12:01 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Thu, 13 Jan 2022 12:07:42 +0530 sundeep subbaraya wrote:
> > > Should we use rx_buf_len = 0 as a way for users to reset the rxbuf len
> > > to the default value? I think that would be handy.
> > >
> > Before this patch we calculate each receive buffer based on mtu set by user.
> > Now user can use rx-buf-len but the old mtu based calculation is also there.
> > Here I am using rx_buf_len == 0 as a switch to calculate buffer length
> > using mtu or
> > just use length set by user. So here I am not setting rx_buf_len to some
> > default value.
> >
> > > > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> > > > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_pf.c
> > > > @@ -66,6 +66,8 @@ static int otx2_change_mtu(struct net_device *netdev, int new_mtu)
> > > > netdev->mtu, new_mtu);
> > > > netdev->mtu = new_mtu;
> > > >
> > > > + pf->hw.rbuf_len = 0;
> > >
> > > Why reset the buf len on mtu change?
> > >
> > As explained above buffer size will be calculated using mtu
> > now instead of rx-buf-len from ethtool.
>
> IIUC you're saying that the way to get back to the default buffer size
> is to change the MTU?
>
> It was discussed on the list in the past in the context of RSS
> indirection tables and the conclusion was that we should not reset
> explicit user configuration (in case of RSS indir table this means
> user-set table survives change in the number of queues).
>
> Having one config reset another is pretty painful to deal with for
> the users. I had to fix many cases of this sort of problem in the
> production env I'm working with. Turning one knob resets other knobs
> so it takes multiple runs of the config daemon (chef or alike) to
> get to the target configuration.
>
> In my mind if user has set the rx-buf-len it should survive all other
> config changes. User can reset to default by setting rx-buf-len to 0.
> It should be possible to set rx-buf-len and mtu in any order and have
> the same result.
Thanks for the clear explanation. I will change as per your comments
and send the next spin.
Sundeep
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-14 5:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-12 17:02 [net-next PATCH] octeontx2-pf: Change receive buffer size using ethtool Subbaraya Sundeep
2022-01-12 19:03 ` Jakub Kicinski
2022-01-13 6:37 ` sundeep subbaraya
2022-01-13 18:31 ` Jakub Kicinski
2022-01-14 5:58 ` sundeep subbaraya
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.