Message ID | 1272406825-11615-1-git-send-email-dm@chelsio.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
On Tue, 27 Apr 2010 15:20:25 -0700 Dimitris Michailidis <dm@chelsio.com> wrote: > Implement the ->set_flags ethtool method to control NETIF_F_RXHASH and > set skb->rxhash to the HW calculated hash accordingly. > > Signed-off-by: Dimitris Michailidis <dm@chelsio.com> > --- > drivers/net/cxgb4/cxgb4_main.c | 15 ++++++++++++++- > drivers/net/cxgb4/sge.c | 7 ++++++- > drivers/net/cxgb4/t4_msg.h | 1 + > 3 files changed, 21 insertions(+), 2 deletions(-) > > diff --git a/drivers/net/cxgb4/cxgb4_main.c b/drivers/net/cxgb4/cxgb4_main.c > index 5f582db..1bad500 100644 > --- a/drivers/net/cxgb4/cxgb4_main.c > +++ b/drivers/net/cxgb4/cxgb4_main.c > @@ -1711,6 +1711,18 @@ static int set_tso(struct net_device *dev, u32 value) > return 0; > } > > +static int set_flags(struct net_device *dev, u32 flags) > +{ > + if (flags & ~ETH_FLAG_RXHASH) > + return -EOPNOTSUPP; > + > + if (flags & ETH_FLAG_RXHASH) > + dev->features |= NETIF_F_RXHASH; > + else > + dev->features &= ~NETIF_F_RXHASH; > + return 0; > +} You need to check for and reject other values NTUPLE and LRO -- 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
Stephen Hemminger wrote: > On Tue, 27 Apr 2010 15:20:25 -0700 > Dimitris Michailidis <dm@chelsio.com> wrote: > >> Implement the ->set_flags ethtool method to control NETIF_F_RXHASH and >> set skb->rxhash to the HW calculated hash accordingly. >> >> Signed-off-by: Dimitris Michailidis <dm@chelsio.com> >> --- >> drivers/net/cxgb4/cxgb4_main.c | 15 ++++++++++++++- >> drivers/net/cxgb4/sge.c | 7 ++++++- >> drivers/net/cxgb4/t4_msg.h | 1 + >> 3 files changed, 21 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/net/cxgb4/cxgb4_main.c b/drivers/net/cxgb4/cxgb4_main.c >> index 5f582db..1bad500 100644 >> --- a/drivers/net/cxgb4/cxgb4_main.c >> +++ b/drivers/net/cxgb4/cxgb4_main.c >> @@ -1711,6 +1711,18 @@ static int set_tso(struct net_device *dev, u32 value) >> return 0; >> } >> >> +static int set_flags(struct net_device *dev, u32 flags) >> +{ >> + if (flags & ~ETH_FLAG_RXHASH) >> + return -EOPNOTSUPP; >> + >> + if (flags & ETH_FLAG_RXHASH) >> + dev->features |= NETIF_F_RXHASH; >> + else >> + dev->features &= ~NETIF_F_RXHASH; >> + return 0; >> +} > > You need to check for and reject other values NTUPLE and LRO The first if does that, doesn't it? -- 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
Le mardi 27 avril 2010 à 15:20 -0700, Dimitris Michailidis a écrit : > Implement the ->set_flags ethtool method to control NETIF_F_RXHASH and > set skb->rxhash to the HW calculated hash accordingly. > > Signed-off-by: Dimitris Michailidis <dm@chelsio.com> > skb_record_rx_queue(skb, rxq->rspq.idx); > + if (rxq->rspq.netdev->features & NETIF_F_RXHASH) > + skb->rxhash = ntohl(pkt->rsshdr.hash_val); > Its probably minor, but hash_val being a semi random 32bits value, you can avoid the ntohl() conversion, using a (__force u32) cast ... -- 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
From: Dimitris Michailidis <dm@chelsio.com> Date: Tue, 27 Apr 2010 15:20:25 -0700 > Implement the ->set_flags ethtool method to control NETIF_F_RXHASH and > set skb->rxhash to the HW calculated hash accordingly. > > Signed-off-by: Dimitris Michailidis <dm@chelsio.com> Does the CXGB4 hash the ports for non-TCP packets? If not, you will need to check the packet type (preferrably using RX descriptor information rather than actually parsing the packet) and only record the ->rxhash for TCP packets. -- 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
From: Stephen Hemminger <shemminger@vyatta.com> Date: Tue, 27 Apr 2010 15:36:56 -0700 >> +static int set_flags(struct net_device *dev, u32 flags) >> +{ >> + if (flags & ~ETH_FLAG_RXHASH) >> + return -EOPNOTSUPP; >> + >> + if (flags & ETH_FLAG_RXHASH) >> + dev->features |= NETIF_F_RXHASH; >> + else >> + dev->features &= ~NETIF_F_RXHASH; >> + return 0; >> +} > > You need to check for and reject other values NTUPLE and LRO He does, it's the first he does in the function. -- 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
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Wed, 28 Apr 2010 00:43:12 +0200 > Le mardi 27 avril 2010 à 15:20 -0700, Dimitris Michailidis a écrit : >> Implement the ->set_flags ethtool method to control NETIF_F_RXHASH and >> set skb->rxhash to the HW calculated hash accordingly. >> >> Signed-off-by: Dimitris Michailidis <dm@chelsio.com> > >> skb_record_rx_queue(skb, rxq->rspq.idx); >> + if (rxq->rspq.netdev->features & NETIF_F_RXHASH) >> + skb->rxhash = ntohl(pkt->rsshdr.hash_val); >> > > Its probably minor, but hash_val being a semi random 32bits value, you > can avoid the ntohl() conversion, using a (__force u32) cast ... Agreed. -- 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
David Miller wrote: > From: Dimitris Michailidis <dm@chelsio.com> > Date: Tue, 27 Apr 2010 15:20:25 -0700 > >> Implement the ->set_flags ethtool method to control NETIF_F_RXHASH and >> set skb->rxhash to the HW calculated hash accordingly. >> >> Signed-off-by: Dimitris Michailidis <dm@chelsio.com> > > Does the CXGB4 hash the ports for non-TCP packets? > > If not, you will need to check the packet type (preferrably using > RX descriptor information rather than actually parsing the packet) > and only record the ->rxhash for TCP packets. > It does, optionally. Someday I need to implement ->set_rxnfc to expose the few available options but it's on by default. -- 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
From: Dimitris Michailidis <dm@chelsio.com> Date: Tue, 27 Apr 2010 15:46:45 -0700 > David Miller wrote: >> From: Dimitris Michailidis <dm@chelsio.com> >> Date: Tue, 27 Apr 2010 15:20:25 -0700 >> >>> Implement the ->set_flags ethtool method to control NETIF_F_RXHASH and >>> set skb->rxhash to the HW calculated hash accordingly. >>> >>> Signed-off-by: Dimitris Michailidis <dm@chelsio.com> >> Does the CXGB4 hash the ports for non-TCP packets? >> If not, you will need to check the packet type (preferrably using >> RX descriptor information rather than actually parsing the packet) >> and only record the ->rxhash for TCP packets. >> > > It does, optionally. Someday I need to implement ->set_rxnfc to > expose the few available options but it's on by default. Great, if you could respin this patch and ditch the ntohl() as Eric suggested I'll apply it. Thanks! -- 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/cxgb4/cxgb4_main.c b/drivers/net/cxgb4/cxgb4_main.c index 5f582db..1bad500 100644 --- a/drivers/net/cxgb4/cxgb4_main.c +++ b/drivers/net/cxgb4/cxgb4_main.c @@ -1711,6 +1711,18 @@ static int set_tso(struct net_device *dev, u32 value) return 0; } +static int set_flags(struct net_device *dev, u32 flags) +{ + if (flags & ~ETH_FLAG_RXHASH) + return -EOPNOTSUPP; + + if (flags & ETH_FLAG_RXHASH) + dev->features |= NETIF_F_RXHASH; + else + dev->features &= ~NETIF_F_RXHASH; + return 0; +} + static struct ethtool_ops cxgb_ethtool_ops = { .get_settings = get_settings, .set_settings = set_settings, @@ -1741,6 +1753,7 @@ static struct ethtool_ops cxgb_ethtool_ops = { .get_wol = get_wol, .set_wol = set_wol, .set_tso = set_tso, + .set_flags = set_flags, .flash_device = set_flash, }; @@ -3203,7 +3216,7 @@ static int __devinit init_one(struct pci_dev *pdev, netdev->features |= NETIF_F_SG | NETIF_F_TSO | NETIF_F_TSO6; netdev->features |= NETIF_F_IP_CSUM | NETIF_F_IPV6_CSUM; - netdev->features |= NETIF_F_GRO | highdma; + netdev->features |= NETIF_F_GRO | NETIF_F_RXHASH | highdma; netdev->features |= NETIF_F_HW_VLAN_TX | NETIF_F_HW_VLAN_RX; netdev->vlan_features = netdev->features & VLAN_FEAT; diff --git a/drivers/net/cxgb4/sge.c b/drivers/net/cxgb4/sge.c index 65d91c4..e804ffc 100644 --- a/drivers/net/cxgb4/sge.c +++ b/drivers/net/cxgb4/sge.c @@ -1524,6 +1524,8 @@ static void do_gro(struct sge_eth_rxq *rxq, const struct pkt_gl *gl, skb->truesize += skb->data_len; skb->ip_summed = CHECKSUM_UNNECESSARY; skb_record_rx_queue(skb, rxq->rspq.idx); + if (rxq->rspq.netdev->features & NETIF_F_RXHASH) + skb->rxhash = ntohl(pkt->rsshdr.hash_val); if (unlikely(pkt->vlan_ex)) { struct port_info *pi = netdev_priv(rxq->rspq.netdev); @@ -1565,7 +1567,7 @@ int t4_ethrx_handler(struct sge_rspq *q, const __be64 *rsp, if (unlikely(*(u8 *)rsp == CPL_TRACE_PKT)) return handle_trace_pkt(q->adap, si); - pkt = (void *)&rsp[1]; + pkt = (const struct cpl_rx_pkt *)rsp; csum_ok = pkt->csum_calc && !pkt->err_vec; if ((pkt->l2info & htonl(RXF_TCP)) && (q->netdev->features & NETIF_F_GRO) && csum_ok && !pkt->ip_frag) { @@ -1583,6 +1585,9 @@ int t4_ethrx_handler(struct sge_rspq *q, const __be64 *rsp, __skb_pull(skb, RX_PKT_PAD); /* remove ethernet header padding */ skb->protocol = eth_type_trans(skb, q->netdev); skb_record_rx_queue(skb, q->idx); + if (skb->dev->features & NETIF_F_RXHASH) + skb->rxhash = ntohl(pkt->rsshdr.hash_val); + pi = netdev_priv(skb->dev); rxq->stats.pkts++; diff --git a/drivers/net/cxgb4/t4_msg.h b/drivers/net/cxgb4/t4_msg.h index fdb1174..7a981b8 100644 --- a/drivers/net/cxgb4/t4_msg.h +++ b/drivers/net/cxgb4/t4_msg.h @@ -503,6 +503,7 @@ struct cpl_rx_data_ack { }; struct cpl_rx_pkt { + struct rss_header rsshdr; u8 opcode; #if defined(__LITTLE_ENDIAN_BITFIELD) u8 iff:4;
Implement the ->set_flags ethtool method to control NETIF_F_RXHASH and set skb->rxhash to the HW calculated hash accordingly. Signed-off-by: Dimitris Michailidis <dm@chelsio.com> --- drivers/net/cxgb4/cxgb4_main.c | 15 ++++++++++++++- drivers/net/cxgb4/sge.c | 7 ++++++- drivers/net/cxgb4/t4_msg.h | 1 + 3 files changed, 21 insertions(+), 2 deletions(-)