On Fri, Dec 14, 2012 at 7:01 PM, Jesse Gross <[email protected]> wrote:
> On Thu, Nov 22, 2012 at 7:56 AM, Pravin B Shelar <[email protected]> > wrote: > > diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h > > index f2af494..585aca7 100644 > > --- a/include/linux/skbuff.h > > +++ b/include/linux/skbuff.h > > +static inline int unclone_skb(struct sk_buff *skb, gfp_t pri) > > I would call this skb_unclone() instead so that it is more clear what > section of code it belongs to. > > ok. > > diff --git a/net/ipv4/gre.c b/net/ipv4/gre.c > > index b837551..7568853 100644 > > --- a/net/ipv4/gre.c > > +++ b/net/ipv4/gre.c > > +static struct sk_buff *gre_gso_segment(struct sk_buff *skb, > > + netdev_features_t features) > > +{ > > + struct sk_buff *segs = ERR_PTR(-EINVAL); > > + struct gre_base_hdr *greh; > > + unsigned char *mac = skb_mac_header(skb); > > + int mac_len = skb->mac_len; > > + int net_hlen = skb_network_header_len(skb); > > + int doffset; > > + int ghl; > > + > > + if (unlikely(skb_shinfo(skb)->gso_type & > > + ~(SKB_GSO_TCPV4 | > > + SKB_GSO_UDP | > > + SKB_GSO_DODGY | > > + SKB_GSO_TCP_ECN | > > + SKB_GSO_GRE | > > + 0))) > > + goto out; > > + > > + if (unlikely(!pskb_may_pull(skb, sizeof(*greh)))) > > + goto out; > > + > > + greh = (struct gre_base_hdr *)skb_transport_header(skb); > > + ghl = gre_header_len(greh); > > + > > + if (unlikely(!pskb_may_pull(skb, ghl))) > > + goto out; > > + > > + __skb_pull(skb, ghl); > > + skb_reset_mac_header(skb); > > + skb_set_network_header(skb, skb->mac_len); > > I don't think it's safe to assume that the inner and outer MAC lengths > are the same. In an ideal world this would actually work for both > Ethernet and IP encapsulated GRE frames but even in the former case, > we could be running Ethernet over GRE over IP over Infiniband. > > > + doffset = skb_mac_header(skb) - mac; > > + /* segment inner packet. */ > > + segs = skb_gso_segment(skb, 0); > > + if (!segs || IS_ERR(segs)) > > + goto out; > > + > > + skb = segs; > > + do { > > + unsigned char *smac; > > + > > + skb_push(skb, doffset); > > + > > + skb_reset_mac_header(skb); > > + skb_set_network_header(skb, mac_len); > > + skb_set_transport_header(skb, > > + skb_network_offset(skb) + > net_hlen); > > + smac = skb_mac_header(skb); > > + skb->mac_len = mac_len; > > + /* Copy entire outer header from original skb. */ > > + memmove(smac, mac, doffset); > > All of this makes me wonder whether we should factor a MAC layer > handler out of skb_gso_segment(). That way we wouldn't need to have > to fool around with all the layer reseting and copying at this point > since it would just get handled by skb_segment(). > > ok, But we can also make use of inner offset added to skb, it is easier and efficient. > > +static int gre_gso_send_check(struct sk_buff *skb) > > +{ > > + return 0; > > +} > > Doesn't this have to call into the lower layers? > > > diff --git a/net/ipv4/ip_gre.c b/net/ipv4/ip_gre.c > > index 829fe3d..6b4e57f 100644 > > --- a/net/ipv4/ip_gre.c > > +++ b/net/ipv4/ip_gre.c > > static int ipgre_tap_init(struct net_device *dev) > > { > > + struct ip_tunnel *tunnel = netdev_priv(dev); > > __gre_tunnel_init(dev); > > + > > + if (!(tunnel->parms.o_flags & TUNNEL_SEQ)) { > > + dev->features |= NETIF_F_TSO; > > + dev->hw_features |= NETIF_F_TSO; > > + } > > Can't we implement support for sequence numbers in GSO? Also, what > about TSO6 and other TCP GSO features? > ok, I will add support for sequence number in gso. Thanks.
_______________________________________________ dev mailing list [email protected] http://openvswitch.org/mailman/listinfo/dev
