Hi Ian, On 07/08/2018 13:08, Stokes, Ian wrote: >> In its current implementation dp_packet_shift() is also unaware of multi- >> seg mbufs (that holds data in memory non-contiguously) and assumes that >> data exists contiguously in memory, memmove'ing data to perform the shift. >> >> To add support for multi-seg mbuds a new set of functions was introduced, >> dp_packet_mbuf_shift() and dp_packet_mbuf_write(). These functions are >> used by dp_packet_shift(), when handling multi-seg mbufs, to shift and >> write data within a chain of mbufs. >> > > Hi Tiago, > > After applying this patch I see the following error during compilation. > > lib/conntrack.c: In function 'handle_ftp_ctl': > lib/conntrack.c:3154:11: error: 'addr_offset_from_ftp_data_start' may be used > uninitialized in this function [-Werror=maybe-uninitialized] > char *pkt_addr_str = ftp_data_start + addr_offset_from_ftp_data_start; > ^~~~~~~~~~~~ > lib/conntrack.c:3173:12: note: 'addr_offset_from_ftp_data_start' was declared > here > size_t addr_offset_from_ftp_data_start; > > It can be fixed with the following incremental. > > diff --git a/lib/conntrack.c b/lib/conntrack.c > index 974f985..7cd1fc9 100644 > --- a/lib/conntrack.c > +++ b/lib/conntrack.c > @@ -3170,7 +3170,7 @@ handle_ftp_ctl(struct conntrack *ct, const struct > conn_lookup_ctx *ctx, > struct ip_header *l3_hdr = dp_packet_l3(pkt); > ovs_be32 v4_addr_rep = 0; > struct ct_addr v6_addr_rep; > - size_t addr_offset_from_ftp_data_start; > + size_t addr_offset_from_ftp_data_start = 0; > size_t addr_size = 0; > char *ftp_data_start; > bool do_seq_skew_adj = true; > > If there are no objections I can roll this into the patch upon application to > dpdk merge. > > Ian >
I see no problem in including the incremental (I also see Darrell is already CC'ed, since this is in the userspace Conntrack). I noticed this myself during testing (are you perhaps using GCC 5.4?), but seems to be a false positive as when I looked there shouldn't be a path where `addr_offset_from_ftp_data_start` is used uninitialized, and when trying to compile with GCC 4.8 and GCC 7.3 there's no such warning. But I agree the incremental is a better practice. Thanks, Tiago. _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
