| Message ID | 1446485500-9782-5-git-send-email-madalin.bucur@freescale.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, 2015-11-02 at 19:31 +0200, Madalin Bucur wrote: > Allow the selection of the transmission queue based on the CPU id. Explain why. > > Signed-off-by: Madalin Bucur <madalin.bucur@freescale.com> > --- > drivers/net/ethernet/freescale/dpaa/Kconfig | 10 ++++++++++ > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 3 +++ > drivers/net/ethernet/freescale/dpaa/dpaa_eth.h | 6 ++++++ > drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c | 8 ++++++++ > drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h | 4 ++++ > 5 files changed, 31 insertions(+) > > diff --git a/drivers/net/ethernet/freescale/dpaa/Kconfig > b/drivers/net/ethernet/freescale/dpaa/Kconfig > index 022d5aa..2577aac 100644 > --- a/drivers/net/ethernet/freescale/dpaa/Kconfig > +++ b/drivers/net/ethernet/freescale/dpaa/Kconfig > @@ -11,6 +11,16 @@ menuconfig FSL_DPAA_ETH > > if FSL_DPAA_ETH > > +config FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE > + bool "Use driver's Tx queue selection mechanism" > + default y > + ---help--- > + The DPAA Ethernet driver defines a ndo_select_queue() callback > for optimal selection > + of the egress FQ. That will override the XPS support for this > netdevice. > + If for whatever reason you want to be in control of the egress FQ > -to-CPU selection and mapping, > + or simply don't want to use the driver's ndo_select_queue() > callback, then unselect this > + and use the standard XPS support instead. Is there a use case for needing this to be configurable? > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > index 31d55b4..894f1a7 100644 > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > @@ -390,6 +390,9 @@ static const struct net_device_ops dpa_private_ops = { > .ndo_get_stats64 = dpa_get_stats64, > .ndo_set_mac_address = dpa_set_mac_address, > .ndo_validate_addr = eth_validate_addr, > +#ifdef CONFIG_FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE > + .ndo_select_queue = dpa_select_queue, > +#endif > .ndo_change_mtu = dpa_change_mtu, > .ndo_set_rx_mode = dpa_set_rx_mode, > .ndo_init = dpa_ndo_init, > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h > b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h > index 1ba6617..87577cf 100644 > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h > @@ -420,9 +420,15 @@ static inline void _dpa_assign_wq(struct dpa_fq *fq) > } > } > > +#ifdef CONFIG_FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE > +/* Use in lieu of skb_get_queue_mapping() */ > +#define dpa_get_queue_mapping(skb) \ > + raw_smp_processor_id() > +#else > /* Use the queue selected by XPS */ > #define dpa_get_queue_mapping(skb) \ > skb_get_queue_mapping(skb) > +#endif Why is this necessary? Shouldn't providing a custom .ndo_select_queue() be sufficient to ensure that skb_get_queue_mapping() returns the same thing? And if this goes away, it's just a matter of a function pointer, so if it does need to be configurable it could be a runtime option. -Scott -- 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
> -----Original Message----- > From: Wood Scott-B07421 > Sent: Wednesday, December 02, 2015 11:41 PM > > On Mon, 2015-11-02 at 19:31 +0200, Madalin Bucur wrote: > > Allow the selection of the transmission queue based on the CPU id. > > Explain why. I'll add more details in the commit log. This is a customer generated feature. Bypassing the standard XPS can increase performance by making use of the DPAA HW particularities. > > > > Signed-off-by: Madalin Bucur <madalin.bucur@freescale.com> > > --- > > drivers/net/ethernet/freescale/dpaa/Kconfig | 10 ++++++++++ > > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 3 +++ > > drivers/net/ethernet/freescale/dpaa/dpaa_eth.h | 6 ++++++ > > drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c | 8 ++++++++ > > drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h | 4 ++++ > > 5 files changed, 31 insertions(+) > > > > diff --git a/drivers/net/ethernet/freescale/dpaa/Kconfig > > b/drivers/net/ethernet/freescale/dpaa/Kconfig > > index 022d5aa..2577aac 100644 > > --- a/drivers/net/ethernet/freescale/dpaa/Kconfig > > +++ b/drivers/net/ethernet/freescale/dpaa/Kconfig > > @@ -11,6 +11,16 @@ menuconfig FSL_DPAA_ETH > > > > if FSL_DPAA_ETH > > > > +config FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE > > + bool "Use driver's Tx queue selection mechanism" > > + default y > > + ---help--- > > + The DPAA Ethernet driver defines a ndo_select_queue() callback > > for optimal selection > > + of the egress FQ. That will override the XPS support for this > > netdevice. > > + If for whatever reason you want to be in control of the egress FQ > > -to-CPU selection and mapping, > > + or simply don't want to use the driver's ndo_select_queue() > > callback, then unselect this > > + and use the standard XPS support instead. > > Is there a use case for needing this to be configurable? If the standard XPS is desired, the Kconfig option allows the driver user to select that. > > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > index 31d55b4..894f1a7 100644 > > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > @@ -390,6 +390,9 @@ static const struct net_device_ops dpa_private_ops = > { > > .ndo_get_stats64 = dpa_get_stats64, > > .ndo_set_mac_address = dpa_set_mac_address, > > .ndo_validate_addr = eth_validate_addr, > > +#ifdef CONFIG_FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE > > + .ndo_select_queue = dpa_select_queue, > > +#endif > > .ndo_change_mtu = dpa_change_mtu, > > .ndo_set_rx_mode = dpa_set_rx_mode, > > .ndo_init = dpa_ndo_init, > > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h > > b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h > > index 1ba6617..87577cf 100644 > > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h > > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h > > @@ -420,9 +420,15 @@ static inline void _dpa_assign_wq(struct dpa_fq > *fq) > > } > > } > > > > +#ifdef CONFIG_FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE > > +/* Use in lieu of skb_get_queue_mapping() */ > > +#define dpa_get_queue_mapping(skb) \ > > + raw_smp_processor_id() > > +#else > > /* Use the queue selected by XPS */ > > #define dpa_get_queue_mapping(skb) \ > > skb_get_queue_mapping(skb) > > +#endif > > Why is this necessary? Shouldn't providing a custom .ndo_select_queue() > be > sufficient to ensure that skb_get_queue_mapping() returns the same thing? dpa_get_queue_mapping() is used in more than one place, the ndo function cannot be used directly in all places, the current setup is justified. > And if this goes away, it's just a matter of a function pointer, so if it > does > need to be configurable it could be a runtime option. > > -Scott It's used on the hot path, adding an extra indirection layer to make it selectable at runtime would defeat the purpose... Madalin
On Thu, 2015-12-03 at 04:02 -0600, Bucur Madalin-Cristian-B32716 wrote: > > -----Original Message----- > > From: Wood Scott-B07421 > > Sent: Wednesday, December 02, 2015 11:41 PM > > > > On Mon, 2015-11-02 at 19:31 +0200, Madalin Bucur wrote: > > > Allow the selection of the transmission queue based on the CPU id. > > > > Explain why. > > I'll add more details in the commit log. This is a customer generated > feature. Bypassing the standard XPS can increase performance by making use > of the DPAA HW particularities. > > > > > > > Signed-off-by: Madalin Bucur <madalin.bucur@freescale.com> > > > --- > > > drivers/net/ethernet/freescale/dpaa/Kconfig | 10 ++++++++++ > > > drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 3 +++ > > > drivers/net/ethernet/freescale/dpaa/dpaa_eth.h | 6 ++++++ > > > drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c | 8 ++++++++ > > > drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h | 4 ++++ > > > 5 files changed, 31 insertions(+) > > > > > > diff --git a/drivers/net/ethernet/freescale/dpaa/Kconfig > > > b/drivers/net/ethernet/freescale/dpaa/Kconfig > > > index 022d5aa..2577aac 100644 > > > --- a/drivers/net/ethernet/freescale/dpaa/Kconfig > > > +++ b/drivers/net/ethernet/freescale/dpaa/Kconfig > > > @@ -11,6 +11,16 @@ menuconfig FSL_DPAA_ETH > > > > > > if FSL_DPAA_ETH > > > > > > +config FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE > > > + bool "Use driver's Tx queue selection mechanism" > > > + default y > > > + ---help--- > > > + The DPAA Ethernet driver defines a ndo_select_queue() > > > callback > > > for optimal selection > > > + of the egress FQ. That will override the XPS support for this > > > netdevice. > > > + If for whatever reason you want to be in control of the > > > egress FQ > > > -to-CPU selection and mapping, > > > + or simply don't want to use the driver's ndo_select_queue() > > > callback, then unselect this > > > + and use the standard XPS support instead. > > > > Is there a use case for needing this to be configurable? > > If the standard XPS is desired, the Kconfig option allows the driver user to > select that. Under what circumstances does it make sense to desire this? Could you put the answer to that in the help text? > > > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > > b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > > index 31d55b4..894f1a7 100644 > > > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c > > > @@ -390,6 +390,9 @@ static const struct net_device_ops dpa_private_ops = > > { > > > .ndo_get_stats64 = dpa_get_stats64, > > > .ndo_set_mac_address = dpa_set_mac_address, > > > .ndo_validate_addr = eth_validate_addr, > > > +#ifdef CONFIG_FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE > > > + .ndo_select_queue = dpa_select_queue, > > > +#endif > > > .ndo_change_mtu = dpa_change_mtu, > > > .ndo_set_rx_mode = dpa_set_rx_mode, > > > .ndo_init = dpa_ndo_init, > > > diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h > > > b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h > > > index 1ba6617..87577cf 100644 > > > --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h > > > +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h > > > @@ -420,9 +420,15 @@ static inline void _dpa_assign_wq(struct dpa_fq > > *fq) > > > } > > > } > > > > > > +#ifdef CONFIG_FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE > > > +/* Use in lieu of skb_get_queue_mapping() */ > > > +#define dpa_get_queue_mapping(skb) \ > > > + raw_smp_processor_id() > > > +#else > > > /* Use the queue selected by XPS */ > > > #define dpa_get_queue_mapping(skb) \ > > > skb_get_queue_mapping(skb) > > > +#endif > > > > Why is this necessary? Shouldn't providing a custom .ndo_select_queue() > > be > > sufficient to ensure that skb_get_queue_mapping() returns the same thing? > > dpa_get_queue_mapping() is used in more than one place, the ndo function > cannot > be used directly in all places, the current setup is justified. It's called in two places that I see. For the call in dpa_tx(), when will skb_get_queue_mapping() not return the correct answer? For the call in dpa_select_queue(), that's already called via function pointer so it would not be a new indirection layer. > > And if this goes away, it's just a matter of a function pointer, so if it > > does > > need to be configurable it could be a runtime option. > > > > -Scott > > It's used on the hot path, adding an extra indirection layer to make it > selectable > at runtime would defeat the purpose... "if this goes away" I wasn't asking for a new indirection layer. -Scott -- 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/freescale/dpaa/Kconfig b/drivers/net/ethernet/freescale/dpaa/Kconfig index 022d5aa..2577aac 100644 --- a/drivers/net/ethernet/freescale/dpaa/Kconfig +++ b/drivers/net/ethernet/freescale/dpaa/Kconfig @@ -11,6 +11,16 @@ menuconfig FSL_DPAA_ETH if FSL_DPAA_ETH +config FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE + bool "Use driver's Tx queue selection mechanism" + default y + ---help--- + The DPAA Ethernet driver defines a ndo_select_queue() callback for optimal selection + of the egress FQ. That will override the XPS support for this netdevice. + If for whatever reason you want to be in control of the egress FQ-to-CPU selection and mapping, + or simply don't want to use the driver's ndo_select_queue() callback, then unselect this + and use the standard XPS support instead. + config FSL_DPAA_ETH_FRIENDLY_IF_NAME bool "Use fmX-macY names for the DPAA interfaces" default y diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c index 31d55b4..894f1a7 100644 --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c @@ -390,6 +390,9 @@ static const struct net_device_ops dpa_private_ops = { .ndo_get_stats64 = dpa_get_stats64, .ndo_set_mac_address = dpa_set_mac_address, .ndo_validate_addr = eth_validate_addr, +#ifdef CONFIG_FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE + .ndo_select_queue = dpa_select_queue, +#endif .ndo_change_mtu = dpa_change_mtu, .ndo_set_rx_mode = dpa_set_rx_mode, .ndo_init = dpa_ndo_init, diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h index 1ba6617..87577cf 100644 --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.h @@ -420,9 +420,15 @@ static inline void _dpa_assign_wq(struct dpa_fq *fq) } } +#ifdef CONFIG_FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE +/* Use in lieu of skb_get_queue_mapping() */ +#define dpa_get_queue_mapping(skb) \ + raw_smp_processor_id() +#else /* Use the queue selected by XPS */ #define dpa_get_queue_mapping(skb) \ skb_get_queue_mapping(skb) +#endif static inline void _dpa_bp_free_pf(void *addr) { diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c b/drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c index b36cbca..89f3b1f 100644 --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c @@ -593,6 +593,14 @@ bool dpa_bpid2pool_use(int bpid) return false; } +#ifdef CONFIG_FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE +u16 dpa_select_queue(struct net_device *net_dev, struct sk_buff *skb, + void *accel_priv, select_queue_fallback_t fallback) +{ + return dpa_get_queue_mapping(skb); +} +#endif + struct dpa_fq *dpa_fq_alloc(struct device *dev, const struct fqid_cell *fqids, struct list_head *list, diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h b/drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h index 9df8f14..2e9471d 100644 --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h @@ -70,6 +70,10 @@ struct dpa_bp *dpa_bpid2pool(int bpid); void dpa_bpid2pool_map(int bpid, struct dpa_bp *dpa_bp); bool dpa_bpid2pool_use(int bpid); void dpa_bp_drain(struct dpa_bp *bp); +#ifdef CONFIG_FSL_DPAA_ETH_USE_NDO_SELECT_QUEUE +u16 dpa_select_queue(struct net_device *net_dev, struct sk_buff *skb, + void *accel_priv, select_queue_fallback_t fallback); +#endif struct dpa_fq *dpa_fq_alloc(struct device *dev, const struct fqid_cell *fqids, struct list_head *list,
Allow the selection of the transmission queue based on the CPU id. Signed-off-by: Madalin Bucur <madalin.bucur@freescale.com> --- drivers/net/ethernet/freescale/dpaa/Kconfig | 10 ++++++++++ drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 3 +++ drivers/net/ethernet/freescale/dpaa/dpaa_eth.h | 6 ++++++ drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.c | 8 ++++++++ drivers/net/ethernet/freescale/dpaa/dpaa_eth_common.h | 4 ++++ 5 files changed, 31 insertions(+)