Re: [PATCH net-next] net: PIM register decapsulation and Forwarding.

[Ido Schimmel <idosch@nvidia.com>]

On Tue, May 17, 2022 at 05:10:26PM -0700, Jakub Kicinski wrote:
> On Mon, 16 May 2022 04:29:06 -0700 Saranya Panjarathina wrote:
> > PIM register packet is decapsulated but not forwarded in RP
> > 
> > __pim_rcv decapsulates the PIM register packet and reinjects for forwarding
> > after replacing the skb->dev to reg_dev (vif with VIFF_Register)
> > 
> > Ideally the incoming device should be same as skb->dev where the
> > original PIM register packet is received. mcache would not have
> > reg_vif as IIF. Decapsulated packet forwarding is failing
> > because of IIF mismatch. In RP for this S,G RPF interface would be
> > skb->dev vif only, so that would be IIF for the cache entry.
> > 
> > Signed-off-by: Saranya Panjarathina <plsaranya@gmail.com>
> 
> Not sure if this can cause any trouble. And why it had become 
> a problem now, seems like the code has been this way forever.
> David? Ido?

Trying to understand the problem:

1. The RP has an (*, G) towards the receiver(s) (receiver joins first)
2. The RP receives a PIM Register packet encapsulating the packet from
the source
3. The kernel decapsulates the packet and injects it into the Rx path as
if the packet was received by the pimreg netdev
4. The kernel forwards the packet according to the (*, G) route (no RPF
check)

At the same time, the PIM Register packet should be received by whatever
routing daemon is running in user space via a raw socket for the PIM
protocol. My understanding is that it should cause the RP to send a PIM
Join towards the FHR, causing the FHR to send two copies of each packet
from the source: encapsulated in the PIM Register packet and over the
(S, G) Tree.

If the RP already has an (S, G) route with IIF of skb->dev and the
decapsulated packet is injected into the Rx path via skb->dev, then what
prevents the RP from forwarding the same packet twice towards the
receiver(s)?

I'm not a PIM expert so the above might be nonsense. Anyway, I will
check with someone from the FRR teams who understands PIM better than
me.

> 
> > diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
> > index 13e6329784fb..7b9586335fb7 100644
> > --- a/net/ipv4/ipmr.c
> > +++ b/net/ipv4/ipmr.c
> > @@ -598,7 +598,7 @@ static int __pim_rcv(struct mr_table *mrt, struct sk_buff *skb,
> >  	skb->protocol = htons(ETH_P_IP);
> >  	skb->ip_summed = CHECKSUM_NONE;
> >  
> > -	skb_tunnel_rx(skb, reg_dev, dev_net(reg_dev));
> > +	skb_tunnel_rx(skb, skb->dev, dev_net(skb->dev));
> >  
> >  	netif_rx(skb);
> >  
> > diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
> > index 4e74bc61a3db..147e29a818ca 100644
> > --- a/net/ipv6/ip6mr.c
> > +++ b/net/ipv6/ip6mr.c
> > @@ -566,7 +566,7 @@ static int pim6_rcv(struct sk_buff *skb)
> >  	skb->protocol = htons(ETH_P_IPV6);
> >  	skb->ip_summed = CHECKSUM_NONE;
> >  
> > -	skb_tunnel_rx(skb, reg_dev, dev_net(reg_dev));
> > +	skb_tunnel_rx(skb, skb->dev, net);
> >  
> >  	netif_rx(skb);
> >  
>