Hi Roni,
Thanks for your feedbacks. See my reply below.
On Tue, Jan 21, 2020 at 11:20 AM Roni Bar Yanai <ron...@mellanox.com> wrote:
>
> Hi William,
>
> Two comments, please inline.
>
<snip>
> > struct dp_packet *
> >+netdev_gtpu_pop_header(struct dp_packet *packet)
> >+{
> >+ struct pkt_metadata *md = &packet->md;
> >+ struct flow_tnl *tnl = &md->tunnel;
> >+ struct gtpuhdr *gtph;
> >+ unsigned int gtpu_hlen;
> >+ unsigned int hlen;
> >+
> >+ ovs_assert(packet->l3_ofs > 0);
> >+ ovs_assert(packet->l4_ofs > 0);
> >+
> >+ pkt_metadata_init_tnl(md);
> >+ if (GTPU_HLEN > dp_packet_l4_size(packet)) {
> >+ goto err;
> >+ }
> >+
> >+ gtph = udp_extract_tnl_md(packet, tnl, &hlen);
> >+ if (!gtph) {
> >+ goto err;
> >+ }
> >+
> >+ tnl->gtpu_flags = gtph->md.flags;
> >+ tnl->gtpu_msgtype = gtph->md.msgtype;
> >+ tnl->tun_id = be32_to_be64(get_16aligned_be32(>ph->teid));
> >+
> >+ if (tnl->gtpu_msgtype == GTPU_MSGTYPE_GPDU) {
> >+ struct ip_header *ip;
> >+
> >+ if (gtph->md.flags & GTPU_S_MASK) {
> >+ gtpu_hlen = GTPU_HLEN + sizeof(struct gtpuhdr_opt);
> >+ } else {
> >+ gtpu_hlen = GTPU_HLEN;
> >+ }
> >+ ip = ALIGNED_CAST(struct ip_header *, (char *)gtph + gtpu_hlen);
> >+
> >+ if (IP_VER(ip->ip_ihl_ver) == 4) {
> >+ packet->packet_type = htonl(PT_IPV4);
> >+ } else if (IP_VER(ip->ip_ihl_ver) == 6) {
> >+ packet->packet_type = htonl(PT_IPV6);
> >+ } else {
> >+ VLOG_WARN_RL(&err_rl, "GTP-U: Receive non-IP packet.");
> >+ }
> >+ dp_packet_reset_packet(packet, hlen + gtpu_hlen);
>
> GTP internal packet has no l2, so stripping it without putting some dummy
> L2 or keeping outer one, sounds like a problem. How OVS can continue
> the processing? Or do expect user to do explicit encap?
Similar to L3 GRE and VxLAN tunnel, I think it's already handled by OVS
when we set the tunnel to be TNL_L3. If you look at the tests/tunnel.at in
this patch, OVS will push a dummy ethernet header
AT_SETUP([tunnel - GTP-U push and pop])
+dnl receive packet from GTP-U port, match it, and output to layer3 GRE
...
+Datapath actions: push_eth(src=00:00:00:00:00:00,dst=00:00:00:00:00:00),1
> >+ } else {
> >+ /* non-GPDU GTP-U messages, ex: echo request, end marker.
> >+ * Users should redirect these packets to controller, or.
> >+ * any application that handles GTP-U messages, so keep
> >+ * the original packet.
> >+ */
> >+ packet->packet_type = htonl(PT_ETH);
> >+ VLOG_WARN_ONCE("Receive non-GPDU msgtype: %"PRIu8,
> >+ gtph->md.msgtype);
> >+ }
> >+
> >+ return packet;
> >+
> >+err:
> >+ dp_packet_delete(packet);
> >+ return NULL;
> >+}
> >+
> >+void
> >+netdev_gtpu_push_header(const struct netdev *netdev,
> >+ struct dp_packet *packet,
> >+ const struct ovs_action_push_tnl *data)
> >+{
> >+ struct netdev_vport *dev = netdev_vport_cast(netdev);
> >+ struct netdev_tunnel_config *tnl_cfg;
> >+ struct udp_header *udp;
> >+ struct gtpuhdr *gtpuh;
> >+ int ip_tot_size;
> >+ unsigned int payload_len;
> >+
> >+ payload_len = dp_packet_size(packet);
> >+ udp = netdev_tnl_push_ip_header(packet, data->header,
> >+ data->header_len, &ip_tot_size);
>
> Same here, you need to pop the l2 before pushing the tunnel.
>
Yes, I think it's also done by OVS already. See tests/tunnel.at
AT_SETUP([tunnel - GTP-U push and pop])
+dnl Encap: in_port=1,actions=3
...
+ [Datapath actions:
set(tunnel(tun_id=0x3,dst=1.1.1.1,ttl=64,tp_dst=2152,flags(df|key))),pop_eth,2152
Regards,
William
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
