[ovs-dev] [PATCH] datapath: reset skb->pkt_type when receiving pkt_type of PACKET_OTHERHOST

[Jesse Gross]

On Tue, Jul 9, 2013 at 10:49 PM, Isaku Yamahata <yamahata at valinux.co.jp> wrote:
> On Tue, Jul 09, 2013 at 10:23:42PM -0700, Jesse Gross wrote:
>> On Tue, Jul 9, 2013 at 7:48 PM, Isaku Yamahata <yamahata at valinux.co.jp> wrote:
>> > On Tue, Jul 09, 2013 at 02:08:03PM -0700, Jesse Gross wrote:
>> >> On Tue, Jul 9, 2013 at 4:07 AM, Isaku Yamahata <yamahata at valinux.co.jp> wrote:
>> >> > The upstream has the fix for dev_forward_skb() to reset pkt_type to
>> >> > PACKET_HOST. the change set of 06a23fe31ca3992863721f21bdb0307af93da807
>> >> > So the pkt_type of PACKET_OTHERHOST doesn't come in.
>> >> > This patch is a workaround for older kernel, reset skb->pkt_type when
>> >> > receiving pkt_type of PACKET_OTHERHOST.
>> >> >
>> >> > When two tunnel ports created on two OVS bridges on same host and
>> >> > two ports are the end point of the tunnel, packets are dropped as below.
>> >> >
>> >> > If a packet has pkt_type of PACKET_OTHERHOST when coming to ovs bridge,
>> >> > the packet is dropped by ip_rcv() if rules are installed and skb are
>> >> > forwarded by loopback device.
>> >> > Packet isn't dropped if flow rule isn't installed in kernel datapath,
>> >> > OVS_ACTION_ATTR_OUTPUT is used to send the packet.
>> >> >
>> >> > netns A |                      root netns                     | netns B
>> >> >    veth<->veth=ovs bridge=gre<-loop back->gre=ovs bridge=veth<->veth
>> >> >
>> >> > arp packet ->
>> >> > pkt_type
>> >> >          BROADCAST------------>ip_rcv()------------------------>
>> >> >
>> >> >                                                              <- arp reply
>> >> >                                                                 pkt_type
>> >> >
>> >> >                                                          rule exists
>> >> >                                ip_rcv()<-----------------OTHERHOST
>> >> >                                drop
>> >> >
>> >> >           <--------------------ip_rcv()<--- PACKET_HOST  rule doesn't exists
>> >> >                                pass         ^              |
>> >> >                                             |              |upcall
>> >> >                                             |              V
>> >> >                                             OVS_ACTION_ATTR_OUTPUT
>> >> >                                             ovs-switchd
>> >> >
>> >> > Cc: Murphy McCauley <murphy.mccauley at gmail.com>
>> >> > Cc: Jesse Gross <jesse at nicira.com>
>> >> > Signed-off-by: Isaku Yamahata <yamahata at valinux.co.jp>
>> >>
>> >> Doesn't the upstream change only work if you go through a veth first?
>> >
>> > Basically right. macvlan/macvtap and l2tp are also covered.
>> > This patch is consistent with iptunnel_pull_header() which sets pkt_type
>> > to PACKET_HOST when receiving skb.
>> >
>> >
>> >> What happens if the physical device is directly attached to OVS?
>> >
>> > If physical device is promiscuous mode off, skb with pkt_type=OTHERHOST
>> > doesn't come in usually. So the behavior is not changed.
>> > If promiscuous mode is on, packets of OTHERHOST is affected with this
>> > patch. And such packets will go through tunnel port and be handled
>> > by host ip layer. This is an expected behavior, I think.
>>
>> My impression is that this is intended to be a backport and not needed
>> on new kernels, right? However, in that case won't we have different
>> behavior in the case of promiscuous mode?
>
> Ah, that's right. The commit message/comments are not appropriate as you point
> out.
> Can you please give me suggestion to make progress?
>
> - change commit message/comment?
>   as this patch is also necessary for new kernels.
>
> - change patch itself?
>   What approach do you prefer?
>   I don't think I will be able to persuade netdev people to modify loop
>   back device.

I think we need to do something equivalent when we encapsulate a
packet, since at that point it changes from the original type to an
outgoing packet. So I think we want to set the type in the GRE data
plane.
X-CudaMail-Whitelist-To: dev at openvswitch.org