* [RFC PATCH net-next 3/5] ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set
From: Vlad Zolotarov @ 2014-12-30 16:30 UTC (permalink / raw)
To: netdev; +Cc: gleb, avi, Vlad Zolotarov
In-Reply-To: <1419957035-1078-1-git-send-email-vladz@cloudius-systems.com>
82599 VFs and PF share the same RSS Key. Therefore we will return
the same RSS key for all VFs.
Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 2 ++
drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 25 +++++++++++++++++++++++++
2 files changed, 27 insertions(+)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
index c1123d9..52e775b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
@@ -97,6 +97,8 @@ enum ixgbe_pfvf_api_rev {
#define IXGBE_VF_GET_RETA_1 0x0b /* get RETA[12..23] */
#define IXGBE_VF_GET_RETA_2 0x0c /* get RETA[24..31] */
+#define IXGBE_VF_GET_RSS_KEY 0x0d /* get RSS key */
+
/* GET_QUEUES return data indices within the mailbox */
#define IXGBE_VF_TX_QUEUES 1 /* number of Tx queues supported */
#define IXGBE_VF_RX_QUEUES 2 /* number of Rx queues supported */
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
index 84db1a5..fc8233e 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
@@ -970,6 +970,28 @@ static int ixgbe_get_vf_reta(struct ixgbe_adapter *adapter,
return 0;
}
+static int ixgbe_get_vf_rss_key(struct ixgbe_adapter *adapter,
+ u32 *msgbuf, u32 vf)
+{
+ struct ixgbe_hw *hw = &adapter->hw;
+ int i;
+ u32 *rss_key = &msgbuf[1];
+
+ /* verify the PF is supporting the correct API */
+ switch (adapter->vfinfo[vf].vf_api) {
+ case ixgbe_mbox_api_12:
+ break;
+ default:
+ return -EPERM;
+ }
+
+ /* Read the RSS KEY */
+ for (i = 0; i < 10; i++)
+ rss_key[i] = IXGBE_READ_REG(hw, IXGBE_RSSRK(i));
+
+ return 0;
+}
+
static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
{
u32 mbx_size = IXGBE_VFMAILBOX_SIZE;
@@ -1035,6 +1057,9 @@ static int ixgbe_rcv_msg_from_vf(struct ixgbe_adapter *adapter, u32 vf)
case IXGBE_VF_GET_RETA_2:
retval = ixgbe_get_vf_reta(adapter, msgbuf, vf, 24, 8);
break;
+ case IXGBE_VF_GET_RSS_KEY:
+ retval = ixgbe_get_vf_rss_key(adapter, msgbuf, vf);
+ break;
default:
e_err(drv, "Unhandled Msg %8.8x\n", msgbuf[0]);
retval = IXGBE_ERR_MBX;
--
2.1.0
^ permalink raw reply related
* [RFC PATCH net-next 5/5] ixgbevf: Add the appropriate ethtool ops to query RSS indirection table and key
From: Vlad Zolotarov @ 2014-12-30 16:30 UTC (permalink / raw)
To: netdev; +Cc: gleb, avi, Vlad Zolotarov
In-Reply-To: <1419957035-1078-1-git-send-email-vladz@cloudius-systems.com>
Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
---
drivers/net/ethernet/intel/ixgbevf/ethtool.c | 37 ++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/drivers/net/ethernet/intel/ixgbevf/ethtool.c b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
index cc0e5b7..255bbc8 100644
--- a/drivers/net/ethernet/intel/ixgbevf/ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbevf/ethtool.c
@@ -792,6 +792,40 @@ static int ixgbevf_set_coalesce(struct net_device *netdev,
return 0;
}
+static u32 ixgbevf_get_rxfh_indir_size(struct net_device *netdev)
+{
+ return 128;
+}
+
+static u32 ixgbevf_get_rxfh_key_size(struct net_device *netdev)
+{
+ return 40;
+}
+
+static int ixgbevf_get_rxfh(struct net_device *netdev, u32 *indir, u8 *key,
+ u8 *hfunc)
+{
+ struct ixgbevf_adapter *adapter = netdev_priv(netdev);
+ int err;
+
+ if (hfunc)
+ *hfunc = ETH_RSS_HASH_TOP;
+
+ if (indir) {
+ err = ixgbevf_get_reta(&adapter->hw, indir);
+ if (err)
+ return err;
+ }
+
+ if (key) {
+ err = ixgbevf_get_rss_key(&adapter->hw, key);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
static const struct ethtool_ops ixgbevf_ethtool_ops = {
.get_settings = ixgbevf_get_settings,
.get_drvinfo = ixgbevf_get_drvinfo,
@@ -809,6 +843,9 @@ static const struct ethtool_ops ixgbevf_ethtool_ops = {
.get_ethtool_stats = ixgbevf_get_ethtool_stats,
.get_coalesce = ixgbevf_get_coalesce,
.set_coalesce = ixgbevf_set_coalesce,
+ .get_rxfh_indir_size = ixgbevf_get_rxfh_indir_size,
+ .get_rxfh_key_size = ixgbevf_get_rxfh_key_size,
+ .get_rxfh = ixgbevf_get_rxfh,
};
void ixgbevf_set_ethtool_ops(struct net_device *netdev)
--
2.1.0
^ permalink raw reply related
* Re: [PATCH 0/4] ath10k: a few incorrect return handling fix-up
From: Sergei Shtylyov @ 2014-12-30 17:18 UTC (permalink / raw)
To: Nicholas Mc Guire, Kalle Valo
Cc: Michal Kazior, Ben Greear, Chun-Yeow Yeoh, Yanbo Li, ath10k,
linux-wireless, netdev, linux-kernel
In-Reply-To: <1419942046-17985-1-git-send-email-der.herr@hofr.at>
Hello.
On 12/30/2014 03:20 PM, Nicholas Mc Guire wrote:
> wait_for_completion_timeout does not return negative values so the tests
> for <= 0 are not needed and the case differentiation in the error handling
> path unnecessary.
I decided to verify your statement and I saw that it seems wrong.
do_wait_for_common() can return -ERESTARTSYS and the return value gets
returned by its callers unchanged.
> patch was only compile tested x86_64_defconfig + CONFIG_ATH_CARDS=m
> CONFIG_ATH10K=m
> patch is against linux-next 3.19.0-rc1 -next-20141226
Rather patches. It would have been better to send one patch instead of 4
patches with the same name.
WBR, Sergei
^ permalink raw reply
* [PATCH v2][Nios2-dev] Altera TSE: Add missing phydev
From: Kostya Belezko @ 2014-12-30 17:27 UTC (permalink / raw)
To: Nios2-dev; +Cc: netdev, Kostya Belezko
Altera network device doesn't come up after
ifconfig eth0 down
ifconfig eth0 up
The reason behind is clearing priv->phydev during tse_shutdown().
The phydev is not restored back at tse_open().
Resubmiting as to follow Tobias Klauser suggestion.
phy_start/phy_stop are called on each ifup/ifdown and
phy_disconnect is called once during the module removal.
Signed-off-by: Kostya Belezko <bkostya@hotmail.com>
---
drivers/net/ethernet/altera/altera_tse_main.c | 15 ++++++---------
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/net/ethernet/altera/altera_tse_main.c b/drivers/net/ethernet/altera/altera_tse_main.c
index 4efc435..720db5f 100644
--- a/drivers/net/ethernet/altera/altera_tse_main.c
+++ b/drivers/net/ethernet/altera/altera_tse_main.c
@@ -1170,10 +1170,6 @@ tx_request_irq_error:
init_error:
free_skbufs(dev);
alloc_skbuf_error:
- if (priv->phydev) {
- phy_disconnect(priv->phydev);
- priv->phydev = NULL;
- }
phy_error:
return ret;
}
@@ -1186,12 +1182,9 @@ static int tse_shutdown(struct net_device *dev)
int ret;
unsigned long int flags;
- /* Stop and disconnect the PHY */
- if (priv->phydev) {
+ /* Stop the PHY */
+ if (priv->phydev)
phy_stop(priv->phydev);
- phy_disconnect(priv->phydev);
- priv->phydev = NULL;
- }
netif_stop_queue(dev);
napi_disable(&priv->napi);
@@ -1525,6 +1518,10 @@ err_free_netdev:
static int altera_tse_remove(struct platform_device *pdev)
{
struct net_device *ndev = platform_get_drvdata(pdev);
+ struct altera_tse_private *priv = netdev_priv(ndev);
+
+ if (priv->phydev)
+ phy_disconnect(priv->phydev);
platform_set_drvdata(pdev, NULL);
altera_tse_mdio_destroy(ndev);
--
2.1.0
^ permalink raw reply related
* *****SPAM***** RE
From: SK @ 2014-12-30 16:18 UTC (permalink / raw)
To: Recipients
This is to request your assistance to execute a lucrative project in Asia. Kindly respond.
Sk
^ permalink raw reply
* Re: [RFC PATCH net-next 0/5]: ixgbevf: Allow querying VFs RSS indirection table and key
From: Jeff Kirsher @ 2014-12-30 18:16 UTC (permalink / raw)
To: Vlad Zolotarov; +Cc: netdev, gleb, avi
In-Reply-To: <1419957035-1078-1-git-send-email-vladz@cloudius-systems.com>
On Tue, Dec 30, 2014 at 8:30 AM, Vlad Zolotarov
<vladz@cloudius-systems.com> wrote:
> Add the ethtool ops to VF driver to allow querying the RSS indirection table
> and RSS Random Key.
>
> - PF driver: Add new VF-PF channel commands.
> - VF driver: Utilize these new commands and add the corresponding
> ethtool callbacks.
>
> Vlad Zolotarov (5):
> ixgbe: Add a RETA query command to VF-PF channel API
> ixgbevf: Add a RETA query code
> ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set
> ixgbevf: Add RSS Key query code
> ixgbevf: Add the appropriate ethtool ops to query RSS indirection
> table and key
>
> drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 8 ++
> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 60 +++++++++++
> drivers/net/ethernet/intel/ixgbevf/ethtool.c | 37 +++++++
> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 +-
> drivers/net/ethernet/intel/ixgbevf/mbx.h | 8 ++
> drivers/net/ethernet/intel/ixgbevf/vf.c | 125 ++++++++++++++++++++++
> drivers/net/ethernet/intel/ixgbevf/vf.h | 2 +
> 7 files changed, 243 insertions(+), 1 deletion(-)
Remember to CC on patches against drivers in drivers/net/ethernet/intel/ please.
--
Cheers,
Jeff
^ permalink raw reply
* Clarification regarding IFLA_BRPORT_LEARNING_SYNC and aging of fdb entries learnt via br_fdb_external_learn_add()
From: Siva Mannem @ 2014-12-30 18:20 UTC (permalink / raw)
To: netdev
Hi,
I am trying to understand the ongoing switch device offload effort and
am following the discussions. I have a question regarding
IFLA_BRPORT_LEARNING_SYNC flag and how aging happens when this flag is
enabled on a port that is attached to a bridge that has vlan filtering
enabled.
If I understand correctly, when IFLA_BRPORT_LEARNING_SYNC is set on a
bridge port, fdb entries that are learnt externally(may be learnt by
hardware and driver is notified) are synced to bridges fdb using
br_fdb_external_learn_add(). The fdb
entries(fdb->added_by_external_learn set to true) that are learnt via
this method are also deleted by the aging logic after the aging time
even though L2 data forwadring happens in hardware. Is there a way
where aging can be disabled for these entries? and let the entries be
removed only via br_fdb_external_learn_delete()? or am I missing
something?
Regards,
Siva.
^ permalink raw reply
* Re: [PATCH 0/4] ath10k: a few incorrect return handling fix-up
From: Nicholas Mc Guire @ 2014-12-30 18:28 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Kalle Valo, Michal Kazior, Ben Greear, Chun-Yeow Yeoh, Yanbo Li,
ath10k, linux-wireless, netdev, linux-kernel
In-Reply-To: <54A2DE7C.1050602@cogentembedded.com>
On Tue, 30 Dec 2014, Sergei Shtylyov wrote:
> Hello.
>
> On 12/30/2014 03:20 PM, Nicholas Mc Guire wrote:
>
>> wait_for_completion_timeout does not return negative values so the tests
>> for <= 0 are not needed and the case differentiation in the error handling
>> path unnecessary.
>
> I decided to verify your statement and I saw that it seems wrong.
> do_wait_for_common() can return -ERESTARTSYS and the return value gets
> returned by its callers unchanged.
the -ERESTARTSYS only can be returned if state matches but
wait_for_completion_timemout passes TASK_UNINTERRUPTIBLE
so signal_pending_state will return 0 and never negativ
my understanding of the callchain is:
wait_for_completion_timemout with TASK_UNINTERRUPTIBLE
-> wait_for_common(...TASK_UNINTERRUPTIBLE)
-> __wait_for_common(...TASK_UNINTERRUPTIBLE)
-> do_wait_for_common(...TASK_UNINTERRUPTIBLE)
-> signal_pending_state(TASK_UNINTERRUPTIBLE...)
static inline int signal_pending_state(long state, struct task_struct *p)
{
if (!(state & (TASK_INTERRUPTIBLE | TASK_WAKEKILL)))
return 0;
so wait_for_completion_timemout should return 0 or 1 only
>
>> patch was only compile tested x86_64_defconfig + CONFIG_ATH_CARDS=m
>> CONFIG_ATH10K=m
>
>> patch is against linux-next 3.19.0-rc1 -next-20141226
>
> Rather patches. It would have been better to send one patch instead of
> 4 patches with the same name.
>
sorry for that - I had split it into separate patches as it was
in different files - giving them the same name of course was a bit
brain-dead.
please do give it one more look - if the above argument is invalid
I apologize for the noise.
thx!
hofrat
^ permalink raw reply
* Re: [PATCH 0/4] ath10k: a few incorrect return handling fix-up
From: Sergei Shtylyov @ 2014-12-30 18:39 UTC (permalink / raw)
To: Nicholas Mc Guire
Cc: Kalle Valo, Michal Kazior, Ben Greear, Chun-Yeow Yeoh, Yanbo Li,
ath10k, linux-wireless, netdev, linux-kernel
In-Reply-To: <20141230182842.GA18361@opentech.at>
On 12/30/2014 09:28 PM, Nicholas Mc Guire wrote:
>>> wait_for_completion_timeout does not return negative values so the tests
>>> for <= 0 are not needed and the case differentiation in the error handling
>>> path unnecessary.
>> I decided to verify your statement and I saw that it seems wrong.
>> do_wait_for_common() can return -ERESTARTSYS and the return value gets
>> returned by its callers unchanged.
> the -ERESTARTSYS only can be returned if state matches but
> wait_for_completion_timemout passes TASK_UNINTERRUPTIBLE
> so signal_pending_state will return 0 and never negativ
> my understanding of the callchain is:
> wait_for_completion_timemout with TASK_UNINTERRUPTIBLE
> -> wait_for_common(...TASK_UNINTERRUPTIBLE)
> -> __wait_for_common(...TASK_UNINTERRUPTIBLE)
> -> do_wait_for_common(...TASK_UNINTERRUPTIBLE)
> -> signal_pending_state(TASK_UNINTERRUPTIBLE...)
> static inline int signal_pending_state(long state, struct task_struct *p)
> {
> if (!(state & (TASK_INTERRUPTIBLE | TASK_WAKEKILL)))
> return 0;
Right. I didn't look into TASK_UNINTERRUPTIBLE thing before sending my mail.
> so wait_for_completion_timemout should return 0 or 1 only
0 or the remaining time, to be precise.
>>> patch was only compile tested x86_64_defconfig + CONFIG_ATH_CARDS=m
>>> CONFIG_ATH10K=m
>>> patch is against linux-next 3.19.0-rc1 -next-20141226
>> Rather patches. It would have been better to send one patch instead of
>> 4 patches with the same name.
> sorry for that - I had split it into separate patches as it was
> in different files - giving them the same name of course was a bit
> brain-dead.
You should have mentioned the modified files in the subject. But IMHO it
would be better to have just one patch.
> please do give it one more look - if the above argument is invalid
> I apologize for the noise.
It's me who should apologize. :-<
> thx!
> hofrat
WBR, Sergei
^ permalink raw reply
* Re: [RFC PATCH net-next 0/5]: ixgbevf: Allow querying VFs RSS indirection table and key
From: Jeff Kirsher @ 2014-12-30 18:52 UTC (permalink / raw)
To: Vladislav Zolotarov; +Cc: Gleb Natapov, netdev, avi
In-Reply-To: <CAOYyTHb+5Wc-e3vBVE+MCh6YNfmompi9Mnsr0nDnHAn7Tvqyew@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1630 bytes --]
On Tue, 2014-12-30 at 20:50 +0200, Vladislav Zolotarov wrote:
> On Dec 30, 2014 8:16 PM, "Jeff Kirsher" <jeffrey.t.kirsher@intel.com>
> wrote:
> >
> > On Tue, Dec 30, 2014 at 8:30 AM, Vlad Zolotarov
> > <vladz@cloudius-systems.com> wrote:
> > > Add the ethtool ops to VF driver to allow querying the RSS
> indirection table
> > > and RSS Random Key.
> > >
> > > - PF driver: Add new VF-PF channel commands.
> > > - VF driver: Utilize these new commands and add the corresponding
> > > ethtool callbacks.
> > >
> > > Vlad Zolotarov (5):
> > > ixgbe: Add a RETA query command to VF-PF channel API
> > > ixgbevf: Add a RETA query code
> > > ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set
> > > ixgbevf: Add RSS Key query code
> > > ixgbevf: Add the appropriate ethtool ops to query RSS
> indirection
> > > table and key
> > >
> > > drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 8 ++
> > > drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 60
> +++++++++++
> > > drivers/net/ethernet/intel/ixgbevf/ethtool.c | 37 +++++++
> > > drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 +-
> > > drivers/net/ethernet/intel/ixgbevf/mbx.h | 8 ++
> > > drivers/net/ethernet/intel/ixgbevf/vf.c | 125
> ++++++++++++++++++++++
> > > drivers/net/ethernet/intel/ixgbevf/vf.h | 2 +
> > > 7 files changed, 243 insertions(+), 1 deletion(-)
> >
> > Remember to CC on patches against drivers in
> drivers/net/ethernet/intel/ please.
>
> To CC who? ;)
>
Sorry, I fat fingered that response. Please CC me.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [RFC PATCH net-next 1/5] ixgbe: Add a RETA query command to VF-PF channel API
From: Jeff Kirsher @ 2014-12-30 19:21 UTC (permalink / raw)
To: Vlad Zolotarov; +Cc: netdev, gleb, avi
In-Reply-To: <1419957035-1078-2-git-send-email-vladz@cloudius-systems.com>
On Tue, Dec 30, 2014 at 8:30 AM, Vlad Zolotarov
<vladz@cloudius-systems.com> wrote:
> 82599 VFs and PF share the same RSS redirection table (RETA). Therefore we
> just return it for all VFs.
>
> RETA table is an array of 32 registers and the maximum number of registers
> that may be delivered in a single VF-PF channel command is 15. Therefore
> we will deliver the whole table in 3 steps: 12, 12 and 8 registers in each
> step correspondingly. Thus this patch does the following:
>
> - Adds a new API version (to specify a new commands set).
> - Adds the IXGBE_VF_GET_RETA_[0,1,2] commands to the VF-PF commands set.
>
> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 6 +++++
> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 35 ++++++++++++++++++++++++++
> 2 files changed, 41 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
> index a5cb755..c1123d9 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
> @@ -73,6 +73,7 @@ enum ixgbe_pfvf_api_rev {
> ixgbe_mbox_api_10, /* API version 1.0, linux/freebsd VF driver */
> ixgbe_mbox_api_20, /* API version 2.0, solaris Phase1 VF driver */
> ixgbe_mbox_api_11, /* API version 1.1, linux/freebsd VF driver */
> + ixgbe_mbox_api_12, /* API version 1.2, linux/freebsd VF driver */
> /* This value should always be last */
> ixgbe_mbox_api_unknown, /* indicates that API version is not known */
> };
> @@ -91,6 +92,11 @@ enum ixgbe_pfvf_api_rev {
> /* mailbox API, version 1.1 VF requests */
> #define IXGBE_VF_GET_QUEUES 0x09 /* get queue configuration */
>
> +/* mailbox API, version 1.2 VF requests */
> +#define IXGBE_VF_GET_RETA_0 0x0a /* get RETA[0..11] */
> +#define IXGBE_VF_GET_RETA_1 0x0b /* get RETA[12..23] */
> +#define IXGBE_VF_GET_RETA_2 0x0c /* get RETA[24..31] */
> +
> /* GET_QUEUES return data indices within the mailbox */
> #define IXGBE_VF_TX_QUEUES 1 /* number of Tx queues supported */
> #define IXGBE_VF_RX_QUEUES 2 /* number of Rx queues supported */
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> index c76ba90..84db1a5 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> @@ -427,6 +427,7 @@ static s32 ixgbe_set_vf_lpe(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
> #endif /* CONFIG_FCOE */
> switch (adapter->vfinfo[vf].vf_api) {
> case ixgbe_mbox_api_11:
> + case ixgbe_mbox_api_12:
> /*
> * Version 1.1 supports jumbo frames on VFs if PF has
> * jumbo frames enabled which means legacy VFs are
> @@ -894,6 +895,7 @@ static int ixgbe_negotiate_vf_api(struct ixgbe_adapter *adapter,
> switch (api) {
> case ixgbe_mbox_api_10:
> case ixgbe_mbox_api_11:
> + case ixgbe_mbox_api_12:
> adapter->vfinfo[vf].vf_api = api;
> return 0;
> default:
> @@ -917,6 +919,7 @@ static int ixgbe_get_vf_queues(struct ixgbe_adapter *adapter,
> switch (adapter->vfinfo[vf].vf_api) {
> case ixgbe_mbox_api_20:
> case ixgbe_mbox_api_11:
> + case ixgbe_mbox_api_12:
> break;
> default:
> return -1;
> @@ -944,6 +947,29 @@ static int ixgbe_get_vf_queues(struct ixgbe_adapter *adapter,
> return 0;
> }
>
> +static int ixgbe_get_vf_reta(struct ixgbe_adapter *adapter,
> + u32 *msgbuf, u32 vf, int reta_offset_dw,
> + size_t dwords)
> +{
> + struct ixgbe_hw *hw = &adapter->hw;
> + int i;
> + u32 *reta = &msgbuf[1];
> +
> + /* verify the PF is supporting the correct API */
> + switch (adapter->vfinfo[vf].vf_api) {
> + case ixgbe_mbox_api_12:
> + break;
> + default:
> + return -EPERM;
> + }
A switch statement is overkill for a single case. It would be far
simpler to just have
if (adapter->vfinfo[vf].vf_api != ixgbe_mbox_api_12)
return -EPERM;
Later on, if there are multiple API's or future API's that also
support this, then we could move to a case statement.
^ permalink raw reply
* Re: [RFC PATCH net-next 3/5] ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set
From: Jeff Kirsher @ 2014-12-30 19:23 UTC (permalink / raw)
To: Vlad Zolotarov; +Cc: netdev, Gleb Natapov, Avi Kivity
In-Reply-To: <1419957035-1078-4-git-send-email-vladz@cloudius-systems.com>
On Tue, Dec 30, 2014 at 8:30 AM, Vlad Zolotarov
<vladz@cloudius-systems.com> wrote:
> 82599 VFs and PF share the same RSS Key. Therefore we will return
> the same RSS key for all VFs.
>
> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 2 ++
> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 25 +++++++++++++++++++++++++
> 2 files changed, 27 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
> index c1123d9..52e775b 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
> @@ -97,6 +97,8 @@ enum ixgbe_pfvf_api_rev {
> #define IXGBE_VF_GET_RETA_1 0x0b /* get RETA[12..23] */
> #define IXGBE_VF_GET_RETA_2 0x0c /* get RETA[24..31] */
>
> +#define IXGBE_VF_GET_RSS_KEY 0x0d /* get RSS key */
> +
> /* GET_QUEUES return data indices within the mailbox */
> #define IXGBE_VF_TX_QUEUES 1 /* number of Tx queues supported */
> #define IXGBE_VF_RX_QUEUES 2 /* number of Rx queues supported */
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> index 84db1a5..fc8233e 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> @@ -970,6 +970,28 @@ static int ixgbe_get_vf_reta(struct ixgbe_adapter *adapter,
> return 0;
> }
>
> +static int ixgbe_get_vf_rss_key(struct ixgbe_adapter *adapter,
> + u32 *msgbuf, u32 vf)
> +{
> + struct ixgbe_hw *hw = &adapter->hw;
> + int i;
> + u32 *rss_key = &msgbuf[1];
> +
> + /* verify the PF is supporting the correct API */
> + switch (adapter->vfinfo[vf].vf_api) {
> + case ixgbe_mbox_api_12:
> + break;
> + default:
> + return -EPERM;
> + }
Same as in patch 01 of the series, a switch statement is overkill for
a single case. It would be far simpler to just have
if (adapter->vfinfo[vf].vf_api != ixgbe_mbox_api_12)
return -EPERM;
Later on, if there are multiple API's or future API's that also
support this, then we could move to a case statement.
^ permalink raw reply
* RE: [PATCH 3/6] bridge: modify IFLA_AF_SPEC parser to parse IFLA_BRIDGE_VLAN_RANGE_INFO
From: Arad, Ronen @ 2014-12-30 19:23 UTC (permalink / raw)
To: roopa, netdev@vger.kernel.org
Cc: shemminger@vyatta.com, vyasevic@redhat.com, Wilson kok
In-Reply-To: <54A2B409.1010708@cumulusnetworks.com>
>-----Original Message-----
>From: roopa [mailto:roopa@cumulusnetworks.com]
>Sent: Tuesday, December 30, 2014 4:18 PM
>To: Arad, Ronen
>Cc: netdev@vger.kernel.org; shemminger@vyatta.com; vyasevic@redhat.com; Wilson
>kok
>Subject: Re: [PATCH 3/6] bridge: modify IFLA_AF_SPEC parser to parse
>IFLA_BRIDGE_VLAN_RANGE_INFO
>
>On 12/30/14, 12:40 AM, Arad, Ronen wrote:
>>
>>> -----Original Message-----
>>> From: netdev-owner@vger.kernel.org [mailto:netdev-owner@vger.kernel.org] On
>>> Behalf Of roopa@cumulusnetworks.com
>>> Sent: Monday, December 29, 2014 11:05 PM
>>> To: netdev@vger.kernel.org; shemminger@vyatta.com; vyasevic@redhat.com
>>> Cc: Roopa Prabhu; Wilson kok
>>> Subject: [PATCH 3/6] bridge: modify IFLA_AF_SPEC parser to parse
>>> IFLA_BRIDGE_VLAN_RANGE_INFO
>>>
>>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>>
>>> This patch modifies br_afspec to parse incoming IFLA_BRIDGE_VLAN_RANGE_INFO
>>>
>>> Signed-off-by: Wilson kok <wkok@cumulusnetworks.com>
>>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>>> ---
>>> net/bridge/br_netlink.c | 70 +++++++++++++++++++++++++++++++++-----------
>--
>>> -
>>> 1 file changed, 49 insertions(+), 21 deletions(-)
>>>
>>> diff --git a/net/bridge/br_netlink.c b/net/bridge/br_netlink.c
>>> index e7d1fc0..4c47ba0 100644
>>> --- a/net/bridge/br_netlink.c
>>> +++ b/net/bridge/br_netlink.c
>>> @@ -227,48 +227,76 @@ static const struct nla_policy
>>> ifla_br_policy[IFLA_MAX+1] = {
>>> .len = sizeof(struct bridge_vlan_range_info), },
>>> };
>>>
>>> +static int br_afspec_vlan_add(struct net_bridge *br,
>>> + struct net_bridge_port *p,
>>> + u16 vid, u16 flags)
>>> +{
>>> + int err = 0;
>>> +
>>> + if (p) {
>>> + err = nbp_vlan_add(p, vid, flags);
>>> + if (err)
>>> + return err;
>>> +
>>> + if (flags & BRIDGE_VLAN_INFO_MASTER)
>>> + err = br_vlan_add(p->br, vid, flags);
>>> + } else {
>>> + err = br_vlan_add(br, vid, flags);
>>> + }
>>> +
>>> + return err;
>>> +}
>>> +
>>> +static void br_afspec_vlan_del(struct net_bridge *br,
>>> + struct net_bridge_port *p,
>>> + u16 vid, u16 flags)
>>> +{
>>> + if (p) {
>>> + nbp_vlan_delete(p, vid);
>>> + if (flags & BRIDGE_VLAN_INFO_MASTER)
>>> + br_vlan_delete(p->br, vid);
>>> + } else {
>>> + br_vlan_delete(br, vid);
>>> + }
>>> +}
>>> +
>>> static int br_afspec(struct net_bridge *br,
>>> struct net_bridge_port *p,
>>> struct nlattr *af_spec,
>>> int cmd)
>>> {
>>> - struct bridge_vlan_info *vinfo;
>>> - int err = 0;
>>> + struct bridge_vlan_range_info *vinfo;
>>> struct nlattr *attr;
>>> int err = 0;
>>> int rem;
>>> u16 vid;
>>>
>>> nla_for_each_nested(attr, af_spec, rem) {
>>> - if (nla_type(attr) != IFLA_BRIDGE_VLAN_INFO)
>>> + if (nla_type(attr) != IFLA_BRIDGE_VLAN_INFO &&
>>> + nla_type(attr) != IFLA_BRIDGE_VLAN_RANGE_INFO)
>>> continue;
>>> -
>>> vinfo = nla_data(attr);
>>> - if (!vinfo->vid || vinfo->vid >= VLAN_VID_MASK)
>>> +
>>> + if (nla_type(attr) == IFLA_BRIDGE_VLAN_INFO)
>>> + vinfo->vid_end = vinfo->vid;
>>> +
>>> + if (!vinfo->vid || vinfo->vid >= VLAN_VID_MASK ||
>>> + vinfo->vid_end >= VLAN_VID_MASK ||
>>> + vinfo->vid > vinfo->vid_end)
>>> return -EINVAL;
>>>
>>> switch (cmd) {
>>> case RTM_SETLINK:
>>> - if (p) {
>>> - err = nbp_vlan_add(p, vinfo->vid, vinfo->flags);
>>> + for (vid = vinfo->vid; vid <= vinfo->vid_end; vid++) {
>>> + err = br_afspec_vlan_add(br, p, vid,
>>> + vinfo->flags);
>> vinfo->flags could have BRIDGE_VLAN_INFO_PVID set. It is really a port
>property and there could only be a single PVID for a port. The loop will make
>the port pvid set, in turn, to each vid in the range until it finally set to
>vid_end.
>> This could be avoided by turning off this flag for all but the last vid in
>range:
>>
>> err = br_afspec_vlan_addr(br, p, vid,
>> ((vid == vinfo-
>>vid_end)
>> ? vinfo->flags
>> : vinfo->flags &
>~BRIDGE_VLAN_INFO_PVID));
>> Another alternative could be to add explicit pvid field to
>bridge_vlan_range_info and disallow PVID flag.
>> This allows for setting the port pvid to any vid in the range.
>>
>> If both alternatives seem somewhat complex we could just disallow PVID flag
>in IFLA_BRIDGE_VLAN_RANGE_INFO and allow it only in IFLA_BRIDGE_VLAN_INFO.
>
>I believe it is a user error to set PVID flag on a range of vlans using
>IFLA_BRIDGE_VLAN_RANGE_INFO
>or for that matter using multiple IFLA_BRIDGE_VLAN_INFO with pvid flag
>set. Today, the last one probably sticks.
>I will check current behavior and mimic that or return EINVAL when
>multiple attributes come in with the PVID flag.
>
I don't believe it was possible to have multiple IFLA_BRIDGE_VLAN_INFO before
the proposed patch. The presence of multiple IFLA_BRIDGE_VLAN_INFO and
IFLA_BRIDGE_VLAN_RANGE_INFO could arise from iproute2 that will take mixed list of vids and vid ranges as such:
# bridge vlan add dev NAME vid VLAN[-VLAN][,VLAN[-VLAN]]* [untagged] [self] \
[master]
Non-consecutive VLANs will be represented by individual IFLA_BRIDGE_VLAN_INFO
While consecutive ranges will be represented by IFLA_BRIDGE_VLAN_RANGE_INFO.
It seems reasonable for iproute2 "bridge vlan" command to
only allow "pvid" keyword when a single vid is entered and forbid it when vid
range is entered. This will make the presence of PVID flag with multiple
IFLA_BRIDGE_VLAN_INFO or in IFLA_BRIDGE_VLAN_RANGE_INFO unlikely. Nevertheless,
br_setlink() should prevent multiple PVID setting using a single setlink
message. EINVAL seems better for this unlikely case.
>
>
>>
>>
>>> if (err)
>>> break;
>>> -
>>> - if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
>>> - err = br_vlan_add(p->br, vinfo->vid,
>>> - vinfo->flags);
>>> - } else
>>> - err = br_vlan_add(br, vinfo->vid, vinfo->flags);
>>> -
>>> + }
>>> break;
>>> -
>>> case RTM_DELLINK:
>>> - if (p) {
>>> - nbp_vlan_delete(p, vinfo->vid);
>>> - if (vinfo->flags & BRIDGE_VLAN_INFO_MASTER)
>>> - br_vlan_delete(p->br, vinfo->vid);
>>> - } else
>>> - br_vlan_delete(br, vinfo->vid);
>>> + for (vid = vinfo->vid; vid <= vinfo->vid_end; vid++)
>>> + br_afspec_vlan_del(br, p, vid, vinfo->flags);
>>> break;
>>> }
>>> }
>>> --
>>> 1.7.10.4
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [RFC PATCH net-next 4/5] ixgbevf: Add RSS Key query code
From: Jeff Kirsher @ 2014-12-30 19:26 UTC (permalink / raw)
To: Vlad Zolotarov; +Cc: netdev, Gleb Natapov, Avi Kivity
In-Reply-To: <1419957035-1078-5-git-send-email-vladz@cloudius-systems.com>
On Tue, Dec 30, 2014 at 8:30 AM, Vlad Zolotarov
<vladz@cloudius-systems.com> wrote:
> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
> ---
> drivers/net/ethernet/intel/ixgbevf/mbx.h | 2 ++
> drivers/net/ethernet/intel/ixgbevf/vf.c | 48 ++++++++++++++++++++++++++++++++
> drivers/net/ethernet/intel/ixgbevf/vf.h | 1 +
> 3 files changed, 51 insertions(+)
>
> diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h b/drivers/net/ethernet/intel/ixgbevf/mbx.h
> index 3e148a8..9674ac8 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/mbx.h
> +++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h
> @@ -110,6 +110,8 @@ enum ixgbe_pfvf_api_rev {
> #define IXGBE_VF_GET_RETA_1 0x0b /* get RETA[12..23] */
> #define IXGBE_VF_GET_RETA_2 0x0c /* get RETA[24..31] */
>
> +#define IXGBE_VF_GET_RSS_KEY 0x0d /* get RSS hash key */
> +
> /* GET_QUEUES return data indices within the mailbox */
> #define IXGBE_VF_TX_QUEUES 1 /* number of Tx queues supported */
> #define IXGBE_VF_RX_QUEUES 2 /* number of Rx queues supported */
> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c
> index 3ebc882..7987ab0 100644
> --- a/drivers/net/ethernet/intel/ixgbevf/vf.c
> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.c
> @@ -290,6 +290,54 @@ static inline int _ixgbevf_get_reta(struct ixgbe_hw *hw, u32 *msgbuf,
> }
>
> /**
> + * ixgbevf_get_rss_key - get the RSS Random Key
> + * @hw: pointer to the HW structure
> + * @reta: buffer to fill with RETA contents.
> + *
> + * The "rss_key" buffer should be big enough to contain 10 registers.
> + *
> + * Returns: 0 on success.
> + * if API doesn't support this operation - (-EPERM).
> + */
> +int ixgbevf_get_rss_key(struct ixgbe_hw *hw, u8 *rss_key)
> +{
> + int err;
> + u32 msgbuf[IXGBE_VFMAILBOX_SIZE];
> +
> + /* Return and error if API doesn't support RSS Random Key retrieval */
> + switch (hw->api_version) {
> + case ixgbe_mbox_api_12:
> + break;
> + default:
> + return -EPERM;
> + }
An if statement here as well, instead of a switch statement.
^ permalink raw reply
* Re: [RFC PATCH net-next 1/5] ixgbe: Add a RETA query command to VF-PF channel API
From: Vlad Zolotarov @ 2014-12-30 19:28 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: netdev, gleb, avi
In-Reply-To: <CAL3LdT4mfE72fmA3b3iG=-b9iP9MXHfAJMBwqFzfYyC1pebc8A@mail.gmail.com>
On 12/30/14 21:21, Jeff Kirsher wrote:
> On Tue, Dec 30, 2014 at 8:30 AM, Vlad Zolotarov
> <vladz@cloudius-systems.com> wrote:
>> 82599 VFs and PF share the same RSS redirection table (RETA). Therefore we
>> just return it for all VFs.
>>
>> RETA table is an array of 32 registers and the maximum number of registers
>> that may be delivered in a single VF-PF channel command is 15. Therefore
>> we will deliver the whole table in 3 steps: 12, 12 and 8 registers in each
>> step correspondingly. Thus this patch does the following:
>>
>> - Adds a new API version (to specify a new commands set).
>> - Adds the IXGBE_VF_GET_RETA_[0,1,2] commands to the VF-PF commands set.
>>
>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
>> ---
>> drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 6 +++++
>> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 35 ++++++++++++++++++++++++++
>> 2 files changed, 41 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>> index a5cb755..c1123d9 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>> @@ -73,6 +73,7 @@ enum ixgbe_pfvf_api_rev {
>> ixgbe_mbox_api_10, /* API version 1.0, linux/freebsd VF driver */
>> ixgbe_mbox_api_20, /* API version 2.0, solaris Phase1 VF driver */
>> ixgbe_mbox_api_11, /* API version 1.1, linux/freebsd VF driver */
>> + ixgbe_mbox_api_12, /* API version 1.2, linux/freebsd VF driver */
>> /* This value should always be last */
>> ixgbe_mbox_api_unknown, /* indicates that API version is not known */
>> };
>> @@ -91,6 +92,11 @@ enum ixgbe_pfvf_api_rev {
>> /* mailbox API, version 1.1 VF requests */
>> #define IXGBE_VF_GET_QUEUES 0x09 /* get queue configuration */
>>
>> +/* mailbox API, version 1.2 VF requests */
>> +#define IXGBE_VF_GET_RETA_0 0x0a /* get RETA[0..11] */
>> +#define IXGBE_VF_GET_RETA_1 0x0b /* get RETA[12..23] */
>> +#define IXGBE_VF_GET_RETA_2 0x0c /* get RETA[24..31] */
>> +
>> /* GET_QUEUES return data indices within the mailbox */
>> #define IXGBE_VF_TX_QUEUES 1 /* number of Tx queues supported */
>> #define IXGBE_VF_RX_QUEUES 2 /* number of Rx queues supported */
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>> index c76ba90..84db1a5 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>> @@ -427,6 +427,7 @@ static s32 ixgbe_set_vf_lpe(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
>> #endif /* CONFIG_FCOE */
>> switch (adapter->vfinfo[vf].vf_api) {
>> case ixgbe_mbox_api_11:
>> + case ixgbe_mbox_api_12:
>> /*
>> * Version 1.1 supports jumbo frames on VFs if PF has
>> * jumbo frames enabled which means legacy VFs are
>> @@ -894,6 +895,7 @@ static int ixgbe_negotiate_vf_api(struct ixgbe_adapter *adapter,
>> switch (api) {
>> case ixgbe_mbox_api_10:
>> case ixgbe_mbox_api_11:
>> + case ixgbe_mbox_api_12:
>> adapter->vfinfo[vf].vf_api = api;
>> return 0;
>> default:
>> @@ -917,6 +919,7 @@ static int ixgbe_get_vf_queues(struct ixgbe_adapter *adapter,
>> switch (adapter->vfinfo[vf].vf_api) {
>> case ixgbe_mbox_api_20:
>> case ixgbe_mbox_api_11:
>> + case ixgbe_mbox_api_12:
>> break;
>> default:
>> return -1;
>> @@ -944,6 +947,29 @@ static int ixgbe_get_vf_queues(struct ixgbe_adapter *adapter,
>> return 0;
>> }
>>
>> +static int ixgbe_get_vf_reta(struct ixgbe_adapter *adapter,
>> + u32 *msgbuf, u32 vf, int reta_offset_dw,
>> + size_t dwords)
>> +{
>> + struct ixgbe_hw *hw = &adapter->hw;
>> + int i;
>> + u32 *reta = &msgbuf[1];
>> +
>> + /* verify the PF is supporting the correct API */
>> + switch (adapter->vfinfo[vf].vf_api) {
>> + case ixgbe_mbox_api_12:
>> + break;
>> + default:
>> + return -EPERM;
>> + }
> A switch statement is overkill for a single case. It would be far
> simpler to just have
>
> if (adapter->vfinfo[vf].vf_api != ixgbe_mbox_api_12)
> return -EPERM;
>
> Later on, if there are multiple API's or future API's that also
> support this, then we could move to a case statement.
I thought about it to but decided to make it future-proof. NP. Will make
it an "if" in a non-RFC series. Thanks.
^ permalink raw reply
* Re: [RFC PATCH net-next 0/5]: ixgbevf: Allow querying VFs RSS indirection table and key
From: Jeff Kirsher @ 2014-12-30 19:28 UTC (permalink / raw)
To: Vladislav Zolotarov, Greg Rose; +Cc: netdev, Gleb Natapov, avi
In-Reply-To: <CAOYyTHathHiGThyHj+yRi9O6jWEAO3wkAK-6egMc2OwK9dQoMQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2296 bytes --]
On Tue, 2014-12-30 at 21:00 +0200, Vladislav Zolotarov wrote:
>
> On Dec 30, 2014 8:52 PM, "Jeff Kirsher" <jeffrey.t.kirsher@intel.com>
> wrote:
> >
> > On Tue, 2014-12-30 at 20:50 +0200, Vladislav Zolotarov wrote:
> > > On Dec 30, 2014 8:16 PM, "Jeff Kirsher"
> <jeffrey.t.kirsher@intel.com>
> > > wrote:
> > > >
> > > > On Tue, Dec 30, 2014 at 8:30 AM, Vlad Zolotarov
> > > > <vladz@cloudius-systems.com> wrote:
> > > > > Add the ethtool ops to VF driver to allow querying the RSS
> > > indirection table
> > > > > and RSS Random Key.
> > > > >
> > > > > - PF driver: Add new VF-PF channel commands.
> > > > > - VF driver: Utilize these new commands and add the
> corresponding
> > > > > ethtool callbacks.
> > > > >
> > > > > Vlad Zolotarov (5):
> > > > > ixgbe: Add a RETA query command to VF-PF channel API
> > > > > ixgbevf: Add a RETA query code
> > > > > ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set
> > > > > ixgbevf: Add RSS Key query code
> > > > > ixgbevf: Add the appropriate ethtool ops to query RSS
> > > indirection
> > > > > table and key
> > > > >
> > > > > drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 8 ++
> > > > > drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 60
> > > +++++++++++
> > > > > drivers/net/ethernet/intel/ixgbevf/ethtool.c | 37
> +++++++
> > > > > drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 +-
> > > > > drivers/net/ethernet/intel/ixgbevf/mbx.h | 8 ++
> > > > > drivers/net/ethernet/intel/ixgbevf/vf.c | 125
> > > ++++++++++++++++++++++
> > > > > drivers/net/ethernet/intel/ixgbevf/vf.h | 2 +
> > > > > 7 files changed, 243 insertions(+), 1 deletion(-)
> > > >
> > > > Remember to CC on patches against drivers in
> > > drivers/net/ethernet/intel/ please.
> > >
> > > To CC who? ;)
> > >
> >
> > Sorry, I fat fingered that response. Please CC me.
>
> Sure. NP.
> Since we are talking already, could u comment on the RFC? Does it look
> ok to you?
>
>
I have provided feedback on 3 of the patches, the others look ok to me.
I would like Greg Rose to look over your changes since he did most of
the ixgbevf work, and I want to make sure that he is fine with these
changes.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [RFC PATCH net-next 3/5] ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set
From: Vlad Zolotarov @ 2014-12-30 19:29 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: netdev, Gleb Natapov, Avi Kivity
In-Reply-To: <CAL3LdT50tUsJOW+y15GbO=90UDYWNLjMyOF6AqFgj_ciqBeTUQ@mail.gmail.com>
On 12/30/14 21:23, Jeff Kirsher wrote:
> On Tue, Dec 30, 2014 at 8:30 AM, Vlad Zolotarov
> <vladz@cloudius-systems.com> wrote:
>> 82599 VFs and PF share the same RSS Key. Therefore we will return
>> the same RSS key for all VFs.
>>
>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
>> ---
>> drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 2 ++
>> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 25 +++++++++++++++++++++++++
>> 2 files changed, 27 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>> index c1123d9..52e775b 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>> @@ -97,6 +97,8 @@ enum ixgbe_pfvf_api_rev {
>> #define IXGBE_VF_GET_RETA_1 0x0b /* get RETA[12..23] */
>> #define IXGBE_VF_GET_RETA_2 0x0c /* get RETA[24..31] */
>>
>> +#define IXGBE_VF_GET_RSS_KEY 0x0d /* get RSS key */
>> +
>> /* GET_QUEUES return data indices within the mailbox */
>> #define IXGBE_VF_TX_QUEUES 1 /* number of Tx queues supported */
>> #define IXGBE_VF_RX_QUEUES 2 /* number of Rx queues supported */
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>> index 84db1a5..fc8233e 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>> @@ -970,6 +970,28 @@ static int ixgbe_get_vf_reta(struct ixgbe_adapter *adapter,
>> return 0;
>> }
>>
>> +static int ixgbe_get_vf_rss_key(struct ixgbe_adapter *adapter,
>> + u32 *msgbuf, u32 vf)
>> +{
>> + struct ixgbe_hw *hw = &adapter->hw;
>> + int i;
>> + u32 *rss_key = &msgbuf[1];
>> +
>> + /* verify the PF is supporting the correct API */
>> + switch (adapter->vfinfo[vf].vf_api) {
>> + case ixgbe_mbox_api_12:
>> + break;
>> + default:
>> + return -EPERM;
>> + }
> Same as in patch 01 of the series, a switch statement is overkill for
> a single case. It would be far simpler to just have
>
> if (adapter->vfinfo[vf].vf_api != ixgbe_mbox_api_12)
> return -EPERM;
>
> Later on, if there are multiple API's or future API's that also
> support this, then we could move to a case statement.
Got it. Will fix.
Thanks.
^ permalink raw reply
* Re: [RFC PATCH net-next 1/5] ixgbe: Add a RETA query command to VF-PF channel API
From: Jeff Kirsher @ 2014-12-30 19:30 UTC (permalink / raw)
To: Vlad Zolotarov; +Cc: netdev, gleb, avi
In-Reply-To: <54A2FCE1.3000000@cloudius-systems.com>
[-- Attachment #1: Type: text/plain, Size: 5285 bytes --]
On Tue, 2014-12-30 at 21:28 +0200, Vlad Zolotarov wrote:
> On 12/30/14 21:21, Jeff Kirsher wrote:
> > On Tue, Dec 30, 2014 at 8:30 AM, Vlad Zolotarov
> > <vladz@cloudius-systems.com> wrote:
> >> 82599 VFs and PF share the same RSS redirection table (RETA). Therefore we
> >> just return it for all VFs.
> >>
> >> RETA table is an array of 32 registers and the maximum number of registers
> >> that may be delivered in a single VF-PF channel command is 15. Therefore
> >> we will deliver the whole table in 3 steps: 12, 12 and 8 registers in each
> >> step correspondingly. Thus this patch does the following:
> >>
> >> - Adds a new API version (to specify a new commands set).
> >> - Adds the IXGBE_VF_GET_RETA_[0,1,2] commands to the VF-PF commands set.
> >>
> >> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
> >> ---
> >> drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 6 +++++
> >> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 35 ++++++++++++++++++++++++++
> >> 2 files changed, 41 insertions(+)
> >>
> >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
> >> index a5cb755..c1123d9 100644
> >> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
> >> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
> >> @@ -73,6 +73,7 @@ enum ixgbe_pfvf_api_rev {
> >> ixgbe_mbox_api_10, /* API version 1.0, linux/freebsd VF driver */
> >> ixgbe_mbox_api_20, /* API version 2.0, solaris Phase1 VF driver */
> >> ixgbe_mbox_api_11, /* API version 1.1, linux/freebsd VF driver */
> >> + ixgbe_mbox_api_12, /* API version 1.2, linux/freebsd VF driver */
> >> /* This value should always be last */
> >> ixgbe_mbox_api_unknown, /* indicates that API version is not known */
> >> };
> >> @@ -91,6 +92,11 @@ enum ixgbe_pfvf_api_rev {
> >> /* mailbox API, version 1.1 VF requests */
> >> #define IXGBE_VF_GET_QUEUES 0x09 /* get queue configuration */
> >>
> >> +/* mailbox API, version 1.2 VF requests */
> >> +#define IXGBE_VF_GET_RETA_0 0x0a /* get RETA[0..11] */
> >> +#define IXGBE_VF_GET_RETA_1 0x0b /* get RETA[12..23] */
> >> +#define IXGBE_VF_GET_RETA_2 0x0c /* get RETA[24..31] */
> >> +
> >> /* GET_QUEUES return data indices within the mailbox */
> >> #define IXGBE_VF_TX_QUEUES 1 /* number of Tx queues supported */
> >> #define IXGBE_VF_RX_QUEUES 2 /* number of Rx queues supported */
> >> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> >> index c76ba90..84db1a5 100644
> >> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> >> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
> >> @@ -427,6 +427,7 @@ static s32 ixgbe_set_vf_lpe(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
> >> #endif /* CONFIG_FCOE */
> >> switch (adapter->vfinfo[vf].vf_api) {
> >> case ixgbe_mbox_api_11:
> >> + case ixgbe_mbox_api_12:
> >> /*
> >> * Version 1.1 supports jumbo frames on VFs if PF has
> >> * jumbo frames enabled which means legacy VFs are
> >> @@ -894,6 +895,7 @@ static int ixgbe_negotiate_vf_api(struct ixgbe_adapter *adapter,
> >> switch (api) {
> >> case ixgbe_mbox_api_10:
> >> case ixgbe_mbox_api_11:
> >> + case ixgbe_mbox_api_12:
> >> adapter->vfinfo[vf].vf_api = api;
> >> return 0;
> >> default:
> >> @@ -917,6 +919,7 @@ static int ixgbe_get_vf_queues(struct ixgbe_adapter *adapter,
> >> switch (adapter->vfinfo[vf].vf_api) {
> >> case ixgbe_mbox_api_20:
> >> case ixgbe_mbox_api_11:
> >> + case ixgbe_mbox_api_12:
> >> break;
> >> default:
> >> return -1;
> >> @@ -944,6 +947,29 @@ static int ixgbe_get_vf_queues(struct ixgbe_adapter *adapter,
> >> return 0;
> >> }
> >>
> >> +static int ixgbe_get_vf_reta(struct ixgbe_adapter *adapter,
> >> + u32 *msgbuf, u32 vf, int reta_offset_dw,
> >> + size_t dwords)
> >> +{
> >> + struct ixgbe_hw *hw = &adapter->hw;
> >> + int i;
> >> + u32 *reta = &msgbuf[1];
> >> +
> >> + /* verify the PF is supporting the correct API */
> >> + switch (adapter->vfinfo[vf].vf_api) {
> >> + case ixgbe_mbox_api_12:
> >> + break;
> >> + default:
> >> + return -EPERM;
> >> + }
> > A switch statement is overkill for a single case. It would be far
> > simpler to just have
> >
> > if (adapter->vfinfo[vf].vf_api != ixgbe_mbox_api_12)
> > return -EPERM;
> >
> > Later on, if there are multiple API's or future API's that also
> > support this, then we could move to a case statement.
>
> I thought about it to but decided to make it future-proof. NP. Will make
> it an "if" in a non-RFC series. Thanks.
>
Thanks fine, and if you CC me on the non-RFC series, I will pick them up
(and make sure that Greg Rose has a chance to review them).
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [RFC PATCH net-next 0/5]: ixgbevf: Allow querying VFs RSS indirection table and key
From: Vlad Zolotarov @ 2014-12-30 19:30 UTC (permalink / raw)
To: Jeff Kirsher, Greg Rose; +Cc: netdev, Gleb Natapov, avi
In-Reply-To: <1419967707.31582.23.camel@jtkirshe-mobl>
On 12/30/14 21:28, Jeff Kirsher wrote:
> On Tue, 2014-12-30 at 21:00 +0200, Vladislav Zolotarov wrote:
>> On Dec 30, 2014 8:52 PM, "Jeff Kirsher" <jeffrey.t.kirsher@intel.com>
>> wrote:
>>> On Tue, 2014-12-30 at 20:50 +0200, Vladislav Zolotarov wrote:
>>>> On Dec 30, 2014 8:16 PM, "Jeff Kirsher"
>> <jeffrey.t.kirsher@intel.com>
>>>> wrote:
>>>>> On Tue, Dec 30, 2014 at 8:30 AM, Vlad Zolotarov
>>>>> <vladz@cloudius-systems.com> wrote:
>>>>>> Add the ethtool ops to VF driver to allow querying the RSS
>>>> indirection table
>>>>>> and RSS Random Key.
>>>>>>
>>>>>> - PF driver: Add new VF-PF channel commands.
>>>>>> - VF driver: Utilize these new commands and add the
>> corresponding
>>>>>> ethtool callbacks.
>>>>>>
>>>>>> Vlad Zolotarov (5):
>>>>>> ixgbe: Add a RETA query command to VF-PF channel API
>>>>>> ixgbevf: Add a RETA query code
>>>>>> ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set
>>>>>> ixgbevf: Add RSS Key query code
>>>>>> ixgbevf: Add the appropriate ethtool ops to query RSS
>>>> indirection
>>>>>> table and key
>>>>>>
>>>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 8 ++
>>>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 60
>>>> +++++++++++
>>>>>> drivers/net/ethernet/intel/ixgbevf/ethtool.c | 37
>> +++++++
>>>>>> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 +-
>>>>>> drivers/net/ethernet/intel/ixgbevf/mbx.h | 8 ++
>>>>>> drivers/net/ethernet/intel/ixgbevf/vf.c | 125
>>>> ++++++++++++++++++++++
>>>>>> drivers/net/ethernet/intel/ixgbevf/vf.h | 2 +
>>>>>> 7 files changed, 243 insertions(+), 1 deletion(-)
>>>>> Remember to CC on patches against drivers in
>>>> drivers/net/ethernet/intel/ please.
>>>>
>>>> To CC who? ;)
>>>>
>>> Sorry, I fat fingered that response. Please CC me.
>> Sure. NP.
>> Since we are talking already, could u comment on the RFC? Does it look
>> ok to you?
>>
>>
> I have provided feedback on 3 of the patches, the others look ok to me.
> I would like Greg Rose to look over your changes since he did most of
> the ixgbevf work, and I want to make sure that he is fine with these
> changes.
I saw them. Thanks, Jeff. I'll wait for Greg's comments before cooking
the final series then.
Again, thanks for a rapid response.
^ permalink raw reply
* Re: [RFC PATCH net-next 0/5]: ixgbevf: Allow querying VFs RSS indirection table and key
From: Jeff Kirsher @ 2014-12-30 19:34 UTC (permalink / raw)
To: Vlad Zolotarov; +Cc: Greg Rose, netdev, Gleb Natapov, avi
In-Reply-To: <54A2FD6C.6050807@cloudius-systems.com>
[-- Attachment #1: Type: text/plain, Size: 2754 bytes --]
On Tue, 2014-12-30 at 21:30 +0200, Vlad Zolotarov wrote:
> On 12/30/14 21:28, Jeff Kirsher wrote:
> > On Tue, 2014-12-30 at 21:00 +0200, Vladislav Zolotarov wrote:
> >> On Dec 30, 2014 8:52 PM, "Jeff Kirsher" <jeffrey.t.kirsher@intel.com>
> >> wrote:
> >>> On Tue, 2014-12-30 at 20:50 +0200, Vladislav Zolotarov wrote:
> >>>> On Dec 30, 2014 8:16 PM, "Jeff Kirsher"
> >> <jeffrey.t.kirsher@intel.com>
> >>>> wrote:
> >>>>> On Tue, Dec 30, 2014 at 8:30 AM, Vlad Zolotarov
> >>>>> <vladz@cloudius-systems.com> wrote:
> >>>>>> Add the ethtool ops to VF driver to allow querying the RSS
> >>>> indirection table
> >>>>>> and RSS Random Key.
> >>>>>>
> >>>>>> - PF driver: Add new VF-PF channel commands.
> >>>>>> - VF driver: Utilize these new commands and add the
> >> corresponding
> >>>>>> ethtool callbacks.
> >>>>>>
> >>>>>> Vlad Zolotarov (5):
> >>>>>> ixgbe: Add a RETA query command to VF-PF channel API
> >>>>>> ixgbevf: Add a RETA query code
> >>>>>> ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set
> >>>>>> ixgbevf: Add RSS Key query code
> >>>>>> ixgbevf: Add the appropriate ethtool ops to query RSS
> >>>> indirection
> >>>>>> table and key
> >>>>>>
> >>>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 8 ++
> >>>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 60
> >>>> +++++++++++
> >>>>>> drivers/net/ethernet/intel/ixgbevf/ethtool.c | 37
> >> +++++++
> >>>>>> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 +-
> >>>>>> drivers/net/ethernet/intel/ixgbevf/mbx.h | 8 ++
> >>>>>> drivers/net/ethernet/intel/ixgbevf/vf.c | 125
> >>>> ++++++++++++++++++++++
> >>>>>> drivers/net/ethernet/intel/ixgbevf/vf.h | 2 +
> >>>>>> 7 files changed, 243 insertions(+), 1 deletion(-)
> >>>>> Remember to CC on patches against drivers in
> >>>> drivers/net/ethernet/intel/ please.
> >>>>
> >>>> To CC who? ;)
> >>>>
> >>> Sorry, I fat fingered that response. Please CC me.
> >> Sure. NP.
> >> Since we are talking already, could u comment on the RFC? Does it look
> >> ok to you?
> >>
> >>
> > I have provided feedback on 3 of the patches, the others look ok to me.
> > I would like Greg Rose to look over your changes since he did most of
> > the ixgbevf work, and I want to make sure that he is fine with these
> > changes.
>
> I saw them. Thanks, Jeff. I'll wait for Greg's comments before cooking
> the final series then.
>
> Again, thanks for a rapid response.
>
Go ahead and send out a non-RFC series. Greg is on vacation till the
5th anyway, so I will pick up your series and have my validation team
run it through their tests in the mean time.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [RFC PATCH net-next 1/5] ixgbe: Add a RETA query command to VF-PF channel API
From: Vlad Zolotarov @ 2014-12-30 19:35 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: netdev, gleb, avi
In-Reply-To: <1419967855.31582.24.camel@jtkirshe-mobl>
On 12/30/14 21:30, Jeff Kirsher wrote:
> On Tue, 2014-12-30 at 21:28 +0200, Vlad Zolotarov wrote:
>> On 12/30/14 21:21, Jeff Kirsher wrote:
>>> On Tue, Dec 30, 2014 at 8:30 AM, Vlad Zolotarov
>>> <vladz@cloudius-systems.com> wrote:
>>>> 82599 VFs and PF share the same RSS redirection table (RETA). Therefore we
>>>> just return it for all VFs.
>>>>
>>>> RETA table is an array of 32 registers and the maximum number of registers
>>>> that may be delivered in a single VF-PF channel command is 15. Therefore
>>>> we will deliver the whole table in 3 steps: 12, 12 and 8 registers in each
>>>> step correspondingly. Thus this patch does the following:
>>>>
>>>> - Adds a new API version (to specify a new commands set).
>>>> - Adds the IXGBE_VF_GET_RETA_[0,1,2] commands to the VF-PF commands set.
>>>>
>>>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com>
>>>> ---
>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 6 +++++
>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 35 ++++++++++++++++++++++++++
>>>> 2 files changed, 41 insertions(+)
>>>>
>>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>>>> index a5cb755..c1123d9 100644
>>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h
>>>> @@ -73,6 +73,7 @@ enum ixgbe_pfvf_api_rev {
>>>> ixgbe_mbox_api_10, /* API version 1.0, linux/freebsd VF driver */
>>>> ixgbe_mbox_api_20, /* API version 2.0, solaris Phase1 VF driver */
>>>> ixgbe_mbox_api_11, /* API version 1.1, linux/freebsd VF driver */
>>>> + ixgbe_mbox_api_12, /* API version 1.2, linux/freebsd VF driver */
>>>> /* This value should always be last */
>>>> ixgbe_mbox_api_unknown, /* indicates that API version is not known */
>>>> };
>>>> @@ -91,6 +92,11 @@ enum ixgbe_pfvf_api_rev {
>>>> /* mailbox API, version 1.1 VF requests */
>>>> #define IXGBE_VF_GET_QUEUES 0x09 /* get queue configuration */
>>>>
>>>> +/* mailbox API, version 1.2 VF requests */
>>>> +#define IXGBE_VF_GET_RETA_0 0x0a /* get RETA[0..11] */
>>>> +#define IXGBE_VF_GET_RETA_1 0x0b /* get RETA[12..23] */
>>>> +#define IXGBE_VF_GET_RETA_2 0x0c /* get RETA[24..31] */
>>>> +
>>>> /* GET_QUEUES return data indices within the mailbox */
>>>> #define IXGBE_VF_TX_QUEUES 1 /* number of Tx queues supported */
>>>> #define IXGBE_VF_RX_QUEUES 2 /* number of Rx queues supported */
>>>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>>>> index c76ba90..84db1a5 100644
>>>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>>>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c
>>>> @@ -427,6 +427,7 @@ static s32 ixgbe_set_vf_lpe(struct ixgbe_adapter *adapter, u32 *msgbuf, u32 vf)
>>>> #endif /* CONFIG_FCOE */
>>>> switch (adapter->vfinfo[vf].vf_api) {
>>>> case ixgbe_mbox_api_11:
>>>> + case ixgbe_mbox_api_12:
>>>> /*
>>>> * Version 1.1 supports jumbo frames on VFs if PF has
>>>> * jumbo frames enabled which means legacy VFs are
>>>> @@ -894,6 +895,7 @@ static int ixgbe_negotiate_vf_api(struct ixgbe_adapter *adapter,
>>>> switch (api) {
>>>> case ixgbe_mbox_api_10:
>>>> case ixgbe_mbox_api_11:
>>>> + case ixgbe_mbox_api_12:
>>>> adapter->vfinfo[vf].vf_api = api;
>>>> return 0;
>>>> default:
>>>> @@ -917,6 +919,7 @@ static int ixgbe_get_vf_queues(struct ixgbe_adapter *adapter,
>>>> switch (adapter->vfinfo[vf].vf_api) {
>>>> case ixgbe_mbox_api_20:
>>>> case ixgbe_mbox_api_11:
>>>> + case ixgbe_mbox_api_12:
>>>> break;
>>>> default:
>>>> return -1;
>>>> @@ -944,6 +947,29 @@ static int ixgbe_get_vf_queues(struct ixgbe_adapter *adapter,
>>>> return 0;
>>>> }
>>>>
>>>> +static int ixgbe_get_vf_reta(struct ixgbe_adapter *adapter,
>>>> + u32 *msgbuf, u32 vf, int reta_offset_dw,
>>>> + size_t dwords)
>>>> +{
>>>> + struct ixgbe_hw *hw = &adapter->hw;
>>>> + int i;
>>>> + u32 *reta = &msgbuf[1];
>>>> +
>>>> + /* verify the PF is supporting the correct API */
>>>> + switch (adapter->vfinfo[vf].vf_api) {
>>>> + case ixgbe_mbox_api_12:
>>>> + break;
>>>> + default:
>>>> + return -EPERM;
>>>> + }
>>> A switch statement is overkill for a single case. It would be far
>>> simpler to just have
>>>
>>> if (adapter->vfinfo[vf].vf_api != ixgbe_mbox_api_12)
>>> return -EPERM;
>>>
>>> Later on, if there are multiple API's or future API's that also
>>> support this, then we could move to a case statement.
>> I thought about it to but decided to make it future-proof. NP. Will make
>> it an "if" in a non-RFC series. Thanks.
>>
> Thanks fine, and if you CC me on the non-RFC series,
Sure thing I will! ;) After such a lightning fast review I'll CC u for
all my future net patches! ;)
> I will pick them up
> (and make sure that Greg Rose has a chance to review them).
Cool. Then I'll cook the series first think tomorrow morning.
Thanks a lot, Jeff
^ permalink raw reply
* Re: [RFC PATCH net-next 0/5]: ixgbevf: Allow querying VFs RSS indirection table and key
From: Vlad Zolotarov @ 2014-12-30 19:36 UTC (permalink / raw)
To: Jeff Kirsher; +Cc: Greg Rose, netdev, Gleb Natapov, avi
In-Reply-To: <1419968078.31582.27.camel@jtkirshe-mobl>
On 12/30/14 21:34, Jeff Kirsher wrote:
> On Tue, 2014-12-30 at 21:30 +0200, Vlad Zolotarov wrote:
>> On 12/30/14 21:28, Jeff Kirsher wrote:
>>> On Tue, 2014-12-30 at 21:00 +0200, Vladislav Zolotarov wrote:
>>>> On Dec 30, 2014 8:52 PM, "Jeff Kirsher" <jeffrey.t.kirsher@intel.com>
>>>> wrote:
>>>>> On Tue, 2014-12-30 at 20:50 +0200, Vladislav Zolotarov wrote:
>>>>>> On Dec 30, 2014 8:16 PM, "Jeff Kirsher"
>>>> <jeffrey.t.kirsher@intel.com>
>>>>>> wrote:
>>>>>>> On Tue, Dec 30, 2014 at 8:30 AM, Vlad Zolotarov
>>>>>>> <vladz@cloudius-systems.com> wrote:
>>>>>>>> Add the ethtool ops to VF driver to allow querying the RSS
>>>>>> indirection table
>>>>>>>> and RSS Random Key.
>>>>>>>>
>>>>>>>> - PF driver: Add new VF-PF channel commands.
>>>>>>>> - VF driver: Utilize these new commands and add the
>>>> corresponding
>>>>>>>> ethtool callbacks.
>>>>>>>>
>>>>>>>> Vlad Zolotarov (5):
>>>>>>>> ixgbe: Add a RETA query command to VF-PF channel API
>>>>>>>> ixgbevf: Add a RETA query code
>>>>>>>> ixgbe: Add GET_RSS_KEY command to VF-PF channel commands set
>>>>>>>> ixgbevf: Add RSS Key query code
>>>>>>>> ixgbevf: Add the appropriate ethtool ops to query RSS
>>>>>> indirection
>>>>>>>> table and key
>>>>>>>>
>>>>>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_mbx.h | 8 ++
>>>>>>>> drivers/net/ethernet/intel/ixgbe/ixgbe_sriov.c | 60
>>>>>> +++++++++++
>>>>>>>> drivers/net/ethernet/intel/ixgbevf/ethtool.c | 37
>>>> +++++++
>>>>>>>> drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c | 4 +-
>>>>>>>> drivers/net/ethernet/intel/ixgbevf/mbx.h | 8 ++
>>>>>>>> drivers/net/ethernet/intel/ixgbevf/vf.c | 125
>>>>>> ++++++++++++++++++++++
>>>>>>>> drivers/net/ethernet/intel/ixgbevf/vf.h | 2 +
>>>>>>>> 7 files changed, 243 insertions(+), 1 deletion(-)
>>>>>>> Remember to CC on patches against drivers in
>>>>>> drivers/net/ethernet/intel/ please.
>>>>>>
>>>>>> To CC who? ;)
>>>>>>
>>>>> Sorry, I fat fingered that response. Please CC me.
>>>> Sure. NP.
>>>> Since we are talking already, could u comment on the RFC? Does it look
>>>> ok to you?
>>>>
>>>>
>>> I have provided feedback on 3 of the patches, the others look ok to me.
>>> I would like Greg Rose to look over your changes since he did most of
>>> the ixgbevf work, and I want to make sure that he is fine with these
>>> changes.
>> I saw them. Thanks, Jeff. I'll wait for Greg's comments before cooking
>> the final series then.
>>
>> Again, thanks for a rapid response.
>>
> Go ahead and send out a non-RFC series. Greg is on vacation till the
> 5th anyway, so I will pick up your series and have my validation team
> run it through their tests in the mean time.
Great! It's a real pleasure working with you!
^ permalink raw reply
* Re: [PATCH 0/4] ath10k: a few incorrect return handling fix-up
From: Nicholas Mc Guire @ 2014-12-30 19:42 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: Kalle Valo, Michal Kazior, Ben Greear, Chun-Yeow Yeoh, Yanbo Li,
ath10k, linux-wireless, netdev, linux-kernel
In-Reply-To: <54A2F151.6040205@cogentembedded.com>
On Tue, 30 Dec 2014, Sergei Shtylyov wrote:
> On 12/30/2014 09:28 PM, Nicholas Mc Guire wrote:
>
>>>> wait_for_completion_timeout does not return negative values so the tests
>>>> for <= 0 are not needed and the case differentiation in the error handling
>>>> path unnecessary.
>
>>> I decided to verify your statement and I saw that it seems wrong.
>>> do_wait_for_common() can return -ERESTARTSYS and the return value gets
>>> returned by its callers unchanged.
>
>> the -ERESTARTSYS only can be returned if state matches but
>> wait_for_completion_timemout passes TASK_UNINTERRUPTIBLE
>> so signal_pending_state will return 0 and never negativ
>
>> my understanding of the callchain is:
>> wait_for_completion_timemout with TASK_UNINTERRUPTIBLE
>> -> wait_for_common(...TASK_UNINTERRUPTIBLE)
>> -> __wait_for_common(...TASK_UNINTERRUPTIBLE)
>> -> do_wait_for_common(...TASK_UNINTERRUPTIBLE)
>> -> signal_pending_state(TASK_UNINTERRUPTIBLE...)
>
>> static inline int signal_pending_state(long state, struct task_struct *p)
>> {
>> if (!(state & (TASK_INTERRUPTIBLE | TASK_WAKEKILL)))
>> return 0;
>
> Right. I didn't look into TASK_UNINTERRUPTIBLE thing before sending my mail.
>
>> so wait_for_completion_timemout should return 0 or 1 only
>
> 0 or the remaining time, to be precise.
>
yup - thanks for the confirmation!
>>>> patch was only compile tested x86_64_defconfig + CONFIG_ATH_CARDS=m
>>>> CONFIG_ATH10K=m
>
>>>> patch is against linux-next 3.19.0-rc1 -next-20141226
>
>>> Rather patches. It would have been better to send one patch instead of
>>> 4 patches with the same name.
>
>> sorry for that - I had split it into separate patches as it was
>> in different files - giving them the same name of course was a bit
>> brain-dead.
>
> You should have mentioned the modified files in the subject. But IMHO
> it would be better to have just one patch.
>
resent as a single patch as v2
thx!
hofrat
^ permalink raw reply
* [PATCH v2] ath10k: fixup wait_for_completion_timeout return handling
From: Nicholas Mc Guire @ 2014-12-30 19:44 UTC (permalink / raw)
To: Kalle Valo
Cc: Michal Kazior, Ben Greear, Chun-Yeow Yeoh, Yanbo Li,
Sergei Shtylyov, ath10k, linux-wireless, netdev, linux-kernel,
Nicholas Mc Guire
wait_for_completion_timeout does not return negative values so the tests
for <= 0 are not needed and the case differentiation in the error handling
path unnecessary.
v2: all wait_for_completion_timeout changes in a single patch
patch was only compile tested x86_64_defconfig + CONFIG_ATH_CARDS=m
CONFIG_ATH10K=m
patch is against linux-next 3.19.0-rc1 -next-20141226
None of the proposed cleanups are critical.
All changes should only be removing unreachable cases.
Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---
drivers/net/wireless/ath/ath10k/debug.c | 2 +-
drivers/net/wireless/ath/ath10k/htc.c | 6 ++----
drivers/net/wireless/ath/ath10k/htt.c | 2 +-
drivers/net/wireless/ath/ath10k/mac.c | 2 +-
4 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index a716758..7e1fe93 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -373,7 +373,7 @@ static int ath10k_debug_fw_stats_request(struct ath10k *ar)
ret = wait_for_completion_timeout(&ar->debug.fw_stats_complete,
1*HZ);
- if (ret <= 0)
+ if (ret == 0)
return -ETIMEDOUT;
spin_lock_bh(&ar->data_lock);
diff --git a/drivers/net/wireless/ath/ath10k/htc.c b/drivers/net/wireless/ath/ath10k/htc.c
index f1946a6..2fd9e18 100644
--- a/drivers/net/wireless/ath/ath10k/htc.c
+++ b/drivers/net/wireless/ath/ath10k/htc.c
@@ -703,11 +703,9 @@ int ath10k_htc_connect_service(struct ath10k_htc *htc,
/* wait for response */
status = wait_for_completion_timeout(&htc->ctl_resp,
ATH10K_HTC_CONN_SVC_TIMEOUT_HZ);
- if (status <= 0) {
- if (status == 0)
- status = -ETIMEDOUT;
+ if (status == 0) {
ath10k_err(ar, "Service connect timeout: %d\n", status);
- return status;
+ return -ETIMEDOUT;
}
/* we controlled the buffer creation, it's aligned */
diff --git a/drivers/net/wireless/ath/ath10k/htt.c b/drivers/net/wireless/ath/ath10k/htt.c
index 56cb4ac..58b6fc1 100644
--- a/drivers/net/wireless/ath/ath10k/htt.c
+++ b/drivers/net/wireless/ath/ath10k/htt.c
@@ -102,7 +102,7 @@ int ath10k_htt_setup(struct ath10k_htt *htt)
status = wait_for_completion_timeout(&htt->target_version_received,
HTT_TARGET_VERSION_TIMEOUT_HZ);
- if (status <= 0) {
+ if (status == 0) {
ath10k_warn(ar, "htt version request timed out\n");
return -ETIMEDOUT;
}
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index c400567..f9d7dbb 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -2151,7 +2151,7 @@ void ath10k_offchan_tx_work(struct work_struct *work)
ret = wait_for_completion_timeout(&ar->offchan_tx_completed,
3 * HZ);
- if (ret <= 0)
+ if (ret == 0)
ath10k_warn(ar, "timed out waiting for offchannel skb %p\n",
skb);
--
1.7.10.4
^ permalink raw reply related
* Re: [Question] What's the noop_qdisc introduced for in the kernel?
From: Cong Wang @ 2014-12-30 20:15 UTC (permalink / raw)
To: Dennis Chen; +Cc: netdev
In-Reply-To: <CA+U0gVi=DTpz-Givbrs771+KYz8EE1ABdy2oO38q4LR9fG37YQ@mail.gmail.com>
On Tue, Dec 30, 2014 at 1:23 AM, Dennis Chen <kernel.org.gnu@gmail.com> wrote:
> After google and the code reading, seems this Qdisc instance is only
> used for the initialization purpose, I can't find the reason that this
> object introduced in the kernel. Does anybody know what the historical
> reason is for this invention? the purpose or the benefit for this
> Qdisc object?
>
Not just for initialization, it is kinda a null qdisc when
the previous qdisc gets removed:
/* ... and graft new one */
if (qdisc == NULL)
qdisc = &noop_qdisc;
or the entire device is not activated yet. It guarantees no
packets can be sent out via this qdisc.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox