* [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).