| Message ID | 1427645497-8339-7-git-send-email-vladz@cloudius-systems.com |
|---|---|
| State | Awaiting Upstream, archived |
| Delegated to: | David Miller |
| Headers | show |
On 03/29/2015 09:11 AM, Vlad Zolotarov wrote: > Add the ixgbevf_get_rss_key() function that queries the PF for an RSS Random Key > using a new VF-PF channel IXGBE_VF_GET_RSS_KEY command. > > This patch adds the support for 82599 and x540 devices only. Support for other > devices will be added later. > > Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com> > --- > New in v9: > - Reduce the support to 82599 and x540 devices only. > - Added IXGBEVF_RSS_HASH_KEY_SIZE macro. > > New in v8: > - Protect a mailbox access. > > New in v6: > - Return a proper return code when an operation is blocked by PF. > > New in v2: > - Added a more detailed patch description. > > New in v1 (compared to RFC): > - Use "if-else" statement instead of a "switch-case" for a single option case > (in ixgbevf_get_rss_key()). > --- > drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 1 + > drivers/net/ethernet/intel/ixgbevf/mbx.h | 1 + > drivers/net/ethernet/intel/ixgbevf/vf.c | 66 ++++++++++++++++++++++++++++ > drivers/net/ethernet/intel/ixgbevf/vf.h | 1 + > 4 files changed, 69 insertions(+) > > diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h > index bc939a1..6771ae3 100644 > --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h > +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h > @@ -145,6 +145,7 @@ struct ixgbevf_ring { > #define MAX_RX_QUEUES IXGBE_VF_MAX_RX_QUEUES > #define MAX_TX_QUEUES IXGBE_VF_MAX_TX_QUEUES > #define IXGBEVF_MAX_RSS_QUEUES 2 > +#define IXGBEVF_RSS_HASH_KEY_SIZE 40 > > #define IXGBEVF_DEFAULT_TXD 1024 > #define IXGBEVF_DEFAULT_RXD 512 > diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h b/drivers/net/ethernet/intel/ixgbevf/mbx.h > index 66e138b..82f44e0 100644 > --- a/drivers/net/ethernet/intel/ixgbevf/mbx.h > +++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h > @@ -110,6 +110,7 @@ enum ixgbe_pfvf_api_rev { > > /* mailbox API, version 1.2 VF requests */ > #define IXGBE_VF_GET_RETA 0x0a /* VF request for RETA */ > +#define IXGBE_VF_GET_RSS_KEY 0x0b /* get RSS hash key */ > > /* length of permanent address message returned from PF */ > #define IXGBE_VF_PERMADDR_MSG_LEN 4 > diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c > index 2676c0b..ec68145 100644 > --- a/drivers/net/ethernet/intel/ixgbevf/vf.c > +++ b/drivers/net/ethernet/intel/ixgbevf/vf.c > @@ -301,6 +301,72 @@ static inline int ixgbevf_get_reta_locked(struct ixgbe_hw *hw, u32 *msgbuf, > return 0; > } > > +static inline int ixgbevf_get_rss_key_locked(struct ixgbe_hw *hw, u8 *rss_key) > +{ > + int err; > + u32 msgbuf[IXGBE_VFMAILBOX_SIZE]; > + > + /* We currently support the RSS Random Key retrieval for 82599 and x540 > + * devices only. > + * > + * Thus return an error if API doesn't support RSS Random Key retrieval > + * or if the operation is not supported for this device type. > + */ > + if (hw->api_version != ixgbe_mbox_api_12 || > + hw->mac.type >= ixgbe_mac_X550_vf) > + return -EPERM; > + The return type here should be not supported, or IXGBE_NOT_IMPLEMENTED, not no permissions. > + msgbuf[0] = IXGBE_VF_GET_RSS_KEY; > + err = hw->mbx.ops.write_posted(hw, msgbuf, 1); > + > + if (err) > + return err; > + > + err = hw->mbx.ops.read_posted(hw, msgbuf, 11); > + > + if (err) > + return err; > + > + msgbuf[0] &= ~IXGBE_VT_MSGTYPE_CTS; > + > + /* If the operation has been refused by a PF return -EPERM */ > + if (msgbuf[0] == (IXGBE_VF_GET_RETA | IXGBE_VT_MSGTYPE_NACK)) > + return -EPERM; > + > + /* If we didn't get an ACK there must have been > + * some sort of mailbox error so we should treat it > + * as such. > + */ > + if (msgbuf[0] != (IXGBE_VF_GET_RSS_KEY | IXGBE_VT_MSGTYPE_ACK)) > + return IXGBE_ERR_MBX; > + > + memcpy(rss_key, msgbuf + 1, IXGBEVF_RSS_HASH_KEY_SIZE); > + > + return 0; > +} > + > +/** > + * 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. > + * Ensures the atomicy of a mailbox access using the adapter.mbx_lock spinlock. > + * > + * Returns: 0 on success. > + * if API doesn't support this operation - (-EPERM). > + */ > +int ixgbevf_get_rss_key(struct ixgbevf_adapter *a, u8 *rss_key) > +{ > + int rc; > + > + spin_lock_bh(&a->mbx_lock); > + rc = ixgbevf_get_rss_key_locked(&a->hw, rss_key); > + spin_unlock_bh(&a->mbx_lock); > + > + return rc; > +} > + Since there is currently no cases where you would be getting the rss_key without the RETA you might just want to combine this function with ixgbevf_get_reta so you only take the lock once and process both messages in one pass instead of having to bounce the spinlock. > /** > * ixgbevf_get_reta - get the RSS redirection table (RETA) contents. > * @adapter: pointer to the port handle > diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.h b/drivers/net/ethernet/intel/ixgbevf/vf.h > index 7944962..a054716 100644 > --- a/drivers/net/ethernet/intel/ixgbevf/vf.h > +++ b/drivers/net/ethernet/intel/ixgbevf/vf.h > @@ -212,4 +212,5 @@ int ixgbevf_negotiate_api_version(struct ixgbe_hw *hw, int api); > int ixgbevf_get_queues(struct ixgbe_hw *hw, unsigned int *num_tcs, > unsigned int *default_tc); > int ixgbevf_get_reta(struct ixgbevf_adapter *adapter, u32 *reta); > +int ixgbevf_get_rss_key(struct ixgbevf_adapter *adapter, u8 *rss_key); > #endif /* __IXGBE_VF_H__ */ > -- 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
On 03/30/15 01:04, Alexander Duyck wrote: > On 03/29/2015 09:11 AM, Vlad Zolotarov wrote: >> Add the ixgbevf_get_rss_key() function that queries the PF for an RSS Random Key >> using a new VF-PF channel IXGBE_VF_GET_RSS_KEY command. >> >> This patch adds the support for 82599 and x540 devices only. Support for other >> devices will be added later. >> >> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com> >> --- >> New in v9: >> - Reduce the support to 82599 and x540 devices only. >> - Added IXGBEVF_RSS_HASH_KEY_SIZE macro. >> >> New in v8: >> - Protect a mailbox access. >> >> New in v6: >> - Return a proper return code when an operation is blocked by PF. >> >> New in v2: >> - Added a more detailed patch description. >> >> New in v1 (compared to RFC): >> - Use "if-else" statement instead of a "switch-case" for a single option case >> (in ixgbevf_get_rss_key()). >> --- >> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 1 + >> drivers/net/ethernet/intel/ixgbevf/mbx.h | 1 + >> drivers/net/ethernet/intel/ixgbevf/vf.c | 66 ++++++++++++++++++++++++++++ >> drivers/net/ethernet/intel/ixgbevf/vf.h | 1 + >> 4 files changed, 69 insertions(+) >> >> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >> index bc939a1..6771ae3 100644 >> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >> @@ -145,6 +145,7 @@ struct ixgbevf_ring { >> #define MAX_RX_QUEUES IXGBE_VF_MAX_RX_QUEUES >> #define MAX_TX_QUEUES IXGBE_VF_MAX_TX_QUEUES >> #define IXGBEVF_MAX_RSS_QUEUES 2 >> +#define IXGBEVF_RSS_HASH_KEY_SIZE 40 >> >> #define IXGBEVF_DEFAULT_TXD 1024 >> #define IXGBEVF_DEFAULT_RXD 512 >> diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h b/drivers/net/ethernet/intel/ixgbevf/mbx.h >> index 66e138b..82f44e0 100644 >> --- a/drivers/net/ethernet/intel/ixgbevf/mbx.h >> +++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h >> @@ -110,6 +110,7 @@ enum ixgbe_pfvf_api_rev { >> >> /* mailbox API, version 1.2 VF requests */ >> #define IXGBE_VF_GET_RETA 0x0a /* VF request for RETA */ >> +#define IXGBE_VF_GET_RSS_KEY 0x0b /* get RSS hash key */ >> >> /* length of permanent address message returned from PF */ >> #define IXGBE_VF_PERMADDR_MSG_LEN 4 >> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c >> index 2676c0b..ec68145 100644 >> --- a/drivers/net/ethernet/intel/ixgbevf/vf.c >> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.c >> @@ -301,6 +301,72 @@ static inline int ixgbevf_get_reta_locked(struct ixgbe_hw *hw, u32 *msgbuf, >> return 0; >> } >> >> +static inline int ixgbevf_get_rss_key_locked(struct ixgbe_hw *hw, u8 *rss_key) >> +{ >> + int err; >> + u32 msgbuf[IXGBE_VFMAILBOX_SIZE]; >> + >> + /* We currently support the RSS Random Key retrieval for 82599 and x540 >> + * devices only. >> + * >> + * Thus return an error if API doesn't support RSS Random Key retrieval >> + * or if the operation is not supported for this device type. >> + */ >> + if (hw->api_version != ixgbe_mbox_api_12 || >> + hw->mac.type >= ixgbe_mac_X550_vf) >> + return -EPERM; >> + > The return type here should be not supported, or IXGBE_NOT_IMPLEMENTED, > not no permissions. I think a standard EOPNOTSUPP would be a much better choice. Why do u have this ixgbe-specific return values in addition to standard POSIX ones anyway? > >> + msgbuf[0] = IXGBE_VF_GET_RSS_KEY; >> + err = hw->mbx.ops.write_posted(hw, msgbuf, 1); >> + >> + if (err) >> + return err; >> + >> + err = hw->mbx.ops.read_posted(hw, msgbuf, 11); >> + >> + if (err) >> + return err; >> + >> + msgbuf[0] &= ~IXGBE_VT_MSGTYPE_CTS; >> + >> + /* If the operation has been refused by a PF return -EPERM */ >> + if (msgbuf[0] == (IXGBE_VF_GET_RETA | IXGBE_VT_MSGTYPE_NACK)) >> + return -EPERM; >> + >> + /* If we didn't get an ACK there must have been >> + * some sort of mailbox error so we should treat it >> + * as such. >> + */ >> + if (msgbuf[0] != (IXGBE_VF_GET_RSS_KEY | IXGBE_VT_MSGTYPE_ACK)) >> + return IXGBE_ERR_MBX; >> + >> + memcpy(rss_key, msgbuf + 1, IXGBEVF_RSS_HASH_KEY_SIZE); >> + >> + return 0; >> +} >> + >> +/** >> + * 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. >> + * Ensures the atomicy of a mailbox access using the adapter.mbx_lock spinlock. >> + * >> + * Returns: 0 on success. >> + * if API doesn't support this operation - (-EPERM). >> + */ >> +int ixgbevf_get_rss_key(struct ixgbevf_adapter *a, u8 *rss_key) >> +{ >> + int rc; >> + >> + spin_lock_bh(&a->mbx_lock); >> + rc = ixgbevf_get_rss_key_locked(&a->hw, rss_key); >> + spin_unlock_bh(&a->mbx_lock); >> + >> + return rc; >> +} >> + > Since there is currently no cases where you would be getting the rss_key > without the RETA you might just want to combine this function with > ixgbevf_get_reta so you only take the lock once and process both > messages in one pass instead of having to bounce the spinlock. > >> /** >> * ixgbevf_get_reta - get the RSS redirection table (RETA) contents. >> * @adapter: pointer to the port handle >> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.h b/drivers/net/ethernet/intel/ixgbevf/vf.h >> index 7944962..a054716 100644 >> --- a/drivers/net/ethernet/intel/ixgbevf/vf.h >> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.h >> @@ -212,4 +212,5 @@ int ixgbevf_negotiate_api_version(struct ixgbe_hw *hw, int api); >> int ixgbevf_get_queues(struct ixgbe_hw *hw, unsigned int *num_tcs, >> unsigned int *default_tc); >> int ixgbevf_get_reta(struct ixgbevf_adapter *adapter, u32 *reta); >> +int ixgbevf_get_rss_key(struct ixgbevf_adapter *adapter, u8 *rss_key); >> #endif /* __IXGBE_VF_H__ */ >> -- 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
On 03/30/15 01:04, Alexander Duyck wrote: > On 03/29/2015 09:11 AM, Vlad Zolotarov wrote: >> Add the ixgbevf_get_rss_key() function that queries the PF for an RSS Random Key >> using a new VF-PF channel IXGBE_VF_GET_RSS_KEY command. >> >> This patch adds the support for 82599 and x540 devices only. Support for other >> devices will be added later. >> >> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com> >> --- >> New in v9: >> - Reduce the support to 82599 and x540 devices only. >> - Added IXGBEVF_RSS_HASH_KEY_SIZE macro. >> >> New in v8: >> - Protect a mailbox access. >> >> New in v6: >> - Return a proper return code when an operation is blocked by PF. >> >> New in v2: >> - Added a more detailed patch description. >> >> New in v1 (compared to RFC): >> - Use "if-else" statement instead of a "switch-case" for a single option case >> (in ixgbevf_get_rss_key()). >> --- >> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 1 + >> drivers/net/ethernet/intel/ixgbevf/mbx.h | 1 + >> drivers/net/ethernet/intel/ixgbevf/vf.c | 66 ++++++++++++++++++++++++++++ >> drivers/net/ethernet/intel/ixgbevf/vf.h | 1 + >> 4 files changed, 69 insertions(+) >> >> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >> index bc939a1..6771ae3 100644 >> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >> @@ -145,6 +145,7 @@ struct ixgbevf_ring { >> #define MAX_RX_QUEUES IXGBE_VF_MAX_RX_QUEUES >> #define MAX_TX_QUEUES IXGBE_VF_MAX_TX_QUEUES >> #define IXGBEVF_MAX_RSS_QUEUES 2 >> +#define IXGBEVF_RSS_HASH_KEY_SIZE 40 >> >> #define IXGBEVF_DEFAULT_TXD 1024 >> #define IXGBEVF_DEFAULT_RXD 512 >> diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h b/drivers/net/ethernet/intel/ixgbevf/mbx.h >> index 66e138b..82f44e0 100644 >> --- a/drivers/net/ethernet/intel/ixgbevf/mbx.h >> +++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h >> @@ -110,6 +110,7 @@ enum ixgbe_pfvf_api_rev { >> >> /* mailbox API, version 1.2 VF requests */ >> #define IXGBE_VF_GET_RETA 0x0a /* VF request for RETA */ >> +#define IXGBE_VF_GET_RSS_KEY 0x0b /* get RSS hash key */ >> >> /* length of permanent address message returned from PF */ >> #define IXGBE_VF_PERMADDR_MSG_LEN 4 >> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c >> index 2676c0b..ec68145 100644 >> --- a/drivers/net/ethernet/intel/ixgbevf/vf.c >> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.c >> @@ -301,6 +301,72 @@ static inline int ixgbevf_get_reta_locked(struct ixgbe_hw *hw, u32 *msgbuf, >> return 0; >> } >> >> +static inline int ixgbevf_get_rss_key_locked(struct ixgbe_hw *hw, u8 *rss_key) >> +{ >> + int err; >> + u32 msgbuf[IXGBE_VFMAILBOX_SIZE]; >> + >> + /* We currently support the RSS Random Key retrieval for 82599 and x540 >> + * devices only. >> + * >> + * Thus return an error if API doesn't support RSS Random Key retrieval >> + * or if the operation is not supported for this device type. >> + */ >> + if (hw->api_version != ixgbe_mbox_api_12 || >> + hw->mac.type >= ixgbe_mac_X550_vf) >> + return -EPERM; >> + > The return type here should be not supported, or IXGBE_NOT_IMPLEMENTED, > not no permissions. > >> + msgbuf[0] = IXGBE_VF_GET_RSS_KEY; >> + err = hw->mbx.ops.write_posted(hw, msgbuf, 1); >> + >> + if (err) >> + return err; >> + >> + err = hw->mbx.ops.read_posted(hw, msgbuf, 11); >> + >> + if (err) >> + return err; >> + >> + msgbuf[0] &= ~IXGBE_VT_MSGTYPE_CTS; >> + >> + /* If the operation has been refused by a PF return -EPERM */ >> + if (msgbuf[0] == (IXGBE_VF_GET_RETA | IXGBE_VT_MSGTYPE_NACK)) >> + return -EPERM; >> + >> + /* If we didn't get an ACK there must have been >> + * some sort of mailbox error so we should treat it >> + * as such. >> + */ >> + if (msgbuf[0] != (IXGBE_VF_GET_RSS_KEY | IXGBE_VT_MSGTYPE_ACK)) >> + return IXGBE_ERR_MBX; >> + >> + memcpy(rss_key, msgbuf + 1, IXGBEVF_RSS_HASH_KEY_SIZE); >> + >> + return 0; >> +} >> + >> +/** >> + * 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. >> + * Ensures the atomicy of a mailbox access using the adapter.mbx_lock spinlock. >> + * >> + * Returns: 0 on success. >> + * if API doesn't support this operation - (-EPERM). >> + */ >> +int ixgbevf_get_rss_key(struct ixgbevf_adapter *a, u8 *rss_key) >> +{ >> + int rc; >> + >> + spin_lock_bh(&a->mbx_lock); >> + rc = ixgbevf_get_rss_key_locked(&a->hw, rss_key); >> + spin_unlock_bh(&a->mbx_lock); >> + >> + return rc; >> +} >> + > Since there is currently no cases where you would be getting the rss_key > without the RETA you might just want to combine this function with > ixgbevf_get_reta so you only take the lock once and process both > messages in one pass instead of having to bounce the spinlock. I'd rather not do this. Let's start from the beginning: the locks here should be removed and added at the caller level to match what u wrote about patch04. Then about the uniting the two functions mentioned above - there isn't any added value of doing this. Taking a lock only once may be done without uniting (see PATCH07 in v10 series). On the other hand, this uniting is going to make the code awkward and unclear ("why are these together anyway?"). So, I'd rather keep these functions as they are apart from fixing the locking issue. I'll essentially export the _locked functions. thanks, vlad > >> /** >> * ixgbevf_get_reta - get the RSS redirection table (RETA) contents. >> * @adapter: pointer to the port handle >> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.h b/drivers/net/ethernet/intel/ixgbevf/vf.h >> index 7944962..a054716 100644 >> --- a/drivers/net/ethernet/intel/ixgbevf/vf.h >> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.h >> @@ -212,4 +212,5 @@ int ixgbevf_negotiate_api_version(struct ixgbe_hw *hw, int api); >> int ixgbevf_get_queues(struct ixgbe_hw *hw, unsigned int *num_tcs, >> unsigned int *default_tc); >> int ixgbevf_get_reta(struct ixgbevf_adapter *adapter, u32 *reta); >> +int ixgbevf_get_rss_key(struct ixgbevf_adapter *adapter, u8 *rss_key); >> #endif /* __IXGBE_VF_H__ */ >> -- 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
On 03/30/2015 05:38 AM, Vlad Zolotarov wrote: > > > On 03/30/15 01:04, Alexander Duyck wrote: >> On 03/29/2015 09:11 AM, Vlad Zolotarov wrote: >>> Add the ixgbevf_get_rss_key() function that queries the PF for an >>> RSS Random Key >>> using a new VF-PF channel IXGBE_VF_GET_RSS_KEY command. >>> >>> This patch adds the support for 82599 and x540 devices only. Support >>> for other >>> devices will be added later. >>> >>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com> >>> --- >>> New in v9: >>> - Reduce the support to 82599 and x540 devices only. >>> - Added IXGBEVF_RSS_HASH_KEY_SIZE macro. >>> >>> New in v8: >>> - Protect a mailbox access. >>> >>> New in v6: >>> - Return a proper return code when an operation is blocked by PF. >>> >>> New in v2: >>> - Added a more detailed patch description. >>> >>> New in v1 (compared to RFC): >>> - Use "if-else" statement instead of a "switch-case" for a >>> single option case >>> (in ixgbevf_get_rss_key()). >>> --- >>> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 1 + >>> drivers/net/ethernet/intel/ixgbevf/mbx.h | 1 + >>> drivers/net/ethernet/intel/ixgbevf/vf.c | 66 >>> ++++++++++++++++++++++++++++ >>> drivers/net/ethernet/intel/ixgbevf/vf.h | 1 + >>> 4 files changed, 69 insertions(+) >>> >>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>> index bc939a1..6771ae3 100644 >>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>> @@ -145,6 +145,7 @@ struct ixgbevf_ring { >>> #define MAX_RX_QUEUES IXGBE_VF_MAX_RX_QUEUES >>> #define MAX_TX_QUEUES IXGBE_VF_MAX_TX_QUEUES >>> #define IXGBEVF_MAX_RSS_QUEUES 2 >>> +#define IXGBEVF_RSS_HASH_KEY_SIZE 40 >>> #define IXGBEVF_DEFAULT_TXD 1024 >>> #define IXGBEVF_DEFAULT_RXD 512 >>> diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h >>> b/drivers/net/ethernet/intel/ixgbevf/mbx.h >>> index 66e138b..82f44e0 100644 >>> --- a/drivers/net/ethernet/intel/ixgbevf/mbx.h >>> +++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h >>> @@ -110,6 +110,7 @@ enum ixgbe_pfvf_api_rev { >>> /* mailbox API, version 1.2 VF requests */ >>> #define IXGBE_VF_GET_RETA 0x0a /* VF request for RETA */ >>> +#define IXGBE_VF_GET_RSS_KEY 0x0b /* get RSS hash key */ >>> /* length of permanent address message returned from PF */ >>> #define IXGBE_VF_PERMADDR_MSG_LEN 4 >>> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c >>> b/drivers/net/ethernet/intel/ixgbevf/vf.c >>> index 2676c0b..ec68145 100644 >>> --- a/drivers/net/ethernet/intel/ixgbevf/vf.c >>> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.c >>> @@ -301,6 +301,72 @@ static inline int >>> ixgbevf_get_reta_locked(struct ixgbe_hw *hw, u32 *msgbuf, >>> return 0; >>> } >>> +static inline int ixgbevf_get_rss_key_locked(struct ixgbe_hw *hw, >>> u8 *rss_key) >>> +{ >>> + int err; >>> + u32 msgbuf[IXGBE_VFMAILBOX_SIZE]; >>> + >>> + /* We currently support the RSS Random Key retrieval for 82599 >>> and x540 >>> + * devices only. >>> + * >>> + * Thus return an error if API doesn't support RSS Random Key >>> retrieval >>> + * or if the operation is not supported for this device type. >>> + */ >>> + if (hw->api_version != ixgbe_mbox_api_12 || >>> + hw->mac.type >= ixgbe_mac_X550_vf) >>> + return -EPERM; >>> + >> The return type here should be not supported, or IXGBE_NOT_IMPLEMENTED, >> not no permissions. > > I think a standard EOPNOTSUPP would be a much better choice. > Why do u have this ixgbe-specific return values in addition to > standard POSIX ones anyway? It has to do with being OS agnostic. If you want to port this to BSD, Linux, DPDK, etc, then you can't always rely on all the error return values being there so there is a tendency to implement your own. As with the other function you might be best off just returning IXGBE_ERR_MBX since the x550 and API_12 don't support the RSS messages. - Alex -- 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
On 03/30/2015 06:53 AM, Vlad Zolotarov wrote: > > > On 03/30/15 01:04, Alexander Duyck wrote: >> On 03/29/2015 09:11 AM, Vlad Zolotarov wrote: >>> Add the ixgbevf_get_rss_key() function that queries the PF for an >>> RSS Random Key >>> using a new VF-PF channel IXGBE_VF_GET_RSS_KEY command. >>> >>> This patch adds the support for 82599 and x540 devices only. Support >>> for other >>> devices will be added later. >>> >>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com> >>> --- >>> New in v9: >>> - Reduce the support to 82599 and x540 devices only. >>> - Added IXGBEVF_RSS_HASH_KEY_SIZE macro. >>> >>> New in v8: >>> - Protect a mailbox access. >>> >>> New in v6: >>> - Return a proper return code when an operation is blocked by PF. >>> >>> New in v2: >>> - Added a more detailed patch description. >>> >>> New in v1 (compared to RFC): >>> - Use "if-else" statement instead of a "switch-case" for a >>> single option case >>> (in ixgbevf_get_rss_key()). >>> --- >>> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 1 + >>> drivers/net/ethernet/intel/ixgbevf/mbx.h | 1 + >>> drivers/net/ethernet/intel/ixgbevf/vf.c | 66 >>> ++++++++++++++++++++++++++++ >>> drivers/net/ethernet/intel/ixgbevf/vf.h | 1 + >>> 4 files changed, 69 insertions(+) >>> >>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>> index bc939a1..6771ae3 100644 >>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>> @@ -145,6 +145,7 @@ struct ixgbevf_ring { >>> #define MAX_RX_QUEUES IXGBE_VF_MAX_RX_QUEUES >>> #define MAX_TX_QUEUES IXGBE_VF_MAX_TX_QUEUES >>> #define IXGBEVF_MAX_RSS_QUEUES 2 >>> +#define IXGBEVF_RSS_HASH_KEY_SIZE 40 >>> #define IXGBEVF_DEFAULT_TXD 1024 >>> #define IXGBEVF_DEFAULT_RXD 512 >>> diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h >>> b/drivers/net/ethernet/intel/ixgbevf/mbx.h >>> index 66e138b..82f44e0 100644 >>> --- a/drivers/net/ethernet/intel/ixgbevf/mbx.h >>> +++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h >>> @@ -110,6 +110,7 @@ enum ixgbe_pfvf_api_rev { >>> /* mailbox API, version 1.2 VF requests */ >>> #define IXGBE_VF_GET_RETA 0x0a /* VF request for RETA */ >>> +#define IXGBE_VF_GET_RSS_KEY 0x0b /* get RSS hash key */ >>> /* length of permanent address message returned from PF */ >>> #define IXGBE_VF_PERMADDR_MSG_LEN 4 >>> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c >>> b/drivers/net/ethernet/intel/ixgbevf/vf.c >>> index 2676c0b..ec68145 100644 >>> --- a/drivers/net/ethernet/intel/ixgbevf/vf.c >>> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.c >>> @@ -301,6 +301,72 @@ static inline int >>> ixgbevf_get_reta_locked(struct ixgbe_hw *hw, u32 *msgbuf, >>> return 0; >>> } >>> +static inline int ixgbevf_get_rss_key_locked(struct ixgbe_hw *hw, >>> u8 *rss_key) >>> +{ >>> + int err; >>> + u32 msgbuf[IXGBE_VFMAILBOX_SIZE]; >>> + >>> + /* We currently support the RSS Random Key retrieval for 82599 >>> and x540 >>> + * devices only. >>> + * >>> + * Thus return an error if API doesn't support RSS Random Key >>> retrieval >>> + * or if the operation is not supported for this device type. >>> + */ >>> + if (hw->api_version != ixgbe_mbox_api_12 || >>> + hw->mac.type >= ixgbe_mac_X550_vf) >>> + return -EPERM; >>> + >> The return type here should be not supported, or IXGBE_NOT_IMPLEMENTED, >> not no permissions. >> >>> + msgbuf[0] = IXGBE_VF_GET_RSS_KEY; >>> + err = hw->mbx.ops.write_posted(hw, msgbuf, 1); >>> + >>> + if (err) >>> + return err; >>> + >>> + err = hw->mbx.ops.read_posted(hw, msgbuf, 11); >>> + >>> + if (err) >>> + return err; >>> + >>> + msgbuf[0] &= ~IXGBE_VT_MSGTYPE_CTS; >>> + >>> + /* If the operation has been refused by a PF return -EPERM */ >>> + if (msgbuf[0] == (IXGBE_VF_GET_RETA | IXGBE_VT_MSGTYPE_NACK)) >>> + return -EPERM; >>> + >>> + /* If we didn't get an ACK there must have been >>> + * some sort of mailbox error so we should treat it >>> + * as such. >>> + */ >>> + if (msgbuf[0] != (IXGBE_VF_GET_RSS_KEY | IXGBE_VT_MSGTYPE_ACK)) >>> + return IXGBE_ERR_MBX; >>> + >>> + memcpy(rss_key, msgbuf + 1, IXGBEVF_RSS_HASH_KEY_SIZE); >>> + >>> + return 0; >>> +} >>> + >>> +/** >>> + * 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. >>> + * Ensures the atomicy of a mailbox access using the >>> adapter.mbx_lock spinlock. >>> + * >>> + * Returns: 0 on success. >>> + * if API doesn't support this operation - (-EPERM). >>> + */ >>> +int ixgbevf_get_rss_key(struct ixgbevf_adapter *a, u8 *rss_key) >>> +{ >>> + int rc; >>> + >>> + spin_lock_bh(&a->mbx_lock); >>> + rc = ixgbevf_get_rss_key_locked(&a->hw, rss_key); >>> + spin_unlock_bh(&a->mbx_lock); >>> + >>> + return rc; >>> +} >>> + >> Since there is currently no cases where you would be getting the rss_key >> without the RETA you might just want to combine this function with >> ixgbevf_get_reta so you only take the lock once and process both >> messages in one pass instead of having to bounce the spinlock. > > I'd rather not do this. > Let's start from the beginning: the locks here should be removed and > added at the caller level to match what u wrote about patch04. > Then about the uniting the two functions mentioned above - there isn't > any added value of doing this. Taking a lock only once may be done > without uniting (see PATCH07 in v10 series). > On the other hand, this uniting is going to make the code awkward and > unclear ("why are these together anyway?"). > So, I'd rather keep these functions as they are apart from fixing the > locking issue. I'll essentially export the _locked functions. > > thanks, > vlad Agreed, just export the _locked functions and drop the _locked extension. - Alex -- 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
On 03/30/15 18:10, Alexander Duyck wrote: > > On 03/30/2015 06:53 AM, Vlad Zolotarov wrote: >> >> >> On 03/30/15 01:04, Alexander Duyck wrote: >>> On 03/29/2015 09:11 AM, Vlad Zolotarov wrote: >>>> Add the ixgbevf_get_rss_key() function that queries the PF for an >>>> RSS Random Key >>>> using a new VF-PF channel IXGBE_VF_GET_RSS_KEY command. >>>> >>>> This patch adds the support for 82599 and x540 devices only. >>>> Support for other >>>> devices will be added later. >>>> >>>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com> >>>> --- >>>> New in v9: >>>> - Reduce the support to 82599 and x540 devices only. >>>> - Added IXGBEVF_RSS_HASH_KEY_SIZE macro. >>>> >>>> New in v8: >>>> - Protect a mailbox access. >>>> >>>> New in v6: >>>> - Return a proper return code when an operation is blocked by PF. >>>> >>>> New in v2: >>>> - Added a more detailed patch description. >>>> >>>> New in v1 (compared to RFC): >>>> - Use "if-else" statement instead of a "switch-case" for a >>>> single option case >>>> (in ixgbevf_get_rss_key()). >>>> --- >>>> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 1 + >>>> drivers/net/ethernet/intel/ixgbevf/mbx.h | 1 + >>>> drivers/net/ethernet/intel/ixgbevf/vf.c | 66 >>>> ++++++++++++++++++++++++++++ >>>> drivers/net/ethernet/intel/ixgbevf/vf.h | 1 + >>>> 4 files changed, 69 insertions(+) >>>> >>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>> index bc939a1..6771ae3 100644 >>>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>> @@ -145,6 +145,7 @@ struct ixgbevf_ring { >>>> #define MAX_RX_QUEUES IXGBE_VF_MAX_RX_QUEUES >>>> #define MAX_TX_QUEUES IXGBE_VF_MAX_TX_QUEUES >>>> #define IXGBEVF_MAX_RSS_QUEUES 2 >>>> +#define IXGBEVF_RSS_HASH_KEY_SIZE 40 >>>> #define IXGBEVF_DEFAULT_TXD 1024 >>>> #define IXGBEVF_DEFAULT_RXD 512 >>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>> b/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>> index 66e138b..82f44e0 100644 >>>> --- a/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>> +++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>> @@ -110,6 +110,7 @@ enum ixgbe_pfvf_api_rev { >>>> /* mailbox API, version 1.2 VF requests */ >>>> #define IXGBE_VF_GET_RETA 0x0a /* VF request for RETA */ >>>> +#define IXGBE_VF_GET_RSS_KEY 0x0b /* get RSS hash key */ >>>> /* length of permanent address message returned from PF */ >>>> #define IXGBE_VF_PERMADDR_MSG_LEN 4 >>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c >>>> b/drivers/net/ethernet/intel/ixgbevf/vf.c >>>> index 2676c0b..ec68145 100644 >>>> --- a/drivers/net/ethernet/intel/ixgbevf/vf.c >>>> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.c >>>> @@ -301,6 +301,72 @@ static inline int >>>> ixgbevf_get_reta_locked(struct ixgbe_hw *hw, u32 *msgbuf, >>>> return 0; >>>> } >>>> +static inline int ixgbevf_get_rss_key_locked(struct ixgbe_hw >>>> *hw, u8 *rss_key) >>>> +{ >>>> + int err; >>>> + u32 msgbuf[IXGBE_VFMAILBOX_SIZE]; >>>> + >>>> + /* We currently support the RSS Random Key retrieval for 82599 >>>> and x540 >>>> + * devices only. >>>> + * >>>> + * Thus return an error if API doesn't support RSS Random Key >>>> retrieval >>>> + * or if the operation is not supported for this device type. >>>> + */ >>>> + if (hw->api_version != ixgbe_mbox_api_12 || >>>> + hw->mac.type >= ixgbe_mac_X550_vf) >>>> + return -EPERM; >>>> + >>> The return type here should be not supported, or IXGBE_NOT_IMPLEMENTED, >>> not no permissions. >>> >>>> + msgbuf[0] = IXGBE_VF_GET_RSS_KEY; >>>> + err = hw->mbx.ops.write_posted(hw, msgbuf, 1); >>>> + >>>> + if (err) >>>> + return err; >>>> + >>>> + err = hw->mbx.ops.read_posted(hw, msgbuf, 11); >>>> + >>>> + if (err) >>>> + return err; >>>> + >>>> + msgbuf[0] &= ~IXGBE_VT_MSGTYPE_CTS; >>>> + >>>> + /* If the operation has been refused by a PF return -EPERM */ >>>> + if (msgbuf[0] == (IXGBE_VF_GET_RETA | IXGBE_VT_MSGTYPE_NACK)) >>>> + return -EPERM; >>>> + >>>> + /* If we didn't get an ACK there must have been >>>> + * some sort of mailbox error so we should treat it >>>> + * as such. >>>> + */ >>>> + if (msgbuf[0] != (IXGBE_VF_GET_RSS_KEY | IXGBE_VT_MSGTYPE_ACK)) >>>> + return IXGBE_ERR_MBX; >>>> + >>>> + memcpy(rss_key, msgbuf + 1, IXGBEVF_RSS_HASH_KEY_SIZE); >>>> + >>>> + return 0; >>>> +} >>>> + >>>> +/** >>>> + * 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. >>>> + * Ensures the atomicy of a mailbox access using the >>>> adapter.mbx_lock spinlock. >>>> + * >>>> + * Returns: 0 on success. >>>> + * if API doesn't support this operation - (-EPERM). >>>> + */ >>>> +int ixgbevf_get_rss_key(struct ixgbevf_adapter *a, u8 *rss_key) >>>> +{ >>>> + int rc; >>>> + >>>> + spin_lock_bh(&a->mbx_lock); >>>> + rc = ixgbevf_get_rss_key_locked(&a->hw, rss_key); >>>> + spin_unlock_bh(&a->mbx_lock); >>>> + >>>> + return rc; >>>> +} >>>> + >>> Since there is currently no cases where you would be getting the >>> rss_key >>> without the RETA you might just want to combine this function with >>> ixgbevf_get_reta so you only take the lock once and process both >>> messages in one pass instead of having to bounce the spinlock. >> >> I'd rather not do this. >> Let's start from the beginning: the locks here should be removed and >> added at the caller level to match what u wrote about patch04. >> Then about the uniting the two functions mentioned above - there isn't >> any added value of doing this. Taking a lock only once may be done >> without uniting (see PATCH07 in v10 series). >> On the other hand, this uniting is going to make the code awkward and >> unclear ("why are these together anyway?"). >> So, I'd rather keep these functions as they are apart from fixing the >> locking issue. I'll essentially export the _locked functions. >> >> thanks, >> vlad > > Agreed, just export the _locked functions and drop the _locked extension. Hmmm... I think keeping the _locked extension would make sense since it would explicitly hint that these functions have to be called under the lock. There are other similar examples in the ixgbe/ixgbevf code already. > > - Alex -- 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
On 03/30/15 18:07, Alexander Duyck wrote: > > On 03/30/2015 05:38 AM, Vlad Zolotarov wrote: >> >> >> On 03/30/15 01:04, Alexander Duyck wrote: >>> On 03/29/2015 09:11 AM, Vlad Zolotarov wrote: >>>> Add the ixgbevf_get_rss_key() function that queries the PF for an >>>> RSS Random Key >>>> using a new VF-PF channel IXGBE_VF_GET_RSS_KEY command. >>>> >>>> This patch adds the support for 82599 and x540 devices only. >>>> Support for other >>>> devices will be added later. >>>> >>>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com> >>>> --- >>>> New in v9: >>>> - Reduce the support to 82599 and x540 devices only. >>>> - Added IXGBEVF_RSS_HASH_KEY_SIZE macro. >>>> >>>> New in v8: >>>> - Protect a mailbox access. >>>> >>>> New in v6: >>>> - Return a proper return code when an operation is blocked by PF. >>>> >>>> New in v2: >>>> - Added a more detailed patch description. >>>> >>>> New in v1 (compared to RFC): >>>> - Use "if-else" statement instead of a "switch-case" for a >>>> single option case >>>> (in ixgbevf_get_rss_key()). >>>> --- >>>> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 1 + >>>> drivers/net/ethernet/intel/ixgbevf/mbx.h | 1 + >>>> drivers/net/ethernet/intel/ixgbevf/vf.c | 66 >>>> ++++++++++++++++++++++++++++ >>>> drivers/net/ethernet/intel/ixgbevf/vf.h | 1 + >>>> 4 files changed, 69 insertions(+) >>>> >>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>> index bc939a1..6771ae3 100644 >>>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>> @@ -145,6 +145,7 @@ struct ixgbevf_ring { >>>> #define MAX_RX_QUEUES IXGBE_VF_MAX_RX_QUEUES >>>> #define MAX_TX_QUEUES IXGBE_VF_MAX_TX_QUEUES >>>> #define IXGBEVF_MAX_RSS_QUEUES 2 >>>> +#define IXGBEVF_RSS_HASH_KEY_SIZE 40 >>>> #define IXGBEVF_DEFAULT_TXD 1024 >>>> #define IXGBEVF_DEFAULT_RXD 512 >>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>> b/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>> index 66e138b..82f44e0 100644 >>>> --- a/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>> +++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>> @@ -110,6 +110,7 @@ enum ixgbe_pfvf_api_rev { >>>> /* mailbox API, version 1.2 VF requests */ >>>> #define IXGBE_VF_GET_RETA 0x0a /* VF request for RETA */ >>>> +#define IXGBE_VF_GET_RSS_KEY 0x0b /* get RSS hash key */ >>>> /* length of permanent address message returned from PF */ >>>> #define IXGBE_VF_PERMADDR_MSG_LEN 4 >>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c >>>> b/drivers/net/ethernet/intel/ixgbevf/vf.c >>>> index 2676c0b..ec68145 100644 >>>> --- a/drivers/net/ethernet/intel/ixgbevf/vf.c >>>> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.c >>>> @@ -301,6 +301,72 @@ static inline int >>>> ixgbevf_get_reta_locked(struct ixgbe_hw *hw, u32 *msgbuf, >>>> return 0; >>>> } >>>> +static inline int ixgbevf_get_rss_key_locked(struct ixgbe_hw >>>> *hw, u8 *rss_key) >>>> +{ >>>> + int err; >>>> + u32 msgbuf[IXGBE_VFMAILBOX_SIZE]; >>>> + >>>> + /* We currently support the RSS Random Key retrieval for 82599 >>>> and x540 >>>> + * devices only. >>>> + * >>>> + * Thus return an error if API doesn't support RSS Random Key >>>> retrieval >>>> + * or if the operation is not supported for this device type. >>>> + */ >>>> + if (hw->api_version != ixgbe_mbox_api_12 || >>>> + hw->mac.type >= ixgbe_mac_X550_vf) >>>> + return -EPERM; >>>> + >>> The return type here should be not supported, or IXGBE_NOT_IMPLEMENTED, >>> not no permissions. >> >> I think a standard EOPNOTSUPP would be a much better choice. >> Why do u have this ixgbe-specific return values in addition to >> standard POSIX ones anyway? > > It has to do with being OS agnostic. If you want to port this to BSD, > Linux, DPDK, etc, then you can't always rely on all the error return > values being there so there is a tendency to implement your own. As > with the other function you might be best off just returning > IXGBE_ERR_MBX since the x550 and API_12 don't support the RSS messages. EOPNOTSUPP is a standard POSIX.1 error code. It's defined both in Linux and in FreeBSD. As far as I know DPDK currently supports either Linux or BSD targets. So, for all mentioned above options there should not be any problem with this error code at all and in the scope of these OSes/SDKs the new lines are absolutely portable. > > - Alex -- 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
On 03/30/15 18:07, Alexander Duyck wrote: > > On 03/30/2015 05:38 AM, Vlad Zolotarov wrote: >> >> >> On 03/30/15 01:04, Alexander Duyck wrote: >>> On 03/29/2015 09:11 AM, Vlad Zolotarov wrote: >>>> Add the ixgbevf_get_rss_key() function that queries the PF for an >>>> RSS Random Key >>>> using a new VF-PF channel IXGBE_VF_GET_RSS_KEY command. >>>> >>>> This patch adds the support for 82599 and x540 devices only. >>>> Support for other >>>> devices will be added later. >>>> >>>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com> >>>> --- >>>> New in v9: >>>> - Reduce the support to 82599 and x540 devices only. >>>> - Added IXGBEVF_RSS_HASH_KEY_SIZE macro. >>>> >>>> New in v8: >>>> - Protect a mailbox access. >>>> >>>> New in v6: >>>> - Return a proper return code when an operation is blocked by PF. >>>> >>>> New in v2: >>>> - Added a more detailed patch description. >>>> >>>> New in v1 (compared to RFC): >>>> - Use "if-else" statement instead of a "switch-case" for a >>>> single option case >>>> (in ixgbevf_get_rss_key()). >>>> --- >>>> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 1 + >>>> drivers/net/ethernet/intel/ixgbevf/mbx.h | 1 + >>>> drivers/net/ethernet/intel/ixgbevf/vf.c | 66 >>>> ++++++++++++++++++++++++++++ >>>> drivers/net/ethernet/intel/ixgbevf/vf.h | 1 + >>>> 4 files changed, 69 insertions(+) >>>> >>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>> index bc939a1..6771ae3 100644 >>>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>> @@ -145,6 +145,7 @@ struct ixgbevf_ring { >>>> #define MAX_RX_QUEUES IXGBE_VF_MAX_RX_QUEUES >>>> #define MAX_TX_QUEUES IXGBE_VF_MAX_TX_QUEUES >>>> #define IXGBEVF_MAX_RSS_QUEUES 2 >>>> +#define IXGBEVF_RSS_HASH_KEY_SIZE 40 >>>> #define IXGBEVF_DEFAULT_TXD 1024 >>>> #define IXGBEVF_DEFAULT_RXD 512 >>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>> b/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>> index 66e138b..82f44e0 100644 >>>> --- a/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>> +++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>> @@ -110,6 +110,7 @@ enum ixgbe_pfvf_api_rev { >>>> /* mailbox API, version 1.2 VF requests */ >>>> #define IXGBE_VF_GET_RETA 0x0a /* VF request for RETA */ >>>> +#define IXGBE_VF_GET_RSS_KEY 0x0b /* get RSS hash key */ >>>> /* length of permanent address message returned from PF */ >>>> #define IXGBE_VF_PERMADDR_MSG_LEN 4 >>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c >>>> b/drivers/net/ethernet/intel/ixgbevf/vf.c >>>> index 2676c0b..ec68145 100644 >>>> --- a/drivers/net/ethernet/intel/ixgbevf/vf.c >>>> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.c >>>> @@ -301,6 +301,72 @@ static inline int >>>> ixgbevf_get_reta_locked(struct ixgbe_hw *hw, u32 *msgbuf, >>>> return 0; >>>> } >>>> +static inline int ixgbevf_get_rss_key_locked(struct ixgbe_hw >>>> *hw, u8 *rss_key) >>>> +{ >>>> + int err; >>>> + u32 msgbuf[IXGBE_VFMAILBOX_SIZE]; >>>> + >>>> + /* We currently support the RSS Random Key retrieval for 82599 >>>> and x540 >>>> + * devices only. >>>> + * >>>> + * Thus return an error if API doesn't support RSS Random Key >>>> retrieval >>>> + * or if the operation is not supported for this device type. >>>> + */ >>>> + if (hw->api_version != ixgbe_mbox_api_12 || >>>> + hw->mac.type >= ixgbe_mac_X550_vf) >>>> + return -EPERM; >>>> + >>> The return type here should be not supported, or IXGBE_NOT_IMPLEMENTED, >>> not no permissions. >> >> I think a standard EOPNOTSUPP would be a much better choice. >> Why do u have this ixgbe-specific return values in addition to >> standard POSIX ones anyway? > > It has to do with being OS agnostic. If you want to port this to BSD, > Linux, DPDK, etc, then you can't always rely on all the error return > values being there so there is a tendency to implement your own. As > with the other function you might be best off just returning > IXGBE_ERR_MBX since the x550 and API_12 don't support the RSS messages. Returning the IXGBE_ERR_MBX from the ethtool callback would be the most confusing since it is defined to -100 which corresponds to ENETDOWN value in Linux. This would result in the most confusing ethtool error message. > > - Alex -- 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
On 03/30/2015 08:17 AM, Vlad Zolotarov wrote: > > > On 03/30/15 18:10, Alexander Duyck wrote: >> >> On 03/30/2015 06:53 AM, Vlad Zolotarov wrote: >>> >>> >>> On 03/30/15 01:04, Alexander Duyck wrote: >>>> On 03/29/2015 09:11 AM, Vlad Zolotarov wrote: >>>>> Add the ixgbevf_get_rss_key() function that queries the PF for an >>>>> RSS Random Key >>>>> using a new VF-PF channel IXGBE_VF_GET_RSS_KEY command. >>>>> >>>>> This patch adds the support for 82599 and x540 devices only. >>>>> Support for other >>>>> devices will be added later. >>>>> >>>>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com> >>>>> --- >>>>> New in v9: >>>>> - Reduce the support to 82599 and x540 devices only. >>>>> - Added IXGBEVF_RSS_HASH_KEY_SIZE macro. >>>>> >>>>> New in v8: >>>>> - Protect a mailbox access. >>>>> >>>>> New in v6: >>>>> - Return a proper return code when an operation is blocked by PF. >>>>> >>>>> New in v2: >>>>> - Added a more detailed patch description. >>>>> >>>>> New in v1 (compared to RFC): >>>>> - Use "if-else" statement instead of a "switch-case" for a >>>>> single option case >>>>> (in ixgbevf_get_rss_key()). >>>>> --- >>>>> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 1 + >>>>> drivers/net/ethernet/intel/ixgbevf/mbx.h | 1 + >>>>> drivers/net/ethernet/intel/ixgbevf/vf.c | 66 >>>>> ++++++++++++++++++++++++++++ >>>>> drivers/net/ethernet/intel/ixgbevf/vf.h | 1 + >>>>> 4 files changed, 69 insertions(+) >>>>> >>>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>>> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>>> index bc939a1..6771ae3 100644 >>>>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>>> @@ -145,6 +145,7 @@ struct ixgbevf_ring { >>>>> #define MAX_RX_QUEUES IXGBE_VF_MAX_RX_QUEUES >>>>> #define MAX_TX_QUEUES IXGBE_VF_MAX_TX_QUEUES >>>>> #define IXGBEVF_MAX_RSS_QUEUES 2 >>>>> +#define IXGBEVF_RSS_HASH_KEY_SIZE 40 >>>>> #define IXGBEVF_DEFAULT_TXD 1024 >>>>> #define IXGBEVF_DEFAULT_RXD 512 >>>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>>> b/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>>> index 66e138b..82f44e0 100644 >>>>> --- a/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>>> +++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>>> @@ -110,6 +110,7 @@ enum ixgbe_pfvf_api_rev { >>>>> /* mailbox API, version 1.2 VF requests */ >>>>> #define IXGBE_VF_GET_RETA 0x0a /* VF request for RETA */ >>>>> +#define IXGBE_VF_GET_RSS_KEY 0x0b /* get RSS hash key */ >>>>> /* length of permanent address message returned from PF */ >>>>> #define IXGBE_VF_PERMADDR_MSG_LEN 4 >>>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c >>>>> b/drivers/net/ethernet/intel/ixgbevf/vf.c >>>>> index 2676c0b..ec68145 100644 >>>>> --- a/drivers/net/ethernet/intel/ixgbevf/vf.c >>>>> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.c >>>>> @@ -301,6 +301,72 @@ static inline int >>>>> ixgbevf_get_reta_locked(struct ixgbe_hw *hw, u32 *msgbuf, >>>>> return 0; >>>>> } >>>>> +static inline int ixgbevf_get_rss_key_locked(struct ixgbe_hw >>>>> *hw, u8 *rss_key) >>>>> +{ >>>>> + int err; >>>>> + u32 msgbuf[IXGBE_VFMAILBOX_SIZE]; >>>>> + >>>>> + /* We currently support the RSS Random Key retrieval for >>>>> 82599 and x540 >>>>> + * devices only. >>>>> + * >>>>> + * Thus return an error if API doesn't support RSS Random Key >>>>> retrieval >>>>> + * or if the operation is not supported for this device type. >>>>> + */ >>>>> + if (hw->api_version != ixgbe_mbox_api_12 || >>>>> + hw->mac.type >= ixgbe_mac_X550_vf) >>>>> + return -EPERM; >>>>> + >>>> The return type here should be not supported, or >>>> IXGBE_NOT_IMPLEMENTED, >>>> not no permissions. >>>> >>>>> + msgbuf[0] = IXGBE_VF_GET_RSS_KEY; >>>>> + err = hw->mbx.ops.write_posted(hw, msgbuf, 1); >>>>> + >>>>> + if (err) >>>>> + return err; >>>>> + >>>>> + err = hw->mbx.ops.read_posted(hw, msgbuf, 11); >>>>> + >>>>> + if (err) >>>>> + return err; >>>>> + >>>>> + msgbuf[0] &= ~IXGBE_VT_MSGTYPE_CTS; >>>>> + >>>>> + /* If the operation has been refused by a PF return -EPERM */ >>>>> + if (msgbuf[0] == (IXGBE_VF_GET_RETA | IXGBE_VT_MSGTYPE_NACK)) >>>>> + return -EPERM; >>>>> + >>>>> + /* If we didn't get an ACK there must have been >>>>> + * some sort of mailbox error so we should treat it >>>>> + * as such. >>>>> + */ >>>>> + if (msgbuf[0] != (IXGBE_VF_GET_RSS_KEY | IXGBE_VT_MSGTYPE_ACK)) >>>>> + return IXGBE_ERR_MBX; >>>>> + >>>>> + memcpy(rss_key, msgbuf + 1, IXGBEVF_RSS_HASH_KEY_SIZE); >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> +/** >>>>> + * 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. >>>>> + * Ensures the atomicy of a mailbox access using the >>>>> adapter.mbx_lock spinlock. >>>>> + * >>>>> + * Returns: 0 on success. >>>>> + * if API doesn't support this operation - (-EPERM). >>>>> + */ >>>>> +int ixgbevf_get_rss_key(struct ixgbevf_adapter *a, u8 *rss_key) >>>>> +{ >>>>> + int rc; >>>>> + >>>>> + spin_lock_bh(&a->mbx_lock); >>>>> + rc = ixgbevf_get_rss_key_locked(&a->hw, rss_key); >>>>> + spin_unlock_bh(&a->mbx_lock); >>>>> + >>>>> + return rc; >>>>> +} >>>>> + >>>> Since there is currently no cases where you would be getting the >>>> rss_key >>>> without the RETA you might just want to combine this function with >>>> ixgbevf_get_reta so you only take the lock once and process both >>>> messages in one pass instead of having to bounce the spinlock. >>> >>> I'd rather not do this. >>> Let's start from the beginning: the locks here should be removed and >>> added at the caller level to match what u wrote about patch04. >>> Then about the uniting the two functions mentioned above - there isn't >>> any added value of doing this. Taking a lock only once may be done >>> without uniting (see PATCH07 in v10 series). >>> On the other hand, this uniting is going to make the code awkward and >>> unclear ("why are these together anyway?"). >>> So, I'd rather keep these functions as they are apart from fixing >>> the locking issue. I'll essentially export the _locked functions. >>> >>> thanks, >>> vlad >> >> Agreed, just export the _locked functions and drop the _locked >> extension. > > Hmmm... I think keeping the _locked extension would make sense since > it would explicitly hint that these functions have to be called under > the lock. There are other similar examples in the ixgbe/ixgbevf code > already. The thing is the mailbox lock is only really needed to avoid contention with the few rare cases where the mailbox is accessed outside of the RTNL lock, also there is going to end up needing to be a different version of the function for x550 since it has direct register access to the redirection table via registers (mailbox lock is not needed since it is direct register access), so it might be worth it to take that into account in the naming. One option you might consider would be to look at adding get_reta and get_rssrk function pointers to the ixgbe_mac_operations structure and then splitting up ixgbevf_mac_ops so that you have the original, and an x550 version that doesn't populate those two function pointers. That might make it easier to then implement the x550 version later. You could probably even do something similar for the PF functionality as that way you could easily identify the functions that support getting the RETA and RSSRK by simply checking to see if those function pointers are present. As far as the other patches I am okay with the return being -ENOTSUPP for the x550 for now. - Alex -- 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
On 03/30/15 19:54, Alexander Duyck wrote: > > On 03/30/2015 08:17 AM, Vlad Zolotarov wrote: >> >> >> On 03/30/15 18:10, Alexander Duyck wrote: >>> >>> On 03/30/2015 06:53 AM, Vlad Zolotarov wrote: >>>> >>>> >>>> On 03/30/15 01:04, Alexander Duyck wrote: >>>>> On 03/29/2015 09:11 AM, Vlad Zolotarov wrote: >>>>>> Add the ixgbevf_get_rss_key() function that queries the PF for an >>>>>> RSS Random Key >>>>>> using a new VF-PF channel IXGBE_VF_GET_RSS_KEY command. >>>>>> >>>>>> This patch adds the support for 82599 and x540 devices only. >>>>>> Support for other >>>>>> devices will be added later. >>>>>> >>>>>> Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com> >>>>>> --- >>>>>> New in v9: >>>>>> - Reduce the support to 82599 and x540 devices only. >>>>>> - Added IXGBEVF_RSS_HASH_KEY_SIZE macro. >>>>>> >>>>>> New in v8: >>>>>> - Protect a mailbox access. >>>>>> >>>>>> New in v6: >>>>>> - Return a proper return code when an operation is blocked by >>>>>> PF. >>>>>> >>>>>> New in v2: >>>>>> - Added a more detailed patch description. >>>>>> >>>>>> New in v1 (compared to RFC): >>>>>> - Use "if-else" statement instead of a "switch-case" for a >>>>>> single option case >>>>>> (in ixgbevf_get_rss_key()). >>>>>> --- >>>>>> drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 1 + >>>>>> drivers/net/ethernet/intel/ixgbevf/mbx.h | 1 + >>>>>> drivers/net/ethernet/intel/ixgbevf/vf.c | 66 >>>>>> ++++++++++++++++++++++++++++ >>>>>> drivers/net/ethernet/intel/ixgbevf/vf.h | 1 + >>>>>> 4 files changed, 69 insertions(+) >>>>>> >>>>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>>>> b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>>>> index bc939a1..6771ae3 100644 >>>>>> --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>>>> +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h >>>>>> @@ -145,6 +145,7 @@ struct ixgbevf_ring { >>>>>> #define MAX_RX_QUEUES IXGBE_VF_MAX_RX_QUEUES >>>>>> #define MAX_TX_QUEUES IXGBE_VF_MAX_TX_QUEUES >>>>>> #define IXGBEVF_MAX_RSS_QUEUES 2 >>>>>> +#define IXGBEVF_RSS_HASH_KEY_SIZE 40 >>>>>> #define IXGBEVF_DEFAULT_TXD 1024 >>>>>> #define IXGBEVF_DEFAULT_RXD 512 >>>>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>>>> b/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>>>> index 66e138b..82f44e0 100644 >>>>>> --- a/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>>>> +++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h >>>>>> @@ -110,6 +110,7 @@ enum ixgbe_pfvf_api_rev { >>>>>> /* mailbox API, version 1.2 VF requests */ >>>>>> #define IXGBE_VF_GET_RETA 0x0a /* VF request for RETA */ >>>>>> +#define IXGBE_VF_GET_RSS_KEY 0x0b /* get RSS hash key */ >>>>>> /* length of permanent address message returned from PF */ >>>>>> #define IXGBE_VF_PERMADDR_MSG_LEN 4 >>>>>> diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c >>>>>> b/drivers/net/ethernet/intel/ixgbevf/vf.c >>>>>> index 2676c0b..ec68145 100644 >>>>>> --- a/drivers/net/ethernet/intel/ixgbevf/vf.c >>>>>> +++ b/drivers/net/ethernet/intel/ixgbevf/vf.c >>>>>> @@ -301,6 +301,72 @@ static inline int >>>>>> ixgbevf_get_reta_locked(struct ixgbe_hw *hw, u32 *msgbuf, >>>>>> return 0; >>>>>> } >>>>>> +static inline int ixgbevf_get_rss_key_locked(struct ixgbe_hw >>>>>> *hw, u8 *rss_key) >>>>>> +{ >>>>>> + int err; >>>>>> + u32 msgbuf[IXGBE_VFMAILBOX_SIZE]; >>>>>> + >>>>>> + /* We currently support the RSS Random Key retrieval for >>>>>> 82599 and x540 >>>>>> + * devices only. >>>>>> + * >>>>>> + * Thus return an error if API doesn't support RSS Random >>>>>> Key retrieval >>>>>> + * or if the operation is not supported for this device type. >>>>>> + */ >>>>>> + if (hw->api_version != ixgbe_mbox_api_12 || >>>>>> + hw->mac.type >= ixgbe_mac_X550_vf) >>>>>> + return -EPERM; >>>>>> + >>>>> The return type here should be not supported, or >>>>> IXGBE_NOT_IMPLEMENTED, >>>>> not no permissions. >>>>> >>>>>> + msgbuf[0] = IXGBE_VF_GET_RSS_KEY; >>>>>> + err = hw->mbx.ops.write_posted(hw, msgbuf, 1); >>>>>> + >>>>>> + if (err) >>>>>> + return err; >>>>>> + >>>>>> + err = hw->mbx.ops.read_posted(hw, msgbuf, 11); >>>>>> + >>>>>> + if (err) >>>>>> + return err; >>>>>> + >>>>>> + msgbuf[0] &= ~IXGBE_VT_MSGTYPE_CTS; >>>>>> + >>>>>> + /* If the operation has been refused by a PF return -EPERM */ >>>>>> + if (msgbuf[0] == (IXGBE_VF_GET_RETA | IXGBE_VT_MSGTYPE_NACK)) >>>>>> + return -EPERM; >>>>>> + >>>>>> + /* If we didn't get an ACK there must have been >>>>>> + * some sort of mailbox error so we should treat it >>>>>> + * as such. >>>>>> + */ >>>>>> + if (msgbuf[0] != (IXGBE_VF_GET_RSS_KEY | IXGBE_VT_MSGTYPE_ACK)) >>>>>> + return IXGBE_ERR_MBX; >>>>>> + >>>>>> + memcpy(rss_key, msgbuf + 1, IXGBEVF_RSS_HASH_KEY_SIZE); >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> +/** >>>>>> + * 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. >>>>>> + * Ensures the atomicy of a mailbox access using the >>>>>> adapter.mbx_lock spinlock. >>>>>> + * >>>>>> + * Returns: 0 on success. >>>>>> + * if API doesn't support this operation - (-EPERM). >>>>>> + */ >>>>>> +int ixgbevf_get_rss_key(struct ixgbevf_adapter *a, u8 *rss_key) >>>>>> +{ >>>>>> + int rc; >>>>>> + >>>>>> + spin_lock_bh(&a->mbx_lock); >>>>>> + rc = ixgbevf_get_rss_key_locked(&a->hw, rss_key); >>>>>> + spin_unlock_bh(&a->mbx_lock); >>>>>> + >>>>>> + return rc; >>>>>> +} >>>>>> + >>>>> Since there is currently no cases where you would be getting the >>>>> rss_key >>>>> without the RETA you might just want to combine this function with >>>>> ixgbevf_get_reta so you only take the lock once and process both >>>>> messages in one pass instead of having to bounce the spinlock. >>>> >>>> I'd rather not do this. >>>> Let's start from the beginning: the locks here should be removed and >>>> added at the caller level to match what u wrote about patch04. >>>> Then about the uniting the two functions mentioned above - there isn't >>>> any added value of doing this. Taking a lock only once may be done >>>> without uniting (see PATCH07 in v10 series). >>>> On the other hand, this uniting is going to make the code awkward and >>>> unclear ("why are these together anyway?"). >>>> So, I'd rather keep these functions as they are apart from fixing >>>> the locking issue. I'll essentially export the _locked functions. >>>> >>>> thanks, >>>> vlad >>> >>> Agreed, just export the _locked functions and drop the _locked >>> extension. >> >> Hmmm... I think keeping the _locked extension would make sense since >> it would explicitly hint that these functions have to be called under >> the lock. There are other similar examples in the ixgbe/ixgbevf code >> already. > > The thing is the mailbox lock is only really needed to avoid > contention with the few rare cases where the mailbox is accessed > outside of the RTNL lock, Since there are such cases - every access to mailbox has to be protected. > also there is going to end up needing to be a different version of the > function for x550 since it has direct register access to the > redirection table via registers (mailbox lock is not needed since it > is direct register access), so it might be worth it to take that into > account in the naming. That's exactly what I'm going to do here, don't u think? ;) Since for 82599 and x540 it should be "locked" the name of the corresponding function reflects it. > > One option you might consider would be to look at adding get_reta and > get_rssrk function pointers to the ixgbe_mac_operations structure and > then splitting up ixgbevf_mac_ops so that you have the original, and > an x550 version that doesn't populate those two function pointers. > That might make it easier to then implement the x550 version later. > You could probably even do something similar for the PF functionality > as that way you could easily identify the functions that support > getting the RETA and RSSRK by simply checking to see if those function > pointers are present. I'll leave this to consider to the one that is going to add the x550 support. > > As far as the other patches I am okay with the return being -ENOTSUPP > for the x550 for now. > > - Alex > -- 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
diff --git a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h index bc939a1..6771ae3 100644 --- a/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h +++ b/drivers/net/ethernet/intel/ixgbevf/ixgbevf.h @@ -145,6 +145,7 @@ struct ixgbevf_ring { #define MAX_RX_QUEUES IXGBE_VF_MAX_RX_QUEUES #define MAX_TX_QUEUES IXGBE_VF_MAX_TX_QUEUES #define IXGBEVF_MAX_RSS_QUEUES 2 +#define IXGBEVF_RSS_HASH_KEY_SIZE 40 #define IXGBEVF_DEFAULT_TXD 1024 #define IXGBEVF_DEFAULT_RXD 512 diff --git a/drivers/net/ethernet/intel/ixgbevf/mbx.h b/drivers/net/ethernet/intel/ixgbevf/mbx.h index 66e138b..82f44e0 100644 --- a/drivers/net/ethernet/intel/ixgbevf/mbx.h +++ b/drivers/net/ethernet/intel/ixgbevf/mbx.h @@ -110,6 +110,7 @@ enum ixgbe_pfvf_api_rev { /* mailbox API, version 1.2 VF requests */ #define IXGBE_VF_GET_RETA 0x0a /* VF request for RETA */ +#define IXGBE_VF_GET_RSS_KEY 0x0b /* get RSS hash key */ /* length of permanent address message returned from PF */ #define IXGBE_VF_PERMADDR_MSG_LEN 4 diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.c b/drivers/net/ethernet/intel/ixgbevf/vf.c index 2676c0b..ec68145 100644 --- a/drivers/net/ethernet/intel/ixgbevf/vf.c +++ b/drivers/net/ethernet/intel/ixgbevf/vf.c @@ -301,6 +301,72 @@ static inline int ixgbevf_get_reta_locked(struct ixgbe_hw *hw, u32 *msgbuf, return 0; } +static inline int ixgbevf_get_rss_key_locked(struct ixgbe_hw *hw, u8 *rss_key) +{ + int err; + u32 msgbuf[IXGBE_VFMAILBOX_SIZE]; + + /* We currently support the RSS Random Key retrieval for 82599 and x540 + * devices only. + * + * Thus return an error if API doesn't support RSS Random Key retrieval + * or if the operation is not supported for this device type. + */ + if (hw->api_version != ixgbe_mbox_api_12 || + hw->mac.type >= ixgbe_mac_X550_vf) + return -EPERM; + + msgbuf[0] = IXGBE_VF_GET_RSS_KEY; + err = hw->mbx.ops.write_posted(hw, msgbuf, 1); + + if (err) + return err; + + err = hw->mbx.ops.read_posted(hw, msgbuf, 11); + + if (err) + return err; + + msgbuf[0] &= ~IXGBE_VT_MSGTYPE_CTS; + + /* If the operation has been refused by a PF return -EPERM */ + if (msgbuf[0] == (IXGBE_VF_GET_RETA | IXGBE_VT_MSGTYPE_NACK)) + return -EPERM; + + /* If we didn't get an ACK there must have been + * some sort of mailbox error so we should treat it + * as such. + */ + if (msgbuf[0] != (IXGBE_VF_GET_RSS_KEY | IXGBE_VT_MSGTYPE_ACK)) + return IXGBE_ERR_MBX; + + memcpy(rss_key, msgbuf + 1, IXGBEVF_RSS_HASH_KEY_SIZE); + + return 0; +} + +/** + * 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. + * Ensures the atomicy of a mailbox access using the adapter.mbx_lock spinlock. + * + * Returns: 0 on success. + * if API doesn't support this operation - (-EPERM). + */ +int ixgbevf_get_rss_key(struct ixgbevf_adapter *a, u8 *rss_key) +{ + int rc; + + spin_lock_bh(&a->mbx_lock); + rc = ixgbevf_get_rss_key_locked(&a->hw, rss_key); + spin_unlock_bh(&a->mbx_lock); + + return rc; +} + /** * ixgbevf_get_reta - get the RSS redirection table (RETA) contents. * @adapter: pointer to the port handle diff --git a/drivers/net/ethernet/intel/ixgbevf/vf.h b/drivers/net/ethernet/intel/ixgbevf/vf.h index 7944962..a054716 100644 --- a/drivers/net/ethernet/intel/ixgbevf/vf.h +++ b/drivers/net/ethernet/intel/ixgbevf/vf.h @@ -212,4 +212,5 @@ int ixgbevf_negotiate_api_version(struct ixgbe_hw *hw, int api); int ixgbevf_get_queues(struct ixgbe_hw *hw, unsigned int *num_tcs, unsigned int *default_tc); int ixgbevf_get_reta(struct ixgbevf_adapter *adapter, u32 *reta); +int ixgbevf_get_rss_key(struct ixgbevf_adapter *adapter, u8 *rss_key); #endif /* __IXGBE_VF_H__ */
Add the ixgbevf_get_rss_key() function that queries the PF for an RSS Random Key using a new VF-PF channel IXGBE_VF_GET_RSS_KEY command. This patch adds the support for 82599 and x540 devices only. Support for other devices will be added later. Signed-off-by: Vlad Zolotarov <vladz@cloudius-systems.com> --- New in v9: - Reduce the support to 82599 and x540 devices only. - Added IXGBEVF_RSS_HASH_KEY_SIZE macro. New in v8: - Protect a mailbox access. New in v6: - Return a proper return code when an operation is blocked by PF. New in v2: - Added a more detailed patch description. New in v1 (compared to RFC): - Use "if-else" statement instead of a "switch-case" for a single option case (in ixgbevf_get_rss_key()). --- drivers/net/ethernet/intel/ixgbevf/ixgbevf.h | 1 + drivers/net/ethernet/intel/ixgbevf/mbx.h | 1 + drivers/net/ethernet/intel/ixgbevf/vf.c | 66 ++++++++++++++++++++++++++++ drivers/net/ethernet/intel/ixgbevf/vf.h | 1 + 4 files changed, 69 insertions(+)