Re: [PATCH net-next 5/5] net: mvpp2: jumbo frames support
flat view
From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Date: 2018-03-02 16:17:25
Also in:
lkml
Hello, On Fri, 2 Mar 2018 16:40:44 +0100, Antoine Tenart wrote:
quoted hunk ↗ jump to hunk
/* Attach long pool to rxq */@@ -4551,7 +4559,7 @@ mvpp2_bm_pool_use(struct mvpp2_port *port, int pool, int pkt_size) struct mvpp2_bm_pool *new_pool = &port->priv->bm_pools[pool]; int num; - if (pool < MVPP2_BM_SHORT || pool > MVPP2_BM_LONG) { + if (pool < MVPP2_BM_SHORT || pool > MVPP2_BM_JUMBO) {
pool could be an unsigned, which would avoid the need for MVPP2_BM_SHORT. And for the upper limit, you have a convenient MVPP2_BM_POOLS_NUM in your mvpp2_bm_pool_log_num enum, so why not use if ?
quoted hunk ↗ jump to hunk
netdev_err(port->dev, "Invalid pool %d\n", pool); return NULL; }@@ -4596,11 +4604,24 @@ mvpp2_bm_pool_use(struct mvpp2_port *port, int pool, int pkt_size) static int mvpp2_swf_bm_pool_init(struct mvpp2_port *port) { int rxq; + enum mvpp2_bm_pool_log_num long_log_pool, short_log_pool; + + /* If port pkt_size is higher than 1518B: + * HW Long pool - SW Jumbo pool, HW Short pool - SW Short pool
The comment is wrong. In this case, the HW short pool is the SW long
pool.
quoted hunk ↗ jump to hunk
+ * else: HW Long pool - SW Long pool, HW Short pool - SW Short pool + */ + if (port->pkt_size > MVPP2_BM_LONG_PKT_SIZE) { + long_log_pool = MVPP2_BM_JUMBO; + short_log_pool = MVPP2_BM_LONG;
See here.
quoted hunk ↗ jump to hunk
+ } else { + long_log_pool = MVPP2_BM_LONG; + short_log_pool = MVPP2_BM_SHORT; + }
+ /* If port MTU is higher than 1518B: + * HW Long pool - SW Jumbo pool, HW Short pool - SW Short pool
And the comment is wrong here as well :)
quoted hunk ↗ jump to hunk
+ * else: HW Long pool - SW Long pool, HW Short pool - SW Short pool + */ + if (pkt_size > MVPP2_BM_LONG_PKT_SIZE) + new_long_pool = MVPP2_BM_JUMBO; + else + new_long_pool = MVPP2_BM_LONG; + + if (new_long_pool != port->pool_long->id) { + /* Remove port from old short & long pool */ + port->pool_long = mvpp2_bm_pool_use(port, port->pool_long->id, + port->pool_long->pkt_size); + port->pool_long->port_map &= ~(1 << port->id);
BIT(port->id) ?
quoted hunk ↗ jump to hunk
+ port->pool_long = NULL; + + port->pool_short = mvpp2_bm_pool_use(port, port->pool_short->id, + port->pool_short->pkt_size); + port->pool_short->port_map &= ~(1 << port->id);
Ditto.
+ if (port->pool_long->id == MVPP2_BM_JUMBO && port->id != 0) {
Again, all over the place we hardcode the fact that Jumbo frames can
only be used on port 0. I know port 0 is the only one that can do 10G,
but are there possibly some use cases where you may want Jumbo frame on
another port ?
This all really feels very hardcoded to me.
quoted hunk ↗ jump to hunk
+ dev->features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM); + dev->hw_features &= ~(NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM); + } + dev->vlan_features |= features; dev->gso_max_segs = MVPP2_MAX_TSO_SEGS; - /* MTU range: 68 - 9676 */ + /* MTU range: 68 - 9704 */ dev->min_mtu = ETH_MIN_MTU; - /* 9676 == 9700 - 20 and rounding to 8 */ - dev->max_mtu = 9676;
How come we already had a max_mtu of 9676 without Jumbo frame support ?
+ /* 9704 == 9728 - 20 and rounding to 8 */ + dev->max_mtu = MVPP2_BM_JUMBO_PKT_SIZE;
Is this correct for all ports ? Shouldn't the maximum MTU be different between port 0 (that supports Jumbo frames) and the other ports ? Thanks! Thomas -- Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons) Embedded Linux and Kernel engineering http://bootlin.com