Received-SPF: pass (google.com: domain of linux-kernel+bounces-186365-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20240520070348.26725-1-chengen.du@canonical.com>
 <664b97e8abe7a_12b4762946f@willemb.c.googlers.com.notmuch>
 <CAPza5qcGyfcUYOoznci4e=1eaScVTgkzAhXfKSG3bTzC=aOwew@mail.gmail.com>
 <eaf33ba66cbdc639b0209b232f892ec8a52a1f21.camel@redhat.com> <664ca1651b66_14f7a8294cb@willemb.c.googlers.com.notmuch>
In-Reply-To: <664ca1651b66_14f7a8294cb@willemb.c.googlers.com.notmuch>
From: Chengen Du <chengen.du@canonical.com>
Date: Wed, 22 May 2024 22:27:28 +0800
Message-ID: <CAPza5qfZ8JPkt4Ez1My=gfpT7VfHo75N01fLQdFaojBv2whi8w@mail.gmail.com>
Subject: Re: [PATCH] af_packet: Handle outgoing VLAN packets without hardware offloading
To: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Cc: Paolo Abeni <pabeni@redhat.com>, davem@davemloft.net, edumazet@google.com, 
	kuba@kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: multipart/mixed; boundary="000000000000a6bba006190bbcfa"

--000000000000a6bba006190bbcfa
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

Hi Paolo,

Thank you for your useful suggestions and information.

Hi Willem,

The issue initially stems from libpcap [1].
Upon their further investigation, another issue was discovered,
leading to a kernel request [2] that describes the problem in detail.

In essence, the kernel does not provide VLAN information if hardware
VLAN offloading is unavailable in cooked mode.
The TCI-TPID is missing because the prb_fill_vlan_info() function in
af_packet.c does not modify the tp_vlan_tci/tp_vlan_tpid values since
the information is in the payload and not in the sk_buff struct.
In cooked mode, the L2 header is stripped, preventing the receiver
from determining the correct TCI-TPID value.
Additionally, the protocol in SLL is incorrect, which means the
receiver cannot parse the L3 header correctly.

To reproduce the issue, please follow these steps:
1. ip link add link ens18 ens18.24 type vlan id 24
2. ifconfig ens18.24 1.0.24.1/24
3. ping -n 1.0.24.3 > /dev/null 2>&1 &
4. tcpdump -nn -i any -Q out not tcp and not udp

The attached experiment results show that the protocol is incorrectly
parsed as IPv4, which leads to inaccurate outcomes.

Thanks to Paolo's suggestion, I propose that we add a new bit in the
status to indicate the presence of VLAN information in the payload and
modify the header's entry (i.e., tp_vlan_tci/tp_vlan_tpid)
accordingly.
For the sll_protocol part, we can introduce a new member in the
sockaddr_ll struct to represent the VLAN-encapsulated protocol, if
applicable.

In my humble opinion, this approach will not affect current users who
rely on the status to handle VLAN parsing, and the sll_protocol will
remain unchanged.
Please kindly provide your feedback on this proposal, as there may be
important points I have overlooked.
If this approach seems feasible, I will submit a new version next week.
Your assistance and opinions on this issue are important to me, and I
truly appreciate them.

Best regards,
Chengen Du

[1] https://github.com/the-tcpdump-group/libpcap/issues/1105
[2] https://marc.info/?l=3Dlinux-netdev&m=3D165074467517201&w=3D4

On Tue, May 21, 2024 at 9:28=E2=80=AFPM Willem de Bruijn
<willemdebruijn.kernel@gmail.com> wrote:
>
> Paolo Abeni wrote:
> > On Tue, 2024-05-21 at 11:31 +0800, Chengen Du wrote:
> > > I would appreciate any suggestions you could offer, as I am not as
> > > familiar with this area as you are.
> > >
> > > I encountered an issue while capturing packets using tcpdump, which
> > > leverages the libpcap library for sniffing functionalities.
> > > Specifically, when I use "tcpdump -i any" to capture packets and
> > > hardware VLAN offloading is unavailable, some bogus packets appear.
>
> Bogus how exactly?
>
> > > In this scenario, Linux uses cooked-mode capture (SLL) for the "any"
> > > device, reading from a PF_PACKET/SOCK_DGRAM socket instead of the
> > > usual PF_PACKET/SOCK_RAW socket.
>
> Trying to extract L2 or VLAN information from the any device may be
> the real issue here.
>
> > >
> > > Using SOCK_DGRAM instead of SOCK_RAW means that the Linux socket code
> > > does not supply the packet's link-layer header.
> > > Based on the code in af_packet.c, SOCK_DGRAM strips L2 headers from
> > > the original packets and provides SLL for some L2 information.
> >
> > > From the receiver's perspective, the VLAN information can only be
> > > parsed from SLL, which causes issues if the kernel stores VLAN
> > > information in the payload.
>
> ETH_HLEN is pulled, but the VLAN tag is still present, right?
>
> > >
> > > As you mentioned, this modification affects existing PF_PACKET receiv=
ers.
> > > For example, libpcap needs to change how it parses VLAN packets with
> > > the PF_PACKET/SOCK_RAW socket.
> > > The lack of VLAN information in SLL may prevent the receiver from
> > > properly decoding the L3 frame in cooked mode.
> > >
> > > I am new to this area and would appreciate it if you could kindly
> > > correct any misunderstandings I might have about the mechanism.
> > > I would also be grateful for any insights you could share on this iss=
ue.
> > > Additionally, I am passionate about contributing to resolving this
> > > issue and am willing to work on patches based on your suggestions.
> >
> > One possible way to address the above in a less invasive manner, could
> > be allocating a new TP_STATUS_VLAN_HEADER_IS_PRESENT bit, set it for
> > SLL when the vlan is not stripped by H/W and patch tcpdump to interpret
> > such info.
>
> Any change must indeed not break existing users. It's not sufficient
> to change pcap/tcpdump. There are lots of other PF_PACKET users out
> there. Related, it is helpful to verify that tcpdump agrees to a patch
> before we change the ABI for it.

--000000000000a6bba006190bbcfa
Content-Type: application/vnd.tcpdump.pcap; name="any_sll.pcap"
Content-Disposition: attachment; filename="any_sll.pcap"
Content-Transfer-Encoding: base64
Content-ID: <f_lwhx0e5n0>
X-Attachment-Id: f_lwhx0e5n0

1MOyoQIABAAAAAAAAAAAAAAABABxAAAAGPhNZqvjCgAsAAAALAAAAAAEAAEABqoRvXX09QAACAYA
AQgABgQAAaoRvXX09QEAGAEAAAAAAAABABgDGPhNZq3jCgAsAAAALAAAAAAEAAEABqoRvXX09QAA
gQAAAQgABgQAAaoRvXX09QEAGAEAAAAAAAABABgDGfhNZsNBCwAsAAAALAAAAAAEAAEABqoRvXX0
9QAACAYAAQgABgQAAaoRvXX09QEAGAEAAAAAAAABABgDGfhNZsRBCwAsAAAALAAAAAAEAAEABqoR
vXX09QAAgQAAAQgABgQAAaoRvXX09QEAGAEAAAAAAAABABgDGvhNZv6eCwAsAAAALAAAAAAEAAEA
BqoRvXX09QAACAYAAQgABgQAAaoRvXX09QEAGAEAAAAAAAABABgDGvhNZgCfCwAsAAAALAAAAAAE
AAEABqoRvXX09QAAgQAAAQgABgQAAaoRvXX09QEAGAEAAAAAAAABABgDG/hNZs38CwAsAAAALAAA
AAAEAAEABqoRvXX09QAACAYAAQgABgQAAaoRvXX09QEAGAEAAAAAAAABABgDG/hNZtL8CwAsAAAA
LAAAAAAEAAEABqoRvXX09QAAgQAAAQgABgQAAaoRvXX09QEAGAEAAAAAAAABABgD
--000000000000a6bba006190bbcfa--