On Mon, Aug 21, 2017 at 04:01:15PM +0200, Amine Kherbouche wrote:

On 08/16/2017 07:01 PM, David Lamparter wrote:

quoted

This wires up the neccessary calls for VPLS into the MPLS forwarding
pieces.  Since CONFIG_MPLS_VPLS doesn't exist yet in Kconfig, it'll
never be enabled, so we're on the stubs for now.

[...]

quoted

+	if (rt->rt_payload_type == MPT_VPLS)
+		return vpls_rcv(skb, dev, pt, rt, hdr, orig_dev);

you should get the ret value of vpls_rcv() and increment stats if error 
occurs.

An error in vpls_rcv() is not a receive error on the outer device's MPLS
layer;  the packet was received correctly (and counted for that at the
beginning of mpls_forward()) and dispatched onto an appropriately
configured VPLS device.  vpls_rcv() counts its own stats on the inner
device.

[...]

quoted

+static inline int vpls_rcv(skb, in_dev, pt, rt, hdr, orig_dev)
+{
+	kfree_skb(skb);
+	return NET_RX_DROP;

just return NET_RX_DROP;

This is not correct;  the skb ownership is passed down to vpls_rcv(),
which itself will pass it on via netif_rx().  This is also why the call
in mpls_forward() does *not* "goto drop", instead directly returning.
vpls_rcv() consumes the skb in all cases.  Doing this the other way
around would incur extra overhead through a skb_clone() in vpls_rcv()
before giving it to netif_rx().

Note that the vpls_rcv() dummy function in this patch can't be called
because the kernel will refuse to install a VPLS route into the label
table (the is_vpls_device() check will be 0).  The dummy is just to keep
the code tidy.

[...]

quoted

+#endif

s/ #endif/#endif /* CONFIG_MPLS_VPLS *//

Fixed in next ver (along with the bugged prototype reported by the
build bot.)

Cheers,


-David