Sure. Will do before the merge.
Thanks for the reviews Ethan.
thanx!
mehak
On Thu, Aug 2, 2012 at 3:58 PM, Ethan Jackson wrote:
> Looks good to me.
>
> > #include "packets.h"
> > #include "unaligned.h"
> > #include "vlog.h"
> > +#include "csum.h"
>
> Minor nit-pick, could you please insert t
Looks good to me.
> #include "packets.h"
> #include "unaligned.h"
> #include "vlog.h"
> +#include "csum.h"
Minor nit-pick, could you please insert this in sorted order (i.e.
after coverage.h). No need to resend the patch just for this change,
go ahead and merge.
Ethan
___
OVS provides a utility to create IP packets for the purpose of testing
using ovs-appctl netdev-dummy/receive. These packets created by
flow_compose() earlier did not have the IP checksum in them. With this
commit, the checksum with be added to these test IP packets.
Signed-off-by: Mehak Mahajan
Sounds good.
You may want to wait a bit to resend it, I sent out a bug fix patch
which should probably go in first.
Ethan
On Wed, Aug 1, 2012 at 1:19 PM, Mehak Mahajan wrote:
> Sure ... Makes sense.
> I will modify it, and since practically the complete patch is changed, I
> will sent out anoth
Sure ... Makes sense.
I will modify it, and since practically the complete patch is changed, I
will sent out another patch.
Thanks!
Mehak
On Wed, Aug 1, 2012 at 12:57 PM, Ethan Jackson wrote:
> > I agree its cleaner, but I am still inclined to keep it as is because
> sizeof
> > *ip does not con
> I agree its cleaner, but I am still inclined to keep it as is because sizeof
> *ip does not contain the options which should also be included in computing
> the IP checksum. IP_IHL takes the header length and uses that which is what
> the IP Checksum calculations should use. What do you think ?
I agree its cleaner, but I am still inclined to keep it as is because
sizeof *ip does not contain the options which should also be included in
computing the IP checksum. IP_IHL takes the header length and uses that
which is what the IP Checksum calculations should use. What do you think ?
thanx!
> +ip->ip_csum = csum(ip, IP_IHL(ip->ip_ihl_ver) * 4);
I think what you have here is correct, but would be a bit
safer/cleaner if we changed it to:
ip->ip_csum = csum(ip, sizeof *ip);
Ethan
> } else if (flow->dl_type == htons(ETH_TYPE_IPV6)) {
> /* XXX */
> } else if
Thanks for pointing that out. I have modified the comment to be in line
with the changes.
diff --git a/lib/flow.c b/lib/flow.c
index bc88718..fd31333 100644
--- a/lib/flow.c
+++ b/lib/flow.c
@@ -1010,7 +1010,8 @@ flow_set_vlan_pcp(struct flow *flow, uint8_t pcp)
* 'flow'.
*
* (This is useful
> * (This is useful only for testing, obviously, and the packet isn't really
> - * valid. It hasn't got any checksums filled in, for one, and lots of fields
> - * are just zeroed.) */
> + * valid. There are lots of fields that are just zeroed. */
I think you either need to compute the l4 checks
OVS provides a utility to create IP packets for the purpose of testing
using ovs-appctl netdev-dummy/receive. These packets created by
flow_compose() earlier did not have the IP checksum in them. With this
commit, the checksum with be added to these test IP packets.
Signed-off-by: Mehak Mahajan
11 matches
Mail list logo
