On Thu, Apr 9, 2015 at 11:00 PM, Pravin B Shelar <pshe...@nicira.com> wrote:
> diff --git a/NEWS b/NEWS
> index 87460a7..6e5e070 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -77,6 +77,7 @@ Post-v2.3.0
> numbers. OpenFlow is 6653 and OVSDB is 6640.
> - Support for DPDK vHost.
> - Support for outer UDP checksums in Geneve and VXLAN.
> + - Support for STT tunneling.
I think there is at least one question in the FAQ that has a table of
tunnels that should probably be updated as well.
> diff --git a/datapath/linux/compat/stt.c b/datapath/linux/compat/stt.c
> new file mode 100644
> index 0000000..0b32d1d
> --- /dev/null
> +++ b/datapath/linux/compat/stt.c
> +static int segment_skb(struct sk_buff **headp, bool ipv4, bool tcp, int
> l4_offset)
> +{
> + int err;
> +
> + err = coalesce_skb(headp);
> + if (err)
> + return err;
> +
> + if ((*headp)->next && !can_segment(*headp, ipv4, tcp, true))
> + return skb_list_linearize(*headp, GFP_ATOMIC);
> +
> + return skb_list_segment(*headp, ipv4, l4_offset);
> +}
We should only call skb_list_segment() if we have a next pointer, right?
> +static struct sk_buff *handle_offloads(struct sk_buff *skb, int min_headroom)
> +{
> + int err;
> +
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,10,0)
> + if (skb_vlan_tag_present(skb) && skb->vlan_proto !=
> htons(ETH_P_8021Q)) {
> +
> + min_headroom += VLAN_HLEN;
> + if (skb_headroom(skb) < min_headroom) {
> + int head_delta = SKB_DATA_ALIGN(min_headroom -
> + skb_headroom(skb) +
> 16);
> +
> + err = pskb_expand_head(skb, max_t(int, head_delta, 0),
> + 0, GFP_ATOMIC);
> + if (unlikely(err))
> + goto error;
> + }
> +
> + skb = vlan_hwaccel_push_inside(skb);
> + if (!skb) {
> + err = -ENOMEM;
> + goto error;
> + }
> + skb->vlan_tci = 0;
vlan_hwaccel_push_inside() clears vlan_tci, so we don't need to. And I
guess that we could us __vlan_hwaccel_push_inside() since we already
checked that a tag is present.
> +int stt_xmit_skb(struct sk_buff *skb, struct rtable *rt,
> + __be32 src, __be32 dst, __u8 tos,
> + __u8 ttl, __be16 df, __be16 src_port, __be16 dst_port,
> + __be64 tun_id)
> +{
> + struct ethhdr *eh = eth_hdr(skb);
> + struct iphdr *iph = ip_hdr(skb);
What about inner IPv6?
> + int ret = 0, min_headroom;
> + __be16 inner_h_proto;
> + u8 inner_nw_proto;
The name inner_nw_proto is probably a little misleading - it makes it
seem like it is the L3 protocol but it is actually L4.
> + inner_h_proto = eh->h_proto;
> + inner_nw_proto = iph->protocol;
The concern that I had about pskb_may_pull() on the transmit path
applies here as well (and in stt_can_offload) - we access these inner
headers without any verification other than the preceding OVS flow
extract.
> +static bool set_offloads(struct sk_buff *skb)
> +{
[...]
> + skb->vlan_tci = ntohs(stth->vlan_tci);
> +#if LINUX_VERSION_CODE >= KERNEL_VERSION(3,10,0)
> + /* STT can only offload 8021Q */
> + if (skb_vlan_tag_present(skb))
> + skb->vlan_proto = htons(ETH_P_8021Q);
> +#endif
I think you can simplify this by using __vlan_hwaccel_put_tag(). It
has a macro that chops out the vlan_proto field on kernels that don't
support it.
> +static unsigned int nf_ip_hook(FIRST_PARAM
> + struct sk_buff *skb,
> + const struct net_device *in,
> + const struct net_device *out,
> + int (*okfn)(struct sk_buff *))
> +{
[...]
> + stt_sock = stt_find_sock(dev_net(skb->dev), tcp_hdr(skb)->dest);
> + if (unlikely(!stt_sock))
> + return NF_ACCEPT;
I would probably drop the unlikely() here since this check will see
all TCP traffic.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
