[ovs-dev] [PATCHv6] userspace: Add GTP-U support.

[William Tu]

On Fri, Jan 17, 2020 at 02:14:27PM -0800, Ben Pfaff wrote:
> On Thu, Jan 16, 2020 at 11:45:20AM -0800, William Tu wrote:
> > GTP, GPRS Tunneling Protocol, is a group of IP-based communications
> > protocols used to carry general packet radio service (GPRS) within
> > GSM, UMTS and LTE networks.  GTP protocol has two parts: Signalling
> > (GTP-Control, GTP-C) and User data (GTP-User, GTP-U). GTP-C is used
> > for setting up GTP-U protocol, which is an IP-in-UDP tunneling
> > protocol. Usually GTP is used in connecting between base station for
> > radio, Serving Gateway (S-GW), and PDN Gateway (P-GW).
> > 
> > This patch implements GTP-U protocol for userspace datapath,
> > supporting only required header fields and G-PDU message type.
> > See spec in:
> > https://tools.ietf.org/html/draft-hmm-dmm-5g-uplane-analysis-00
> > 
> > Signed-off-by: Yi Yang <yangyi01 at inspur.com>
> > Co-authored-by: Yi Yang <yangyi01 at inspur.com>
> > Signed-off-by: William Tu <u9012063 at gmail.com>
> 
> Thanks for the patch!
> 
> I have only a few comments.
> 
> To me, the "update" in the name netdev_tnl_update_udp_csum() suggests
> that it's starting from an existing checksum and then adding a few more
> things into it.  I think that it calculates the whole checksum from the
> beginning.  I would say "calculate" or "calc" or "set" instead of
> "update".
> 
> In netdev_gtpu_push_header(), I think that be32_to_be16(htonl(...)) is
> the same as htons().
> 
> In ovs_parse_tnl_push(), I suggest using "%"SCNi8, etc., instead of
> "0x%"SCNx8.  That allows the user to specify the values in decimal if
> desired.  (That is particularly desirable if the value is 0, since it's
> not really a normal thing to write 0x0 if one does not need to.)
> 
> Acked-by: Ben Pfaff <blp at ovn.org>

Thanks Ben,
I will fix the above and send v7.
Probably wait for a couple days to see if there is any more feedbacks.

Regards,
William