| Message ID | 20160311001826.GP1684@ovn.org |
|---|---|
| State | Not Applicable |
| Headers | show |
On Thu, Mar 10, 2016 at 4:18 PM, Ben Pfaff <blp@ovn.org> wrote: > On Wed, Mar 09, 2016 at 04:40:42PM -0800, Pravin B Shelar wrote: >> Following patch fixes number of issues with compose nd, like >> setting ip packet header, set ICMP opt-len, checksum. >> >> Signed-off-by: Pravin B Shelar <pshelar@ovn.org> > > sparse says: > > ../lib/packets.c:1332:45: warning: incorrect type in argument 2 (different base types) > ../lib/packets.c:1332:45: expected restricted ovs_be32 [usertype] <noident> > ../lib/packets.c:1332:45: got restricted ovs_be16 > > to fix: > ok. > diff --git a/lib/packets.c b/lib/packets.c > index 6e2c68b..c414694 100644 > --- a/lib/packets.c > +++ b/lib/packets.c > @@ -1329,7 +1329,7 @@ compose_nd(struct dp_packet *b, const struct eth_addr eth_src, > > ns->icmph.icmp6_type = ND_NEIGHBOR_SOLICIT; > ns->icmph.icmp6_code = 0; > - put_16aligned_be32(&ns->rco_flags, htons(0)); > + put_16aligned_be32(&ns->rco_flags, htonl(0)); > > nd_opt = &ns->options[0]; > nd_opt->nd_opt_type = ND_OPT_SOURCE_LINKADDR; > >> @@ -845,7 +845,7 @@ packet_rh_present(struct dp_packet *packet) >> size_t remaining; >> uint8_t *data = dp_packet_l3(packet); >> >> - remaining = packet->l4_ofs - packet->l3_ofs; >> + remaining = dp_packet_size(packet) - packet->l3_ofs; > > Isn't this only correct if the packet has no header, that is, if > packet->data_ofs == 0? > right, I will keep current code. >> if (remaining < sizeof *nh) { >> return false; > >> +/* This function expect packet with ehernet header with correct > > s/ehernet/Ethernet/ > ok. >> + * l3 pointer set. */ >> +static void * >> +compose_ipv6(struct dp_packet *packet, uint8_t proto, const ovs_be32 src[4], >> + const ovs_be32 dst[4], uint8_t key_tc, ovs_be32 key_fl, >> + uint8_t key_hl, int size) > > The indentation of the two lines above is a few spaces too far to the > right and doesn't align with the (. > > It would be nice to add a test for the composition functions, so that > any regression later gets noticed. ok
diff --git a/lib/packets.c b/lib/packets.c index 6e2c68b..c414694 100644 --- a/lib/packets.c +++ b/lib/packets.c @@ -1329,7 +1329,7 @@ compose_nd(struct dp_packet *b, const struct eth_addr eth_src, ns->icmph.icmp6_type = ND_NEIGHBOR_SOLICIT; ns->icmph.icmp6_code = 0; - put_16aligned_be32(&ns->rco_flags, htons(0)); + put_16aligned_be32(&ns->rco_flags, htonl(0)); nd_opt = &ns->options[0]; nd_opt->nd_opt_type = ND_OPT_SOURCE_LINKADDR;