From: "Chandran, Sugesh" <sugesh.chand...@intel.com>
Date: Wednesday, August 9, 2017 at 12:55 PM
To: Darrell Ball <db...@vmware.com>, Ben Pfaff <b...@ovn.org>
Cc: "d...@openvswitch.org" <d...@openvswitch.org>
Subject: RE: [ovs-dev] [patch_v5] dp-packet: Reset DPDK HWOL checksum flags on
> > >
> > > Correct, I reused reset_dp_packet_checksum_ol_flags() to do
> > initialization as well
> > > I could also have created a separate function.
> > >
> > > In case a DPDK dev is used, those flags will be managed by
> > >
> > > That sounds like a
> > > bug in itself--is there a missing call to initialize the
> > >
> > > Are you suggesting to initialize the whole mbuf for each
> > The issue that I'm raising is that it's unusual to take an
> > uninitialized, indeterminate field, and then initialize it by
> > few bits. It's much more conventional to initialize it by
setting it to
> > zero, like this:
> > p->mbuf.ol_flags = 0;
> > That is better; I will create a separate function then.
> > I will resend
> > Thanks
> [Sugesh] I also agree with Ben here.
> Currently OVS uses only checksum offload flags from mbuf(As I am aware
> But there are other flag bits that may get used in future like TSO.
> So its better to initialize the mbuf properly before using.
> Here is the mbuf reset function in DPDK that get called when an
> memory is mapped to
> I believe only the ol_flags are relevant for now in OVS.
> There is no higher cost associated with initializing all the ol_flags vs
> flags, so that is fine.
> It will be done twice in the case of a packet received from a dpdk
> it is a small cost.
> I was more concerned about the dual responsibility conflict when packets
> received from a DPDK device and this is why I wanted to limit the scope of
> the flag management in OVS; it should be ok, though.
> Hence, I mentioned that I will initialize all the ol_flags.
> JTBC, are you suggesting to initialize all the fields below ?
> This would mean when packets are received from a DPDK dev, both the rte
> library and OVS would separately initialize all the fields below –
> would be done twice for each packet.
[Sugesh] No, we don’t have to initialize all the fields. But we can have a
to init all mbuf fields that are relevant in OVS.
For now its only ol_flags. In the future this function must be updated when
we use more
fields from rte_mbuf.
We are on the same page then.
I plan for the function to have a generic name, so this fine.
Sorry, I really didn’t get the case you mentioned above, where the init get
What I can see in the code is , during the dpdk mp init, the
the dp_packet_init is called for rte_mbuf initialization. This happens at
On packet reception, there wont any call to dp_packet_init.
I meant for the memory related to packets that are received, not during the
reception time itself.
However, the memory will be reused for subsequent packets, right. You can’t use
a piece of memory
once and never use it again – this is called a memory leak. This means each
mbuf will need to be initialized again and again.
> static inline void rte_pktmbuf_reset(struct rte_mbuf *m)
> m->next = NULL;
> m->pkt_len = 0;
> m->tx_offload = 0;
> m->vlan_tci = 0;
> m->vlan_tci_outer = 0;
> m->nb_segs = 1;
> m->port = 0xff;
> m->ol_flags = 0;
> m->packet_type = 0;
> m->data_len = 0;
> __rte_mbuf_sanity_check(m, 1);
> > That made me wonder whether there's a larger problem of a
> > initialize the mbuf more generally, although of course that
> > necessarily follow.
> > _______________________________________________
> > dev mailing list
> > d...@openvswitch.org
> > https://urldefense.proofpoint.com/v2/url?u=https-
dev mailing list