Hello,
On Mon, Mar 29, 2021 at 08:44:26AM +0000, Schreilechner, Dominik wrote:
> Hi,
>
> </snip>
>
> > yes, you are right. below is updated diff I would like to commit.
> > >
> > > Appart from that, adding a special task seems the way to go.
> >
> > I think so too. Alternative way would be to pass send flags via m_pkthdr
> > in mbuf, however there is no space. We would have to add a new member
> > to m_pkthdr. I see such change as too intrusive (given we address a true
> > corner case).
>
> I just wanted to reply, but you beat me to it :)
> I think the changes in ip_insertoptions() can be dropped completely,
> because the if-statement uses ip-ip_hl, which might not be initialized.
yes, you are right, I think we should rather go for this tweak:
--------8<---------------8<---------------8<------------------8<--------
diff --git a/sys/netinet/ip_output.c b/sys/netinet/ip_output.c
index e19b744fdf3..5c1222c0c86 100644
--- a/sys/netinet/ip_output.c
+++ b/sys/netinet/ip_output.c
@@ -767,7 +767,7 @@ ip_insertoptions(struct mbuf *m, struct mbuf *opt, int
*phlen)
return (m); /* XXX should fail */
/* check if options will fit to IP header */
- if ((optlen + (ip->ip_hl << 2)) > (0x0f << 2)) {
+ if ((optlen + (sizeof (struct ip))) > (0x0f << 2)) {
*phlen = sizeof (struct ip);
return (m);
}
--------8<---------------8<---------------8<------------------8<--------
>
> Also, in icmp_send(), ip_insertoptions() might return a different mbuf
> pointer. Thus, the struct ip pointer must be updated as well, right?
> block for refrence (Whole diff below):
yes, this is a good point. updated diff is below.
thanks and
regards
sashan
--------8<---------------8<---------------8<------------------8<--------
diff --git a/sys/netinet/ip_icmp.c b/sys/netinet/ip_icmp.c
index a007aa6c2b3..ee8888688bb 100644
--- a/sys/netinet/ip_icmp.c
+++ b/sys/netinet/ip_icmp.c
@@ -846,10 +846,21 @@ icmp_send(struct mbuf *m, struct mbuf *opts)
printf("icmp_send dst %s src %s\n", dst, src);
}
#endif
- if (opts != NULL)
+ /*
+ * ip_send() cannot handle IP options properly. So in case we have
+ * options fill out the IP header here and use ip_send_raw() instead.
+ */
+ if (opts != NULL) {
m = ip_insertoptions(m, opts, &hlen);
-
- ip_send(m);
+ ip = mtod(m, struct ip *);
+ ip->ip_hl = (hlen >> 2);
+ ip->ip_v = IPVERSION;
+ ip->ip_off &= htons(IP_DF);
+ ip->ip_id = htons(ip_randomid());
+ ipstat_inc(ips_localout);
+ ip_send_raw(m);
+ } else
+ ip_send(m);
}
u_int32_t
diff --git a/sys/netinet/ip_input.c b/sys/netinet/ip_input.c
index 0ec3f723be4..6c935cc88c2 100644
--- a/sys/netinet/ip_input.c
+++ b/sys/netinet/ip_input.c
@@ -139,6 +139,7 @@ struct cpumem *ipcounters;
int ip_sysctl_ipstat(void *, size_t *, void *);
static struct mbuf_queue ipsend_mq;
+static struct mbuf_queue ipsendraw_mq;
extern struct niqueue arpinq;
@@ -147,7 +148,11 @@ int ip_dooptions(struct mbuf *, struct ifnet *);
int in_ouraddr(struct mbuf *, struct ifnet *, struct rtentry **);
static void ip_send_dispatch(void *);
+static void ip_sendraw_dispatch(void *);
static struct task ipsend_task = TASK_INITIALIZER(ip_send_dispatch,
&ipsend_mq);
+static struct task ipsendraw_task =
+ TASK_INITIALIZER(ip_sendraw_dispatch, &ipsendraw_mq);
+
/*
* Used to save the IP options in case a protocol wants to respond
* to an incoming packet over the same route if the packet got here
@@ -217,6 +222,7 @@ ip_init(void)
DP_SET(rootonlyports.udp, defrootonlyports_udp[i]);
mq_init(&ipsend_mq, 64, IPL_SOFTNET);
+ mq_init(&ipsendraw_mq, 64, IPL_SOFTNET);
#ifdef IPSEC
ipsec_init();
@@ -1777,7 +1783,7 @@ ip_savecontrol(struct inpcb *inp, struct mbuf **mp,
struct ip *ip,
}
void
-ip_send_dispatch(void *xmq)
+ip_send_do_dispatch(void *xmq, int flags)
{
struct mbuf_queue *mq = xmq;
struct mbuf *m;
@@ -1789,14 +1795,33 @@ ip_send_dispatch(void *xmq)
NET_LOCK();
while ((m = ml_dequeue(&ml)) != NULL) {
- ip_output(m, NULL, NULL, 0, NULL, NULL, 0);
+ ip_output(m, NULL, NULL, flags, NULL, NULL, 0);
}
NET_UNLOCK();
}
+void
+ip_sendraw_dispatch(void *xmq)
+{
+ ip_send_do_dispatch(xmq, IP_RAWOUTPUT);
+}
+
+void
+ip_send_dispatch(void *xmq)
+{
+ ip_send_do_dispatch(xmq, 0);
+}
+
void
ip_send(struct mbuf *m)
{
mq_enqueue(&ipsend_mq, m);
task_add(net_tq(0), &ipsend_task);
}
+
+void
+ip_send_raw(struct mbuf *m)
+{
+ mq_enqueue(&ipsendraw_mq, m);
+ task_add(net_tq(0), &ipsendraw_task);
+}
diff --git a/sys/netinet/ip_output.c b/sys/netinet/ip_output.c
index c01a3e7803c..5c1222c0c86 100644
--- a/sys/netinet/ip_output.c
+++ b/sys/netinet/ip_output.c
@@ -765,6 +765,13 @@ ip_insertoptions(struct mbuf *m, struct mbuf *opt, int
*phlen)
optlen = opt->m_len - sizeof(p->ipopt_dst);
if (optlen + ntohs(ip->ip_len) > IP_MAXPACKET)
return (m); /* XXX should fail */
+
+ /* check if options will fit to IP header */
+ if ((optlen + sizeof (struct ip)) > (0x0f << 2)) {
+ *phlen = sizeof (struct ip);
+ return (m);
+ }
+
if (p->ipopt_dst.s_addr)
ip->ip_dst = p->ipopt_dst;
if (m->m_flags & M_EXT || m->m_data - optlen < m->m_pktdat) {
diff --git a/sys/netinet/ip_var.h b/sys/netinet/ip_var.h
index 7ede24ce922..1a43675a7ac 100644
--- a/sys/netinet/ip_var.h
+++ b/sys/netinet/ip_var.h
@@ -240,6 +240,7 @@ struct mbuf *
u_int16_t
ip_randomid(void);
void ip_send(struct mbuf *);
+void ip_send_raw(struct mbuf *);
void ip_slowtimo(void);
struct mbuf *
ip_srcroute(struct mbuf *);
