>> > > This clearly is a layering/abstraction violation and would have been
>> > > good to discuss upfront.
>> > >
>> > > Where do you make use of that information? What about other packet
>> > > injection
>> > > paths?
>> >
>> > The next commit uses it in if_arp to ensure that the DaD probe sending
>> > interface
>> > hardware address matches the sending hardware address in the ARP packet as
>> > specified in RFC 5227 section 1.1
>> >
>> > I couldn't think of a better way of achieving this.
>>
>> RFC 5227 says senders must follow the spec but doesn't say receivers'
>> check is must IIUC.
>>
>> I don't think it is a good idea to increase the mbuf size just for
>> broken clients.
>> I think it is better to make the strict check optional (by sysctl or
>> something) and use mtag,
>> so the change doesn't impact on most of us.
>
>Well... another possible option is to unionize l2_* with pattr_*.
>This is possible (IIUC)
>because l2_* are used only on receiving packets while pattr_* are used only on
>sending packets.
Since l2_sha continues to point outside of m_data manipulated by m_adj(),
it can be corrupted by subsequent m_pullup() or other mbuf m_*() operations.
I still believe that using MTAG is appropriate.
How about something like this? (not tested)
git diff -u -u .
diff --git a/sys/net/if_ethersubr.c b/sys/net/if_ethersubr.c
index 34be2d1cf91..18c8c1dcef9 100644
--- a/sys/net/if_ethersubr.c
+++ b/sys/net/if_ethersubr.c
@@ -886,9 +886,17 @@ ether_input(struct ifnet *ifp, struct mbuf *m)
#endif
}
- /* Store the senders hardware address */
- m->m_pkthdr.l2_sha = &eh->ether_shost;
- m->m_pkthdr.l2_shalen = ETHER_ADDR_LEN;
+ //if (arp_do_strict_check_sha) /* XXX: sysctl? */
+ if (etype == ETHERTYPE_ARP) {
+ /* Store the senders hardware address */
+ struct m_tag *mtag;
+ mtag = m_tag_get(PACKET_TAG_ETHERNET_SRC, ETHER_ADDR_LEN,
+ M_NOWAIT);
+ if (mtag != NULL) {
+ memcpy(mtag + 1, &eh->ether_shost, ETHER_ADDR_LEN);
+ m_tag_prepend(m, mtag);
+ }
+ }
/* Strip off the Ethernet header. */
m_adj(m, ehlen);
diff --git a/sys/netinet/if_arp.c b/sys/netinet/if_arp.c
index 90b0be9ff59..3334452c496 100644
--- a/sys/netinet/if_arp.c
+++ b/sys/netinet/if_arp.c
@@ -945,18 +945,23 @@ again:
(in_hosteq(isaddr, myaddr) ||
(in_nullhost(isaddr) && in_hosteq(itaddr, myaddr) &&
m->m_flags & M_BCAST &&
- ia->ia4_flags & (IN_IFF_TENTATIVE | IN_IFF_DUPLICATED))) &&
- m->m_pkthdr.l2_shalen == ah->ar_hln && (
- ah->ar_hln == 0 ||
- memcmp(m->m_pkthdr.l2_sha, ar_sha(ah), ah->ar_hln) == 0))
+ ia->ia4_flags & (IN_IFF_TENTATIVE | IN_IFF_DUPLICATED))))
{
struct sockaddr_dl sdl, *sdlp;
+ struct m_tag *mtag = NULL;
- sdlp = sockaddr_dl_init(&sdl, sizeof(sdl),
- ifp->if_index, ifp->if_type,
- NULL, 0, ar_sha(ah), ah->ar_hln);
- arp_dad_duplicated((struct ifaddr *)ia, sdlp);
- goto out;
+ //if (arp_do_strict_check_sha) /* XXX: sysctl? */
+ mtag = m_tag_find(m, PACKET_TAG_ETHERNET_SRC);
+
+ if (mtag == NULL || (ah->ar_hln == ETHER_ADDR_LEN &&
+ memcmp(mtag + 1, ar_sha(ah), ETHER_ADDR_LEN) == 0)) {
+
+ sdlp = sockaddr_dl_init(&sdl, sizeof(sdl),
+ ifp->if_index, ifp->if_type,
+ NULL, 0, ar_sha(ah), ah->ar_hln);
+ arp_dad_duplicated((struct ifaddr *)ia, sdlp);
+ goto out;
+ }
}
/*
diff --git a/sys/sys/mbuf.h b/sys/sys/mbuf.h
index 6b441b56bd8..bf69372f083 100644
--- a/sys/sys/mbuf.h
+++ b/sys/sys/mbuf.h
@@ -178,8 +178,8 @@ struct m_hdr {
* checksum) -- this is so we can accumulate the checksum for fragmented
* packets during reassembly.
*
- * Size ILP32: 48
- * LP64: 72
+ * Size ILP32: 40
+ * LP64: 56
*/
struct pkthdr {
union {
@@ -203,9 +203,6 @@ struct pkthdr {
int pattr_af; /* ALTQ: address family */
void *pattr_class; /* ALTQ: sched class set by classifier
*/
void *pattr_hdr; /* ALTQ: saved header position in mbuf
*/
-
- void *l2_sha; /* l2 sender host address */
- size_t l2_shalen; /* length of the sender address
*/
};
/* Checksumming flags (csum_flags). */
@@ -803,6 +800,7 @@ int m_tag_copy_chain(struct mbuf *, struct mbuf *);
*/
#define PACKET_TAG_MPLS 29 /* Indicate it's for MPLS */
#define PACKET_TAG_SRCROUTE 30 /* IPv4 source routing */
+#define PACKET_TAG_ETHERNET_SRC 31
/*
* Return the number of bytes in the mbuf chain, m.
Since l2_shalen is fixed to ETHER_ADDR_LEN for now, the tag name should be
PACKET_TAG_ETHERNET_SRC instead of PACKET_TAG_L2SHA for now.
If all L2 addresses are to be treated extensively, the structure of mtag
should include the size, but that will not be necessary just yet.
--
ryo shimizu
