> 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). >
Agreed, would be interested in Darrells input here also. > 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. > I tested it with gcc (GCC) 6.3.1. > But I agree the incremental is a better practice. > > Thanks, > Tiago. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
