Hi Konstantin,
On Thu, Jun 29, 2017 at 01:41:40AM +0800, Ananyev, Konstantin wrote:
>
> Hi Jiayu,
>
> >
> > >
> > > > +
> > > > +/**
> > > > + * GRO table, which is used to merge packets. It keeps many reassembly
> > > > + * tables of desired GRO types. Applications need to create GRO tables
> > > > + * before using rte_gro_reassemble to perform GRO.
> > > > + */
> > > > +struct rte_gro_tbl {
> > > > + uint64_t desired_gro_types; /**< GRO types to perform */
> > > > + /* max TTL measured in nanosecond */
> > > > + uint64_t max_timeout_cycles;
> > > > + /* max length of merged packet measured in byte */
> > > > + uint32_t max_packet_size;
> > > > + /* reassebly tables of desired GRO types */
> > > > + void *tbls[GRO_TYPE_MAX_NUM];
> > > > +};
> > >
> > > Not sure why do you need to define that structure here.
> > > As I understand it is internal to the library.
> > > Just declaration should be enough.
> >
> > This structure defines a GRO table, which is used by rte_gro_reassemble
> > to merge packets. Applications need to create this table before calling
> > rte_gro_reassemble. So I define it in rte_gro.h.
>
> Yes, application has to call gro_table_create().
> But application don't need to access contents of struct rte_gro_tbl,
> which means at it can (and should) treat it as opaque pointer.
Thanks, I will modify it.
>
> > > > +
> > > > +/**
> > > > + * Reassembly function, which tries to merge the inputted packet with
> > > > + * one packet in a given GRO table. This function assumes the inputted
> > > > + * packet is with correct checksums. And it won't update checksums if
> > > > + * two packets are merged. Besides, if the inputted packet is IP
> > > > + * fragmented, this function assumes it's a complete packet (i.e. with
> > > > + * L4 header).
> > > > + *
> > > > + * If the inputted packet doesn't have data or it's with unsupported
> > > > GRO
> > > > + * type, function returns immediately. Otherwise, the inputted packet
> > > > is
> > > > + * either merged or inserted into the table. If applications want get
> > > > + * packets in the table, they need to call flush APIs.
> > > > + *
> > > > + * @param pkt
> > > > + * packet to reassemble.
> > > > + * @param gro_tbl
> > > > + * a pointer points to a GRO table.
> > > > + * @return
> > > > + * if merge the packet successfully, return a positive value. If fail
> > > > + * to merge, return zero. If the packet doesn't have data, or its GRO
> > > > + * type is unsupported, return a negative value.
> > > > + */
> > > > +int rte_gro_reassemble(struct rte_mbuf *pkt,
> > > > + struct rte_gro_tbl *gro_tbl);
> > >
> > >
> > > Ok, and why tbl one can't do bursts?
> >
> > In current design, if applications want to do bursts, they don't need to
> > create gro_tbl. rte_gro_reassemble_burst will create a temporary table
> > in stack. So when do bursts (we call it lightweight mode), the operations
> > of applications is very simple: calling rte_gro_reassemble_burst. And
> > after rte_gro_reassemble_burst returns, applications can get all merged
> > packets. rte_gro_reassemble is another mode API, called heavyweight mode.
> > The gro_tbl is just used in rte_gro_reassemble. rte_gro_reassemble just
> > processes one packet at a time.
> >
> > So you mean: we should enable rte_gro_reassemble to merge N inputted
> > packets with the packets in a given gro_tbl?
>
> Yes, I suppose that will be faster.
Thanks, I will enable it to process N packets at a time.
>
> >
> > >
> > >
> > > > +
> > > > +/**
> > > > + * This function flushed packets from reassembly tables of desired GRO
> > > > + * types. It won't re-calculate checksums for merged packets in the
> > > > + * tables. That is, the returned packets may be with wrong checksums.
> > > > + *
> > > > + * @param gro_tbl
> > > > + * a pointer points to a GRO table object.
> > > > + * @param desired_gro_types
> > > > + * GRO types whose packets will be flushed.
> > > > + * @param out
> > > > + * a pointer array that is used to keep flushed packets.
> > > > + * @param nb_out
> > > > + * the size of out.
> > > > + * @return
> > > > + * the number of flushed packets. If no packets are flushed, return 0.
> > > > + */
> > > > +uint16_t rte_gro_flush(struct rte_gro_tbl *gro_tbl,
> > > > + uint64_t desired_gro_types,
> > > > + struct rte_mbuf **out,
> > > > + const uint16_t max_nb_out);
> > > > +
> > > > +/**
> > > > + * This function flushes the timeout packets from reassembly tables of
> > > > + * desired GRO types. It won't re-calculate checksums for merged
> > > > packets
> > > > + * in the tables. That is, the returned packets may be with wrong
> > > > + * checksums.
> > > > + *
> > > > + * @param gro_tbl
> > > > + * a pointer points to a GRO table object.
> > > > + * @param desired_gro_types
> > > > + * rte_gro_timeout_flush only processes packets which belong to the
> > > > + * GRO types specified by desired_gro_types.
> > > > + * @param out
> > > > + * a pointer array that is used to keep flushed packets.
> > > > + * @param nb_out
> > > > + * the size of out.
> > > > + * @return
> > > > + * the number of flushed packets. If no packets are flushed, return 0.
> > > > + */
> > > > +uint16_t rte_gro_timeout_flush(struct rte_gro_tbl *gro_tbl,
> > > > + uint64_t desired_gro_types,
> > > > + struct rte_mbuf **out,
> > > > + const uint16_t max_nb_out);
> > >
> > > No point to have 2 flush() functions.
> > > I suggest to merge them together.
> >
> > rte_gro_flush flush all packets from table, but rte_gro_timeout_flush only
> > flush timeout packets. They have different operations. But if we merge them
> > together, we need to flush all or only timeout ones?
>
> We can specify that if timeout is zero (or less then current time) then
> we flush all packets.
Thanks, I will merge them together.
