Simon Horman <[email protected]> 于2022年11月9日周三 02:48写道：
> On Fri, Nov 04, 2022 at 06:46:05AM +0000, Peng He wrote:
> > The tail of the struct dp_netdev_flow contains a whole netdev_flow_key
> > struct.
> >
> > We need to first minus the size of netdev_flow_key then add back
> > the real size of this netdev_flow_key.
>
> Hi Peng,
>
> It seems to me that as a result of this change the space allocated for
> struct dp_netdev_flow will be truncated just before the cr.flow.mf field,
> rather than just before the cf.flow field.
>
Yes, because the mask.len will be used for the space after cr.flow.mf.
the netdev_flow_key is defined with a maximum length of bytes that one
flow key can have:
/* Must be public as it is instantiated in subtable struct below. */
struct netdev_flow_key {
uint32_t hash; /* Hash function differs for different users. */
uint32_t len; /* Length of the following miniflow (incl. map).
*/
struct miniflow mf;
uint64_t buf[FLOW_MAX_PACKET_U64S];
};
that is to say, the "real" space of a netdev_flow_key
should be offsetof(struct netdev_flow_key, mf) + mask.len, not
sizeof(netdev_flow_key).
the dp_netdev_flow contains the whole netdev_flow_key as cr.key.
We should first minus the sizeof cr.key, then, use the
offsetof(struct netdev_flow_key, mf) + mask.len, not
sizeof(netdev_flow_key).
to recalculate the size.
> And that as a result space is allocated for the cr.flow.hash and
> cr.flow.len
> fields.
>
> But, based on the patch description, it is not clear to me why this change
> is being made. Perhaps it would be useful to add that information to the
> patch description.
>
cr.flow.
>
> > Signed-off-by: Peng He <[email protected]>
> > ---
> > lib/dpif-netdev.c | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
> > index 2c08a71c8..a8779a979 100644
> > --- a/lib/dpif-netdev.c
> > +++ b/lib/dpif-netdev.c
> > @@ -4072,7 +4072,8 @@ dp_netdev_flow_add(struct dp_netdev_pmd_thread
> *pmd,
> > && !FLOWMAP_HAS_FIELD(&mask.mf.map, regs));
> >
> > /* Do not allocate extra space. */
> > - flow = xmalloc(sizeof *flow - sizeof flow->cr.flow.mf + mask.len);
> > + flow = xmalloc(sizeof *flow - sizeof(flow->cr.flow) +
> > + offsetof(struct netdev_flow_key, mf) + mask.len);
> > memset(&flow->stats, 0, sizeof flow->stats);
> > atomic_init(&flow->netdev_flow_get_result, 0);
> > memset(&flow->last_stats, 0, sizeof flow->last_stats);
> > @@ -9744,7 +9745,8 @@ dpcls_create_subtable(struct dpcls *cls, const
> struct netdev_flow_key *mask)
> >
> > /* Need to add one. */
> > subtable = xmalloc(sizeof *subtable
> > - - sizeof subtable->mask.mf + mask->len);
> > + - sizeof subtable->mask
> > + + offsetof(struct netdev_flow_key, mf) +
> mask->len);
> > cmap_init(&subtable->rules);
> > subtable->hit_cnt = 0;
> > netdev_flow_key_clone(&subtable->mask, mask);
> > --
> > 2.25.1
> >
> > _______________________________________________
> > dev mailing list
> > [email protected]
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
--
hepeng
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
