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.
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.
>
> 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
