Message ID | 1291896556.8745.7.camel@lb-tlvb-dmitry |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
From: "Dmitry Kravkov" <dmitry@broadcom.com> Date: Thu, 9 Dec 2010 14:09:16 +0200 > +#endif > + /* Select queue (if defined) adjust for fcoe */ > + fp_index = skb_tx_hash(dev, skb) - FCOE_CONTEXT_USE; > + > + return (fp_index >= 0 ? fp_index : 0); This doesn't make any sense, and it's one of the reasons I really hate the fact that drivers sometimes need to override the select_queue method. Because 9 times out of 10 they get it wrong. You're mapping queues 0 --> FCOE_CONTEXT_USE to zero. But that's not what you actually want. You're actually trying to make non-FCOE traffic hash only amongst the non-FCOE queues. So make your code actually do that instead of screwing up the distribution of the hash result. -- 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 Fri, 2010-12-10 at 14:11 -0800, David Miller wrote: > From: "Dmitry Kravkov" <dmitry@broadcom.com> > Date: Thu, 9 Dec 2010 14:09:16 +0200 > > > +#endif > > + /* Select queue (if defined) adjust for fcoe */ > > + fp_index = skb_tx_hash(dev, skb) - FCOE_CONTEXT_USE; > > + > > + return (fp_index >= 0 ? fp_index : 0); > > This doesn't make any sense, and it's one of the reasons I really > hate the fact that drivers sometimes need to override the > select_queue method. > > Because 9 times out of 10 they get it wrong. > > You're mapping queues 0 --> FCOE_CONTEXT_USE to zero. > > But that's not what you actually want. > > You're actually trying to make non-FCOE traffic hash only amongst the > non-FCOE queues. > > So make your code actually do that instead of screwing up the > distribution of the hash result. > Indeed, the current suggestion is giving queue 0 double weight compared to all other queues just so the skb_tx_hash will be used as is to benefit from future enhancement changes to it. We will re-spin this patch and move the content of the skb_tx_hash to __skb_tx_hash that will receive the netdev->skb_real_num_tx_queues as a parameter - this way, we can call the __skb_tx_hash with a lower number. Thanks, Eilon -- 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/bnx2x/bnx2x_cmn.c b/drivers/net/bnx2x/bnx2x_cmn.c index a553fba..862b29e 100644 --- a/drivers/net/bnx2x/bnx2x_cmn.c +++ b/drivers/net/bnx2x/bnx2x_cmn.c @@ -1167,6 +1167,37 @@ void bnx2x_netif_stop(struct bnx2x *bp, int disable_hw) netif_tx_disable(bp->dev); } +u16 bnx2x_select_queue(struct net_device *dev, struct sk_buff *skb) +{ + int fp_index = 0; + +#ifdef BCM_CNIC + struct bnx2x *bp = netdev_priv(dev); + if (NO_FCOE(bp)) + return skb_tx_hash(dev, skb); + else { + struct ethhdr *hdr = (struct ethhdr *)skb->data; + u16 ether_type = ntohs(hdr->h_proto); + + /* Skip VLAN tag if present */ + if (ether_type == ETH_P_8021Q) { + struct vlan_ethhdr *vhdr = + (struct vlan_ethhdr *)skb->data; + + ether_type = ntohs(vhdr->h_vlan_encapsulated_proto); + } + + /* If ethertype is FCoE or FIP - use FCoE ring */ + if ((ether_type == ETH_P_FCOE) || (ether_type == ETH_P_FIP)) + return bnx2x_fcoe(bp, index); + } +#endif + /* Select queue (if defined) adjust for fcoe */ + fp_index = skb_tx_hash(dev, skb) - FCOE_CONTEXT_USE; + + return (fp_index >= 0 ? fp_index : 0); +} + void bnx2x_set_num_queues(struct bnx2x *bp) { switch (bp->multi_mode) { diff --git a/drivers/net/bnx2x/bnx2x_cmn.h b/drivers/net/bnx2x/bnx2x_cmn.h index 4bb0113..258f0c0 100644 --- a/drivers/net/bnx2x/bnx2x_cmn.h +++ b/drivers/net/bnx2x/bnx2x_cmn.h @@ -343,6 +343,9 @@ int bnx2x_nic_load(struct bnx2x *bp, int load_mode); /* hard_xmit callback */ netdev_tx_t bnx2x_start_xmit(struct sk_buff *skb, struct net_device *dev); +/* select_queue callback */ +u16 bnx2x_select_queue(struct net_device *dev, struct sk_buff *skb); + int bnx2x_change_mac_addr(struct net_device *dev, void *p); /* NAPI poll Rx part */ diff --git a/drivers/net/bnx2x/bnx2x_main.c b/drivers/net/bnx2x/bnx2x_main.c index 6c70ee5..c4dfb42 100644 --- a/drivers/net/bnx2x/bnx2x_main.c +++ b/drivers/net/bnx2x/bnx2x_main.c @@ -8977,6 +8977,7 @@ static const struct net_device_ops bnx2x_netdev_ops = { .ndo_open = bnx2x_open, .ndo_stop = bnx2x_close, .ndo_start_xmit = bnx2x_start_xmit, + .ndo_select_queue = bnx2x_select_queue, .ndo_set_multicast_list = bnx2x_set_rx_mode, .ndo_set_mac_address = bnx2x_change_mac_addr, .ndo_validate_addr = eth_validate_addr,