On Sun, Jan 10, 2016 at 11:18 PM, Pravin B Shelar <pshe...@nicira.com> wrote:
> diff --git a/lib/packets.c b/lib/packets.c
> index d82341d..a910f50 100644
> --- a/lib/packets.c
> +++ b/lib/packets.c
> +void
> +IP_ECN_set_ce(struct dp_packet *pkt, bool is_ipv6)
> +{
> + if (is_ipv6) {
> + struct ovs_16aligned_ip6_hdr *ip6 = dp_packet_l3(pkt);
> +
> + put_16aligned_be32((ovs_16aligned_be32 *)ip6, htonl(IP_ECN_CE <<
> 20));
Isn't this going to blow away the rest of the things that are already
in that word?
> + } else {
> + struct ip_header *nh = dp_packet_l4(pkt);
This should be the L3 header, right?
> + uint8_t tos = nh->ip_tos;
> +
> + tos |= IP_ECN_CE;
> + if (nh->ip_tos != tos) {
> + uint8_t *field = &nh->ip_tos;
> +
> + nh->ip_csum = recalc_csum16(nh->ip_csum, htons((uint16_t)
> *field),
> + htons((uint16_t) tos));
> + *field = tos;
Can we just pass the original nh->ip_tos in instead of using the field
pointer? It seems simpler.
What about checking to see if the packet is originally ECT or even IP?
It doesn't inherently have to be done in this function but skipping
ahead to the STT patch I don't see that we check when it is called. (I
see the check for IPv6 but it seems to me that it will interpret all
non-IP traffic as IPv4.)
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
