-----Original Message-----
From: "Chandran, Sugesh" <[email protected]>
Date: Wednesday, August 9, 2017 at 12:55 PM
To: Darrell Ball <[email protected]>, Ben Pfaff <[email protected]>
Cc: "[email protected]" <[email protected]>
Subject: RE: [ovs-dev] [patch_v5] dp-packet: Reset DPDK HWOL checksum flags on
init.
Regards
_Sugesh
<snip>
> > >
> > > Correct, I reused reset_dp_packet_checksum_ol_flags() to do
the
> > initialization as well
> > > I could also have created a separate function.
> > >
> > > In case a DPDK dev is used, those flags will be managed by
DPDK.
> > >
> > > That sounds like a
> > > bug in itself--is there a missing call to initialize the
mbuf
> somewhere?
> > >
> > > Are you suggesting to initialize the whole mbuf for each
packet ?
> >
> > The issue that I'm raising is that it's unusual to take an
> > uninitialized, indeterminate field, and then initialize it by
clearing a
> > 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
> of).
> 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
existing
> memory is mapped to
> Mbuf.
> 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
some
> flags, so that is fine.
> It will be done twice in the case of a packet received from a dpdk
device, but
> it is a small cost.
> I was more concerned about the dual responsibility conflict when packets
are
> 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 –
meaning, it
> would be done twice for each packet.
>
[Sugesh] No, we don’t have to initialize all the fields. But we can have a
generic function
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
called twice.
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
port initiation.
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;
> rte_pktmbuf_reset_headroom(m);
>
> m->data_len = 0;
> __rte_mbuf_sanity_check(m, 1);
> }
>
> >
> >
> > That made me wonder whether there's a larger problem of a
failure to
> > initialize the mbuf more generally, although of course that
does not
> > necessarily follow.
> >
> >
> > _______________________________________________
> > dev mailing list
> > [email protected]
> > https://urldefense.proofpoint.com/v2/url?u=https-
> 3A__mail.openvswitch.org_mailman_listinfo_ovs-
> 2Ddev&d=DwIFAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-
> uZnsw&m=wM40bZgtG4Ul1nb53ufLGymPOplaXttP5W6_wL2w_XA&s=05T6d
> 75pGHINP78-6gwS87E3IhAXzHojsFzyNSDdaTY&e=
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
