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
>     > d...@openvswitch.org
>     > 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
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to