From: Phil Sutter <phil@nwl.cc>
To: netdev@vger.kernel.org
Cc: Johann Baudy <johann.baudy@gnu-log.net>,
Eric Dumazet <eric.dumazet@gmail.com>
Subject: sending VLAN packets via packet_mmap
Date: Thu, 30 Sep 2010 21:24:14 +0200 [thread overview]
Message-ID: <20100930192414.GD26509@orbit.nwl.cc> (raw)
Hi,
support for VLAN tags in af_packet.c seems to be incomplete. While it's
possible to receive a full packet using SOCK_RAW, sending one will fail
due to size constraints. tpacket_snd() does not account for the
additional four bytes.
There are a few possible solutions to this problem. When searching for
the most appropriate one, I've been looking at tpacket_rcv() which
simply writes the whole frame out, setting tpacket2_hdr.tp_vlan_tci on
the go. So from a user's point of view, information is redundantly
available.
The actual problem in tpacket_snd() is this:
| reserve = dev->hard_header_len;
| [...]
| if (size_max > dev->mtu + reserve)
| size_max = dev->mtu + reserve;
I guess the check is there to avoid skb overflows on malicious data
input. Is this correct? Are there other reasons for it's existence?
As af_packet.c has no knowledge about VLANs (other than a call to
vlan_tx_tag_get()), I guess avoiding expensive parsing of the inserted
data for the VLAN tag should be appropriate. Nevertheless the check from
above needs to account for the additional VLAN_HLEN when the tag exists.
So a rather trivial solution would be to drop the check completely
(given no other constraints, of course), thereby giving the user a
little more ability to break things. Alternatively, one could require
that tpacket2_hdr.tp_vlan_tci be set (at least non-zero) to identify
packets containing a VLAN tag and allow the additional size (probably
mostly consistent to the logic inside tpacket_rcv()).
A third solution could be like the second one, but not accepting
prebuilt packets including VLAN header at all and using
tpacket2_hdr.tp_vlan_tci together with vlan_put_tag() to instead insert
it from inside the kernel.
Hopefully I didn't overlook something crucial. Feel free to flame me if
that's the case! :)
Greetings, Phil
next reply other threads:[~2010-09-30 19:30 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-09-30 19:24 Phil Sutter [this message]
2010-10-07 6:53 ` sending VLAN packets via packet_mmap David Miller
2010-10-11 13:15 ` Phil Sutter
2010-10-11 13:25 ` [PATCH] af_packet: account for VLAN when checking packet size Phil Sutter
2010-10-11 14:03 ` Eric Dumazet
2010-10-11 16:01 ` David Miller
2010-10-11 17:29 ` Phil Sutter
2010-10-12 17:19 ` Michael S. Tsirkin
2010-10-12 17:40 ` David Miller
2010-10-22 8:41 ` Simon Horman
2010-10-27 15:48 ` David Miller
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100930192414.GD26509@orbit.nwl.cc \
--to=phil@nwl.cc \
--cc=eric.dumazet@gmail.com \
--cc=johann.baudy@gnu-log.net \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).