Hi Harald,
On 7/13/17, 12:26 AM, "Harald Welte"
<laforge(a)gnumonks.org> wrote:
·
static inline void gtp_set_pktinfo_ipv4(struct
gtp_pktinfo *pktinfo,
struct sock *sk, struct iphdr *iph,
- struct pdp_ctx *pctx, struct rtable *rt,
- struct flowi4 *fl4,
+ struct rtable *rt, struct flowi4 *fl4,
struct net_device *dev)
{
[...]
+ __be32 tun_id;
·
you are breaking GTPv0 functionality here. GTPv0 has 64 bit tunnel
identifiers, and this function is called both from GTPv1 and GTPv0
context.
·
This makes me wonder how you did verify that your changes do not break
the existing operation with both GTPv0 and GTPv1?
·
Good catch. I only fully tested the GTPv1 path against oai-cn. Will fix
this and test the GTPv0 path as well.
I had doubts on how this flow-based GTPv1 code path should fit in,
which is why the GTPv0 and the GTPv1 code pieces are mixed in my changes.
Should I explicitly claim that the flow-based change is for GTPv1 only?
+ //
flow-based GTP1U encap
+ info = skb_tunnel_info(skb);
+ if (gtp->collect_md && info && ntohs(info->key.tp_dst) ==
GTP1U_PORT) {
·
I think it's typically safe to assume that GTP is only operated on
standard ports, but it is something you chould/should think about, i.e.
whether you want that kind of restriction. In the existing use case, we
have the v0/v1 information stored in the per-pdp context structure.
·
The reason I’m checking GTP1U_PORT here is to filter GTP1U traffic.
It possible to pass a port number from ovs into the gtp module. I will
investigate how to support programmable port.
+ tun_id =
htonl(pctx->u.v1.o_tei);
·
here is where you're assuming GTPv1 in two ways from code that is called
from both v0 and v1.
* you're dereferencing a v1 specific element in the pctx union
* you're storing the result in a 32bit variable
·
Right, will fix this for GTPv0.
gtp =
netdev_priv(dev);
+ gtp->net = src_net;
·
Isn't this a generic change that's independent of your work on OVS GTP?
It is meant to be OVS independent. What makes it not? Should I leave
this field un-initialized?
Thanks
-Jiannan