* [PATCH net-next v1 0/3] mlx4_en: set number of rx/tx channels using ethtool
@ 2012-11-29 19:21 Amir Vadai
2012-11-29 19:21 ` [PATCH net-next v1 1/3] MAINTAINERS: Add Mellanox ethernet driver - mlx4_en Amir Vadai
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Amir Vadai @ 2012-11-29 19:21 UTC (permalink / raw)
To: David S. Miller; +Cc: Amir Vadai, Or Gerlitz, Oren Duer, netdev
1. Added a record in the MAINTAINERS file for the mlx4_en driver
2. Fix set_ringparam not to forget tx moderation info + remove code duplication
3. Add support to changing number of rx/tx channels using ethtool
---
Changes from V0:
- Added file pattern to MAINAINERS file
Amir Vadai (3):
MAINTAINERS: Add Mellanox ethernet driver - mlx4_en
net/mlx4_en: Fix TX moderation info loss after set_ringparam is
called
net/mlx4_en: Set number of rx/tx channels using ethtool
MAINTAINERS | 8 ++
drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 128 +++++++++++++++++-----
drivers/net/ethernet/mellanox/mlx4/en_main.c | 2 +-
drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 26 +++--
drivers/net/ethernet/mellanox/mlx4/en_tx.c | 2 +-
drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 8 ++-
6 files changed, 131 insertions(+), 43 deletions(-)
--
1.7.8.2
^ permalink raw reply [flat|nested] 10+ messages in thread* [PATCH net-next v1 1/3] MAINTAINERS: Add Mellanox ethernet driver - mlx4_en 2012-11-29 19:21 [PATCH net-next v1 0/3] mlx4_en: set number of rx/tx channels using ethtool Amir Vadai @ 2012-11-29 19:21 ` Amir Vadai 2012-11-29 19:21 ` [PATCH net-next v1 2/3] net/mlx4_en: Fix TX moderation info loss after set_ringparam is called Amir Vadai 2012-11-29 19:21 ` [PATCH net-next v1 3/3] net/mlx4_en: Set number of rx/tx channels using ethtool Amir Vadai 2 siblings, 0 replies; 10+ messages in thread From: Amir Vadai @ 2012-11-29 19:21 UTC (permalink / raw) To: David S. Miller Cc: Amir Vadai, Or Gerlitz, Oren Duer, netdev, Yevgeny Petrilin Set mlx4_en maintainer to Amir Vadai instead of Yevgeny Petrilin. Signed-off-by: Amir Vadai <amirv@mellanox.com> Cc: Yevgeny Petrilin <yevgenyp@mellanox.com> --- MAINTAINERS | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index 5d72dd5..3c6e8cd 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4822,6 +4822,14 @@ F: Documentation/scsi/megaraid.txt F: drivers/scsi/megaraid.* F: drivers/scsi/megaraid/ +MELLANOX ETHERNET DRIVER (mlx4_en) +M: Amir Vadai <amirv@mellanox.com> +L: netdev@vger.kernel.org +S: Supported +W: http://www.mellanox.com +Q: http://patchwork.ozlabs.org/project/netdev/list/ +F: drivers/net/ethernet/mellanox/mlx4/en_* + MEMORY MANAGEMENT L: linux-mm@kvack.org W: http://www.linux-mm.org -- 1.7.8.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH net-next v1 2/3] net/mlx4_en: Fix TX moderation info loss after set_ringparam is called 2012-11-29 19:21 [PATCH net-next v1 0/3] mlx4_en: set number of rx/tx channels using ethtool Amir Vadai 2012-11-29 19:21 ` [PATCH net-next v1 1/3] MAINTAINERS: Add Mellanox ethernet driver - mlx4_en Amir Vadai @ 2012-11-29 19:21 ` Amir Vadai 2012-11-29 19:21 ` [PATCH net-next v1 3/3] net/mlx4_en: Set number of rx/tx channels using ethtool Amir Vadai 2 siblings, 0 replies; 10+ messages in thread From: Amir Vadai @ 2012-11-29 19:21 UTC (permalink / raw) To: David S. Miller; +Cc: Amir Vadai, Or Gerlitz, Oren Duer, netdev We need to re-set tx moderation information after calling set_ringparam else default tx moderation will be used. Also avoid related code duplication, by putting it in a utility function. Signed-off-by: Amir Vadai <amirv@mellanox.com> --- drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 59 ++++++++++++----------- 1 files changed, 30 insertions(+), 29 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c index 9d0b88e..dc8ccb4 100644 --- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c +++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c @@ -43,6 +43,34 @@ #define EN_ETHTOOL_SHORT_MASK cpu_to_be16(0xffff) #define EN_ETHTOOL_WORD_MASK cpu_to_be32(0xffffffff) +static int mlx4_en_moderation_update(struct mlx4_en_priv *priv) +{ + int i; + int err = 0; + + for (i = 0; i < priv->tx_ring_num; i++) { + priv->tx_cq[i].moder_cnt = priv->tx_frames; + priv->tx_cq[i].moder_time = priv->tx_usecs; + err = mlx4_en_set_cq_moder(priv, &priv->tx_cq[i]); + if (err) + return err; + } + + if (priv->adaptive_rx_coal) + return 0; + + for (i = 0; i < priv->rx_ring_num; i++) { + priv->rx_cq[i].moder_cnt = priv->rx_frames; + priv->rx_cq[i].moder_time = priv->rx_usecs; + priv->last_moder_time[i] = MLX4_EN_AUTO_CONF; + err = mlx4_en_set_cq_moder(priv, &priv->rx_cq[i]); + if (err) + return err; + } + + return err; +} + static void mlx4_en_get_drvinfo(struct net_device *dev, struct ethtool_drvinfo *drvinfo) { @@ -381,7 +409,6 @@ static int mlx4_en_set_coalesce(struct net_device *dev, struct ethtool_coalesce *coal) { struct mlx4_en_priv *priv = netdev_priv(dev); - int err, i; priv->rx_frames = (coal->rx_max_coalesced_frames == MLX4_EN_AUTO_CONF) ? @@ -397,14 +424,6 @@ static int mlx4_en_set_coalesce(struct net_device *dev, coal->tx_max_coalesced_frames != priv->tx_frames) { priv->tx_usecs = coal->tx_coalesce_usecs; priv->tx_frames = coal->tx_max_coalesced_frames; - for (i = 0; i < priv->tx_ring_num; i++) { - priv->tx_cq[i].moder_cnt = priv->tx_frames; - priv->tx_cq[i].moder_time = priv->tx_usecs; - if (mlx4_en_set_cq_moder(priv, &priv->tx_cq[i])) { - en_warn(priv, "Failed changing moderation " - "for TX cq %d\n", i); - } - } } /* Set adaptive coalescing params */ @@ -414,18 +433,8 @@ static int mlx4_en_set_coalesce(struct net_device *dev, priv->rx_usecs_high = coal->rx_coalesce_usecs_high; priv->sample_interval = coal->rate_sample_interval; priv->adaptive_rx_coal = coal->use_adaptive_rx_coalesce; - if (priv->adaptive_rx_coal) - return 0; - for (i = 0; i < priv->rx_ring_num; i++) { - priv->rx_cq[i].moder_cnt = priv->rx_frames; - priv->rx_cq[i].moder_time = priv->rx_usecs; - priv->last_moder_time[i] = MLX4_EN_AUTO_CONF; - err = mlx4_en_set_cq_moder(priv, &priv->rx_cq[i]); - if (err) - return err; - } - return 0; + return mlx4_en_moderation_update(priv); } static int mlx4_en_set_pauseparam(struct net_device *dev, @@ -466,7 +475,6 @@ static int mlx4_en_set_ringparam(struct net_device *dev, u32 rx_size, tx_size; int port_up = 0; int err = 0; - int i; if (param->rx_jumbo_pending || param->rx_mini_pending) return -EINVAL; @@ -505,14 +513,7 @@ static int mlx4_en_set_ringparam(struct net_device *dev, en_err(priv, "Failed starting port\n"); } - for (i = 0; i < priv->rx_ring_num; i++) { - priv->rx_cq[i].moder_cnt = priv->rx_frames; - priv->rx_cq[i].moder_time = priv->rx_usecs; - priv->last_moder_time[i] = MLX4_EN_AUTO_CONF; - err = mlx4_en_set_cq_moder(priv, &priv->rx_cq[i]); - if (err) - goto out; - } + err = mlx4_en_moderation_update(priv); out: mutex_unlock(&mdev->state_lock); -- 1.7.8.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH net-next v1 3/3] net/mlx4_en: Set number of rx/tx channels using ethtool 2012-11-29 19:21 [PATCH net-next v1 0/3] mlx4_en: set number of rx/tx channels using ethtool Amir Vadai 2012-11-29 19:21 ` [PATCH net-next v1 1/3] MAINTAINERS: Add Mellanox ethernet driver - mlx4_en Amir Vadai 2012-11-29 19:21 ` [PATCH net-next v1 2/3] net/mlx4_en: Fix TX moderation info loss after set_ringparam is called Amir Vadai @ 2012-11-29 19:21 ` Amir Vadai 2012-11-30 17:16 ` David Miller 2012-11-30 23:58 ` Ben Hutchings 2 siblings, 2 replies; 10+ messages in thread From: Amir Vadai @ 2012-11-29 19:21 UTC (permalink / raw) To: David S. Miller; +Cc: Amir Vadai, Or Gerlitz, Oren Duer, netdev Add support to changing number of rx/tx channels using ethtool ('ethtool -[lL]'). Where the number of tx channels specified in ethtool is the number of rings per user priority - not total number of tx rings. Signed-off-by: Amir Vadai <amirv@mellanox.com> --- drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 69 +++++++++++++++++++++++ drivers/net/ethernet/mellanox/mlx4/en_main.c | 2 +- drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 26 +++++---- drivers/net/ethernet/mellanox/mlx4/en_tx.c | 2 +- drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 8 ++- 5 files changed, 93 insertions(+), 14 deletions(-) diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c index dc8ccb4..681bc1b 100644 --- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c +++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c @@ -999,6 +999,73 @@ static int mlx4_en_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd) return err; } +static void mlx4_en_get_channels(struct net_device *dev, + struct ethtool_channels *channel) +{ + struct mlx4_en_priv *priv = netdev_priv(dev); + + memset(channel, 0, sizeof(*channel)); + + channel->max_rx = MAX_RX_RINGS; + channel->max_tx = MLX4_EN_MAX_TX_RING_P_UP; + + channel->rx_count = priv->rx_ring_num; + channel->tx_count = priv->tx_ring_num / MLX4_EN_NUM_UP; +} + +static int mlx4_en_set_channels(struct net_device *dev, + struct ethtool_channels *channel) +{ + struct mlx4_en_priv *priv = netdev_priv(dev); + struct mlx4_en_dev *mdev = priv->mdev; + int port_up; + int err = 0; + + if (channel->other_count || channel->combined_count || + channel->tx_count > channel->max_tx || + channel->rx_count > channel->max_rx || + !channel->tx_count || !channel->rx_count) + return -EINVAL; + + mutex_lock(&mdev->state_lock); + if (priv->port_up) { + port_up = 1; + mlx4_en_stop_port(dev); + } + + mlx4_en_free_resources(priv); + + priv->num_tx_rings_p_up = channel->tx_count; + priv->tx_ring_num = channel->tx_count * MLX4_EN_NUM_UP; + priv->rx_ring_num = channel->rx_count; + + err = mlx4_en_alloc_resources(priv); + if (err) { + en_err(priv, "Failed reallocating port resources\n"); + goto out; + } + + netif_set_real_num_tx_queues(dev, priv->tx_ring_num); + netif_set_real_num_rx_queues(dev, priv->rx_ring_num); + + mlx4_en_setup_tc(dev, MLX4_EN_NUM_UP); + + en_warn(priv, "Using %d TX rings\n", priv->tx_ring_num); + en_warn(priv, "Using %d RX rings\n", priv->rx_ring_num); + + if (port_up) { + err = mlx4_en_start_port(dev); + if (err) + en_err(priv, "Failed starting port\n"); + } + + err = mlx4_en_moderation_update(priv); + +out: + mutex_unlock(&mdev->state_lock); + return err; +} + const struct ethtool_ops mlx4_en_ethtool_ops = { .get_drvinfo = mlx4_en_get_drvinfo, .get_settings = mlx4_en_get_settings, @@ -1023,6 +1090,8 @@ const struct ethtool_ops mlx4_en_ethtool_ops = { .get_rxfh_indir_size = mlx4_en_get_rxfh_indir_size, .get_rxfh_indir = mlx4_en_get_rxfh_indir, .set_rxfh_indir = mlx4_en_set_rxfh_indir, + .get_channels = mlx4_en_get_channels, + .set_channels = mlx4_en_set_channels, }; diff --git a/drivers/net/ethernet/mellanox/mlx4/en_main.c b/drivers/net/ethernet/mellanox/mlx4/en_main.c index a52922e..3a2b8c6 100644 --- a/drivers/net/ethernet/mellanox/mlx4/en_main.c +++ b/drivers/net/ethernet/mellanox/mlx4/en_main.c @@ -250,7 +250,7 @@ static void *mlx4_en_add(struct mlx4_dev *dev) rounddown_pow_of_two(max_t(int, MIN_RX_RINGS, min_t(int, dev->caps.num_comp_vectors, - MAX_RX_RINGS))); + DEF_RX_RINGS))); } else { mdev->profile.prof[i].rx_ring_num = rounddown_pow_of_two( min_t(int, dev->caps.comp_pool/ diff --git a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c index 2b23ca2..7d1287f 100644 --- a/drivers/net/ethernet/mellanox/mlx4/en_netdev.c +++ b/drivers/net/ethernet/mellanox/mlx4/en_netdev.c @@ -47,11 +47,11 @@ #include "mlx4_en.h" #include "en_port.h" -static int mlx4_en_setup_tc(struct net_device *dev, u8 up) +int mlx4_en_setup_tc(struct net_device *dev, u8 up) { struct mlx4_en_priv *priv = netdev_priv(dev); int i; - unsigned int q, offset = 0; + unsigned int offset = 0; if (up && up != MLX4_EN_NUM_UP) return -EINVAL; @@ -59,10 +59,9 @@ static int mlx4_en_setup_tc(struct net_device *dev, u8 up) netdev_set_num_tc(dev, up); /* Partition Tx queues evenly amongst UP's */ - q = priv->tx_ring_num / up; for (i = 0; i < up; i++) { - netdev_set_tc_queue(dev, i, q, offset); - offset += q; + netdev_set_tc_queue(dev, i, priv->num_tx_rings_p_up, offset); + offset += priv->num_tx_rings_p_up; } return 0; @@ -1114,7 +1113,7 @@ int mlx4_en_start_port(struct net_device *dev) /* Configure ring */ tx_ring = &priv->tx_ring[i]; err = mlx4_en_activate_tx_ring(priv, tx_ring, cq->mcq.cqn, - i / priv->mdev->profile.num_tx_rings_p_up); + i / priv->num_tx_rings_p_up); if (err) { en_err(priv, "Failed allocating Tx ring\n"); mlx4_en_deactivate_cq(priv, cq); @@ -1564,10 +1563,13 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port, int err; dev = alloc_etherdev_mqs(sizeof(struct mlx4_en_priv), - prof->tx_ring_num, prof->rx_ring_num); + MAX_TX_RINGS, MAX_RX_RINGS); if (dev == NULL) return -ENOMEM; + netif_set_real_num_tx_queues(dev, prof->tx_ring_num); + netif_set_real_num_rx_queues(dev, prof->rx_ring_num); + SET_NETDEV_DEV(dev, &mdev->dev->pdev->dev); dev->dev_id = port - 1; @@ -1586,15 +1588,17 @@ int mlx4_en_init_netdev(struct mlx4_en_dev *mdev, int port, priv->flags = prof->flags; priv->ctrl_flags = cpu_to_be32(MLX4_WQE_CTRL_CQ_UPDATE | MLX4_WQE_CTRL_SOLICITED); + priv->num_tx_rings_p_up = mdev->profile.num_tx_rings_p_up; priv->tx_ring_num = prof->tx_ring_num; - priv->tx_ring = kzalloc(sizeof(struct mlx4_en_tx_ring) * - priv->tx_ring_num, GFP_KERNEL); + + priv->tx_ring = kzalloc(sizeof(struct mlx4_en_tx_ring) * MAX_TX_RINGS, + GFP_KERNEL); if (!priv->tx_ring) { err = -ENOMEM; goto out; } - priv->tx_cq = kzalloc(sizeof(struct mlx4_en_cq) * priv->tx_ring_num, - GFP_KERNEL); + priv->tx_cq = kzalloc(sizeof(struct mlx4_en_cq) * MAX_RX_RINGS, + GFP_KERNEL); if (!priv->tx_cq) { err = -ENOMEM; goto out; diff --git a/drivers/net/ethernet/mellanox/mlx4/en_tx.c b/drivers/net/ethernet/mellanox/mlx4/en_tx.c index b35094c..1f571d0 100644 --- a/drivers/net/ethernet/mellanox/mlx4/en_tx.c +++ b/drivers/net/ethernet/mellanox/mlx4/en_tx.c @@ -523,7 +523,7 @@ static void build_inline_wqe(struct mlx4_en_tx_desc *tx_desc, struct sk_buff *sk u16 mlx4_en_select_queue(struct net_device *dev, struct sk_buff *skb) { struct mlx4_en_priv *priv = netdev_priv(dev); - u16 rings_p_up = priv->mdev->profile.num_tx_rings_p_up; + u16 rings_p_up = priv->num_tx_rings_p_up; u8 up = 0; if (dev->num_tc) diff --git a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h index d3eba8b..334ec48 100644 --- a/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h +++ b/drivers/net/ethernet/mellanox/mlx4/mlx4_en.h @@ -67,7 +67,8 @@ #define MLX4_EN_PAGE_SHIFT 12 #define MLX4_EN_PAGE_SIZE (1 << MLX4_EN_PAGE_SHIFT) -#define MAX_RX_RINGS 16 +#define DEF_RX_RINGS 16 +#define MAX_RX_RINGS 128 #define MIN_RX_RINGS 4 #define TXBB_SIZE 64 #define HEADROOM (2048 / TXBB_SIZE + 1) @@ -118,6 +119,8 @@ enum { #define MLX4_EN_NUM_UP 8 #define MLX4_EN_DEF_TX_RING_SIZE 512 #define MLX4_EN_DEF_RX_RING_SIZE 1024 +#define MAX_TX_RINGS (MLX4_EN_MAX_TX_RING_P_UP * \ + MLX4_EN_NUM_UP) /* Target number of packets to coalesce with interrupt moderation */ #define MLX4_EN_RX_COAL_TARGET 44 @@ -476,6 +479,7 @@ struct mlx4_en_priv { u32 flags; #define MLX4_EN_FLAG_PROMISC 0x1 #define MLX4_EN_FLAG_MC_PROMISC 0x2 + u8 num_tx_rings_p_up; u32 tx_ring_num; u32 rx_ring_num; u32 rx_skb_size; @@ -596,6 +600,8 @@ int mlx4_en_QUERY_PORT(struct mlx4_en_dev *mdev, u8 port); extern const struct dcbnl_rtnl_ops mlx4_en_dcbnl_ops; #endif +int mlx4_en_setup_tc(struct net_device *dev, u8 up); + #ifdef CONFIG_RFS_ACCEL void mlx4_en_cleanup_filters(struct mlx4_en_priv *priv, struct mlx4_en_rx_ring *rx_ring); -- 1.7.8.2 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v1 3/3] net/mlx4_en: Set number of rx/tx channels using ethtool 2012-11-29 19:21 ` [PATCH net-next v1 3/3] net/mlx4_en: Set number of rx/tx channels using ethtool Amir Vadai @ 2012-11-30 17:16 ` David Miller 2012-11-30 23:58 ` Ben Hutchings 1 sibling, 0 replies; 10+ messages in thread From: David Miller @ 2012-11-30 17:16 UTC (permalink / raw) To: amirv; +Cc: ogerlitz, oren, netdev From: Amir Vadai <amirv@mellanox.com> Date: Thu, 29 Nov 2012 21:21:43 +0200 > +static void mlx4_en_get_channels(struct net_device *dev, > + struct ethtool_channels *channel) This is not formatted correctly, the argument on the second line must line up with the first column after the openning "(" on the previous line. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v1 3/3] net/mlx4_en: Set number of rx/tx channels using ethtool 2012-11-29 19:21 ` [PATCH net-next v1 3/3] net/mlx4_en: Set number of rx/tx channels using ethtool Amir Vadai 2012-11-30 17:16 ` David Miller @ 2012-11-30 23:58 ` Ben Hutchings 2012-12-01 15:50 ` Or Gerlitz 1 sibling, 1 reply; 10+ messages in thread From: Ben Hutchings @ 2012-11-30 23:58 UTC (permalink / raw) To: Amir Vadai; +Cc: David S. Miller, Or Gerlitz, Oren Duer, netdev On Thu, 2012-11-29 at 21:21 +0200, Amir Vadai wrote: > Add support to changing number of rx/tx channels using > ethtool ('ethtool -[lL]'). Where the number of tx channels specified in ethtool > is the number of rings per user priority - not total number of tx rings. > > Signed-off-by: Amir Vadai <amirv@mellanox.com> > --- > drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 69 +++++++++++++++++++++++ > drivers/net/ethernet/mellanox/mlx4/en_main.c | 2 +- > drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 26 +++++---- > drivers/net/ethernet/mellanox/mlx4/en_tx.c | 2 +- > drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 8 ++- > 5 files changed, 93 insertions(+), 14 deletions(-) > > diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c > index dc8ccb4..681bc1b 100644 > --- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c > +++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c > @@ -999,6 +999,73 @@ static int mlx4_en_set_rxnfc(struct net_device *dev, struct ethtool_rxnfc *cmd) [...] > +static int mlx4_en_set_channels(struct net_device *dev, > + struct ethtool_channels *channel) > +{ > + struct mlx4_en_priv *priv = netdev_priv(dev); > + struct mlx4_en_dev *mdev = priv->mdev; > + int port_up; > + int err = 0; > + > + if (channel->other_count || channel->combined_count || > + channel->tx_count > channel->max_tx || > + channel->rx_count > channel->max_rx || The values of max_tx and max_rx are passed in from userland, so you can't trust them. > + !channel->tx_count || !channel->rx_count) > + return -EINVAL; [...] Ben. -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v1 3/3] net/mlx4_en: Set number of rx/tx channels using ethtool 2012-11-30 23:58 ` Ben Hutchings @ 2012-12-01 15:50 ` Or Gerlitz 2012-12-01 16:18 ` Ben Hutchings 2012-12-01 16:25 ` David Miller 0 siblings, 2 replies; 10+ messages in thread From: Or Gerlitz @ 2012-12-01 15:50 UTC (permalink / raw) To: Ben Hutchings; +Cc: Amir Vadai, David S. Miller, Or Gerlitz, Oren Duer, netdev On Sat, Dec 1, 2012 at 1:58 AM, Ben Hutchings <bhutchings@solarflare.com> wrote: > > On Thu, 2012-11-29 at 21:21 +0200, Amir Vadai wrote: > > Add support to changing number of rx/tx channels using > > ethtool ('ethtool -[lL]'). Where the number of tx channels specified in > > ethtool > > is the number of rings per user priority - not total number of tx rings. > > > > Signed-off-by: Amir Vadai <amirv@mellanox.com> > > --- > > drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 69 > > +++++++++++++++++++++++ > > drivers/net/ethernet/mellanox/mlx4/en_main.c | 2 +- > > drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 26 +++++---- > > drivers/net/ethernet/mellanox/mlx4/en_tx.c | 2 +- > > drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 8 ++- > > 5 files changed, 93 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c > > b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c > > index dc8ccb4..681bc1b 100644 > > --- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c > > +++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c > > @@ -999,6 +999,73 @@ static int mlx4_en_set_rxnfc(struct net_device > > *dev, struct ethtool_rxnfc *cmd) > [...] > > +static int mlx4_en_set_channels(struct net_device *dev, > > + struct ethtool_channels *channel) > > +{ > > + struct mlx4_en_priv *priv = netdev_priv(dev); > > + struct mlx4_en_dev *mdev = priv->mdev; > > + int port_up; > > + int err = 0; > > + > > + if (channel->other_count || channel->combined_count || > > + channel->tx_count > channel->max_tx || > > + channel->rx_count > channel->max_rx || > > The values of max_tx and max_rx are passed in from userland, so you > can't trust them. Is this a general statement re ethtool values passed from user space or something specific here? can't one assume that the ethtool process runs under the appropriate admin permission? >> + !channel->tx_count || !channel->rx_count) >> + return -EINVAL; > [...] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v1 3/3] net/mlx4_en: Set number of rx/tx channels using ethtool 2012-12-01 15:50 ` Or Gerlitz @ 2012-12-01 16:18 ` Ben Hutchings 2012-12-01 16:25 ` David Miller 1 sibling, 0 replies; 10+ messages in thread From: Ben Hutchings @ 2012-12-01 16:18 UTC (permalink / raw) To: Or Gerlitz; +Cc: Amir Vadai, David S. Miller, Or Gerlitz, Oren Duer, netdev On Sat, 2012-12-01 at 17:50 +0200, Or Gerlitz wrote: > On Sat, Dec 1, 2012 at 1:58 AM, Ben Hutchings <bhutchings@solarflare.com> wrote: > > > > On Thu, 2012-11-29 at 21:21 +0200, Amir Vadai wrote: > > > Add support to changing number of rx/tx channels using > > > ethtool ('ethtool -[lL]'). Where the number of tx channels specified in > > > ethtool > > > is the number of rings per user priority - not total number of tx rings. > > > > > > Signed-off-by: Amir Vadai <amirv@mellanox.com> > > > --- > > > drivers/net/ethernet/mellanox/mlx4/en_ethtool.c | 69 > > > +++++++++++++++++++++++ > > > drivers/net/ethernet/mellanox/mlx4/en_main.c | 2 +- > > > drivers/net/ethernet/mellanox/mlx4/en_netdev.c | 26 +++++---- > > > drivers/net/ethernet/mellanox/mlx4/en_tx.c | 2 +- > > > drivers/net/ethernet/mellanox/mlx4/mlx4_en.h | 8 ++- > > > 5 files changed, 93 insertions(+), 14 deletions(-) > > > > > > diff --git a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c > > > b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c > > > index dc8ccb4..681bc1b 100644 > > > --- a/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c > > > +++ b/drivers/net/ethernet/mellanox/mlx4/en_ethtool.c > > > @@ -999,6 +999,73 @@ static int mlx4_en_set_rxnfc(struct net_device > > > *dev, struct ethtool_rxnfc *cmd) > > [...] > > > +static int mlx4_en_set_channels(struct net_device *dev, > > > + struct ethtool_channels *channel) > > > +{ > > > + struct mlx4_en_priv *priv = netdev_priv(dev); > > > + struct mlx4_en_dev *mdev = priv->mdev; > > > + int port_up; > > > + int err = 0; > > > + > > > + if (channel->other_count || channel->combined_count || > > > + channel->tx_count > channel->max_tx || > > > + channel->rx_count > channel->max_rx || > > > > The values of max_tx and max_rx are passed in from userland, so you > > can't trust them. > > Is this a general statement re ethtool values passed from user space > or something specific here? General statement. > can't one assume that the ethtool process > runs under the appropriate admin permission? It has CAP_NET_ADMIN, which is not the same thing as CAP_SYS_ADMIN (and even that doesn't necessarily mean ultimate trust). Ben. > >> + !channel->tx_count || !channel->rx_count) > >> + return -EINVAL; > > [...] -- Ben Hutchings, Staff Engineer, Solarflare Not speaking for my employer; that's the marketing department's job. They asked us to note that Solarflare product names are trademarked. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v1 3/3] net/mlx4_en: Set number of rx/tx channels using ethtool 2012-12-01 15:50 ` Or Gerlitz 2012-12-01 16:18 ` Ben Hutchings @ 2012-12-01 16:25 ` David Miller 2012-12-02 12:23 ` Or Gerlitz 1 sibling, 1 reply; 10+ messages in thread From: David Miller @ 2012-12-01 16:25 UTC (permalink / raw) To: or.gerlitz; +Cc: bhutchings, amirv, ogerlitz, oren, netdev From: Or Gerlitz <or.gerlitz@gmail.com> Date: Sat, 1 Dec 2012 17:50:09 +0200 > On Sat, Dec 1, 2012 at 1:58 AM, Ben Hutchings <bhutchings@solarflare.com> wrote: >> >> The values of max_tx and max_rx are passed in from userland, so you >> can't trust them. > > Is this a general statement re ethtool values passed from user space > or something specific here? can't one assume that the ethtool process > runs under the appropriate admin permission? He's saying that you need to carefully validate the arguments, not that appropriate permissions haven't been checked. ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH net-next v1 3/3] net/mlx4_en: Set number of rx/tx channels using ethtool 2012-12-01 16:25 ` David Miller @ 2012-12-02 12:23 ` Or Gerlitz 0 siblings, 0 replies; 10+ messages in thread From: Or Gerlitz @ 2012-12-02 12:23 UTC (permalink / raw) To: David Miller; +Cc: bhutchings, amirv, ogerlitz, oren, netdev On Sat, Dec 1, 2012 at 6:25 PM, David Miller <davem@davemloft.net> wrote: > He's saying that you need to carefully validate the arguments, not > that appropriate permissions haven't been checked. OK, understood. Or. ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-12-02 12:23 UTC | newest] Thread overview: 10+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-11-29 19:21 [PATCH net-next v1 0/3] mlx4_en: set number of rx/tx channels using ethtool Amir Vadai 2012-11-29 19:21 ` [PATCH net-next v1 1/3] MAINTAINERS: Add Mellanox ethernet driver - mlx4_en Amir Vadai 2012-11-29 19:21 ` [PATCH net-next v1 2/3] net/mlx4_en: Fix TX moderation info loss after set_ringparam is called Amir Vadai 2012-11-29 19:21 ` [PATCH net-next v1 3/3] net/mlx4_en: Set number of rx/tx channels using ethtool Amir Vadai 2012-11-30 17:16 ` David Miller 2012-11-30 23:58 ` Ben Hutchings 2012-12-01 15:50 ` Or Gerlitz 2012-12-01 16:18 ` Ben Hutchings 2012-12-01 16:25 ` David Miller 2012-12-02 12:23 ` Or Gerlitz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).