lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Tue, 12 Jun 2012 17:47:59 -0700 From: Jay Vosburgh <fubar@...ibm.com> To: Eric Dumazet <eric.dumazet@...il.com> cc: David Miller <davem@...emloft.net>, netdev <netdev@...r.kernel.org>, Andy Gospodarek <andy@...yhouse.net>, Jiri Bohac <jbohac@...e.cz>, Nicolas de =?ISO-8859-1?Q?Peslo=FCan?= <nicolas.2p.debian@...e.fr>, Maciej =?UTF-8?Q?=C5=BBenczykowski?= <maze@...gle.com> Subject: Re: [PATCH net-next] bonding: remove packet cloning in recv_probe() Eric Dumazet <eric.dumazet@...il.com> wrote: >From: Eric Dumazet <edumazet@...gle.com> > >Cloning all packets in input path have a significant cost. > >Use skb_header_pointer()/skb_copy_bits() instead of pskb_may_pull() so >that recv_probe handlers (bond_3ad_lacpdu_recv / bond_arp_rcv / >rlb_arp_recv ) dont touch input skb. > >bond_handle_frame() can avoid the skb_clone()/dev_kfree_skb() > >Signed-off-by: Eric Dumazet <edumazet@...gle.com> >Cc: Jay Vosburgh <fubar@...ibm.com> >Cc: Andy Gospodarek <andy@...yhouse.net> >Cc: Jiri Bohac <jbohac@...e.cz> >Cc: Nicolas de Pesloüan <nicolas.2p.debian@...e.fr> >Cc: Maciej Żenczykowski <maze@...gle.com> This looks really good to me. -J Signed-off-by: Jay Vosburgh <fubar@...ibm.com> >--- > drivers/net/bonding/bond_3ad.c | 11 +++++--- > drivers/net/bonding/bond_3ad.h | 4 +-- > drivers/net/bonding/bond_alb.c | 20 ++++------------ > drivers/net/bonding/bond_main.c | 37 ++++++++++++++++-------------- > drivers/net/bonding/bonding.h | 4 +-- > 5 files changed, 36 insertions(+), 40 deletions(-) > >diff --git a/drivers/net/bonding/bond_3ad.c b/drivers/net/bonding/bond_3ad.c >index 3463b46..3031e04 100644 >--- a/drivers/net/bonding/bond_3ad.c >+++ b/drivers/net/bonding/bond_3ad.c >@@ -2460,18 +2460,21 @@ out: > return NETDEV_TX_OK; > } > >-int bond_3ad_lacpdu_recv(struct sk_buff *skb, struct bonding *bond, >- struct slave *slave) >+int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond, >+ struct slave *slave) > { > int ret = RX_HANDLER_ANOTHER; >+ struct lacpdu *lacpdu, _lacpdu; >+ > if (skb->protocol != PKT_TYPE_LACPDU) > return ret; > >- if (!pskb_may_pull(skb, sizeof(struct lacpdu))) >+ lacpdu = skb_header_pointer(skb, 0, sizeof(_lacpdu), &_lacpdu); >+ if (!lacpdu) > return ret; > > read_lock(&bond->lock); >- ret = bond_3ad_rx_indication((struct lacpdu *) skb->data, slave, skb->len); >+ ret = bond_3ad_rx_indication(lacpdu, slave, skb->len); > read_unlock(&bond->lock); > return ret; > } >diff --git a/drivers/net/bonding/bond_3ad.h b/drivers/net/bonding/bond_3ad.h >index 5ee7e3c..0cfaa4a 100644 >--- a/drivers/net/bonding/bond_3ad.h >+++ b/drivers/net/bonding/bond_3ad.h >@@ -274,8 +274,8 @@ void bond_3ad_adapter_duplex_changed(struct slave *slave); > void bond_3ad_handle_link_change(struct slave *slave, char link); > int bond_3ad_get_active_agg_info(struct bonding *bond, struct ad_info *ad_info); > int bond_3ad_xmit_xor(struct sk_buff *skb, struct net_device *dev); >-int bond_3ad_lacpdu_recv(struct sk_buff *skb, struct bonding *bond, >- struct slave *slave); >+int bond_3ad_lacpdu_recv(const struct sk_buff *skb, struct bonding *bond, >+ struct slave *slave); > int bond_3ad_set_carrier(struct bonding *bond); > void bond_3ad_update_lacp_rate(struct bonding *bond); > #endif //__BOND_3AD_H__ >diff --git a/drivers/net/bonding/bond_alb.c b/drivers/net/bonding/bond_alb.c >index 0f59c15..ef3791a 100644 >--- a/drivers/net/bonding/bond_alb.c >+++ b/drivers/net/bonding/bond_alb.c >@@ -342,27 +342,17 @@ static void rlb_update_entry_from_arp(struct bonding *bond, struct arp_pkt *arp) > _unlock_rx_hashtbl_bh(bond); > } > >-static int rlb_arp_recv(struct sk_buff *skb, struct bonding *bond, >- struct slave *slave) >+static int rlb_arp_recv(const struct sk_buff *skb, struct bonding *bond, >+ struct slave *slave) > { >- struct arp_pkt *arp; >+ struct arp_pkt *arp, _arp; > > if (skb->protocol != cpu_to_be16(ETH_P_ARP)) > goto out; > >- arp = (struct arp_pkt *) skb->data; >- if (!arp) { >- pr_debug("Packet has no ARP data\n"); >+ arp = skb_header_pointer(skb, 0, sizeof(_arp), &_arp); >+ if (!arp) > goto out; >- } >- >- if (!pskb_may_pull(skb, arp_hdr_len(bond->dev))) >- goto out; >- >- if (skb->len < sizeof(struct arp_pkt)) { >- pr_debug("Packet is too small to be an ARP\n"); >- goto out; >- } > > if (arp->op_code == htons(ARPOP_REPLY)) { > /* update rx hash table for this ARP */ >diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c >index 2ee8cf9..9e2301e 100644 >--- a/drivers/net/bonding/bond_main.c >+++ b/drivers/net/bonding/bond_main.c >@@ -1444,8 +1444,8 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb) > struct sk_buff *skb = *pskb; > struct slave *slave; > struct bonding *bond; >- int (*recv_probe)(struct sk_buff *, struct bonding *, >- struct slave *); >+ int (*recv_probe)(const struct sk_buff *, struct bonding *, >+ struct slave *); > int ret = RX_HANDLER_ANOTHER; > > skb = skb_share_check(skb, GFP_ATOMIC); >@@ -1462,15 +1462,10 @@ static rx_handler_result_t bond_handle_frame(struct sk_buff **pskb) > > recv_probe = ACCESS_ONCE(bond->recv_probe); > if (recv_probe) { >- struct sk_buff *nskb = skb_clone(skb, GFP_ATOMIC); >- >- if (likely(nskb)) { >- ret = recv_probe(nskb, bond, slave); >- dev_kfree_skb(nskb); >- if (ret == RX_HANDLER_CONSUMED) { >- consume_skb(skb); >- return ret; >- } >+ ret = recv_probe(skb, bond, slave); >+ if (ret == RX_HANDLER_CONSUMED) { >+ consume_skb(skb); >+ return ret; > } > } > >@@ -2737,25 +2732,31 @@ static void bond_validate_arp(struct bonding *bond, struct slave *slave, __be32 > } > } > >-static int bond_arp_rcv(struct sk_buff *skb, struct bonding *bond, >- struct slave *slave) >+static int bond_arp_rcv(const struct sk_buff *skb, struct bonding *bond, >+ struct slave *slave) > { >- struct arphdr *arp; >+ struct arphdr *arp = (struct arphdr *)skb->data; > unsigned char *arp_ptr; > __be32 sip, tip; >+ int alen; > > if (skb->protocol != __cpu_to_be16(ETH_P_ARP)) > return RX_HANDLER_ANOTHER; > > read_lock(&bond->lock); >+ alen = arp_hdr_len(bond->dev); > > pr_debug("bond_arp_rcv: bond %s skb->dev %s\n", > bond->dev->name, skb->dev->name); > >- if (!pskb_may_pull(skb, arp_hdr_len(bond->dev))) >- goto out_unlock; >+ if (alen > skb_headlen(skb)) { >+ arp = kmalloc(alen, GFP_ATOMIC); >+ if (!arp) >+ goto out_unlock; >+ if (skb_copy_bits(skb, 0, arp, alen) < 0) >+ goto out_unlock; >+ } > >- arp = arp_hdr(skb); > if (arp->ar_hln != bond->dev->addr_len || > skb->pkt_type == PACKET_OTHERHOST || > skb->pkt_type == PACKET_LOOPBACK || >@@ -2790,6 +2791,8 @@ static int bond_arp_rcv(struct sk_buff *skb, struct bonding *bond, > > out_unlock: > read_unlock(&bond->lock); >+ if (arp != (struct arphdr *)skb->data) >+ kfree(arp); > return RX_HANDLER_ANOTHER; > } > >diff --git a/drivers/net/bonding/bonding.h b/drivers/net/bonding/bonding.h >index 4581aa5..f8af2fc 100644 >--- a/drivers/net/bonding/bonding.h >+++ b/drivers/net/bonding/bonding.h >@@ -218,8 +218,8 @@ struct bonding { > struct slave *primary_slave; > bool force_primary; > s32 slave_cnt; /* never change this value outside the attach/detach wrappers */ >- int (*recv_probe)(struct sk_buff *, struct bonding *, >- struct slave *); >+ int (*recv_probe)(const struct sk_buff *, struct bonding *, >+ struct slave *); > rwlock_t lock; > rwlock_t curr_slave_lock; > u8 send_peer_notif; > > -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists