Date: Sat, 28 Dec 2019 14:59:08 +0100 From: Kristof Provost <kristof@sigsegv.be> To: Andreas Longwitz <longwitz@incore.de> Cc: freebsd-pf@freebsd.org Subject: Re: Flow of broadcast/multicast packets in pf when a bridge is present Message-ID: <F95EB0E0-8CAF-4F76-858F-C7479967BCB4@sigsegv.be> In-Reply-To: <5E074209.2070801@incore.de> References: <5E074209.2070801@incore.de>
next in thread | previous in thread | raw e-mail | index | archive | help
> On 28 Dec 2019, at 12:52, Andreas Longwitz <longwitz@incore.de> wrote:
>=20
> =EF=BB=BFIn the meantime I have understand I was wrong about the code snip=
pet
>=20
>> mc2 =3D m_dup(m, M_NOWAIT);
>> if (mc2 !=3D NULL) {
>> /* Keep the layer3 header aligned */
>> int i =3D min(mc2->m_pkthdr.len, max_protohdr);
>> mc2 =3D m_copyup(mc2, i, ETHER_ALIGN);
>> }
>> if (mc2 !=3D NULL) {
>> mc2->m_pkthdr.rcvif =3D bifp;
>> (*bifp->if_input)(bifp, mc2);
>> }
>=20
> My mistake concerned the function call m_copyup(): The mbuf chain is
> copied correct and not shortened, I was confused because of the field
> m_len in mc2. So reinjecting the packet in the bridge is ok.
>=20
> Another aspect is what is done next with the broadcast/multicast packet
> handled by this code:
>=20
>> /* Return the original packet for local processing. */
>> return (m);
>=20
> Therefore local processing on the member interface is done for
> broadcast/multicast packets without checking the pfil_local_phys
> variable. That was confusing me because these packets are counting twice
> in the pf rules. I think this is needless and pfil_local_phys should
> respect all packets not only unicast.
>=20
> After introducing the patch
>=20
> --- if_bridge.c.iorig 2019-05-14 09:43:33.000000000 +0200
> +++ if_bridge.c 2019-12-28 11:54:52.000000000 +0100
> @@ -2386,6 +2386,10 @@
> (*bifp->if_input)(bifp, mc2);
> }
>=20
> + if (!pfil_local_phys ) {
> + m_freem(m);
> + return (NULL);
> + }
> /* Return the original packet for local processing. */
> return (m);
> }
>=20
> everything works fine and all the counters in pf have values as expected
>=20
Can you put that in Phabricator and cc me and kevans@? (I seem to remember h=
im touching related code a few months ago).
Thanks,
Kristof=
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?F95EB0E0-8CAF-4F76-858F-C7479967BCB4>