* [fragmentation ICMP 0/2] fragmentation ICMP @ 2015-05-06 1:59 Andy Zhou 2015-05-06 1:59 ` [net-next fragmenation icmp 1/2] ipv4_fragment: Add a bit in IPCB to control ICMP generation Andy Zhou 2015-05-06 1:59 ` [net-next fragmenation icmp 2/2] bridge: do not send icmp on fragmentation or defragmentation error Andy Zhou 0 siblings, 2 replies; 5+ messages in thread From: Andy Zhou @ 2015-05-06 1:59 UTC (permalink / raw) To: davem; +Cc: netdev, Andy Zhou Currently, we send ICMP packets when errors occur during fragmentation or de-fragmentation. However, it is a bug when sending those ICMP packets in the context of using netfilter for bridging. Those ICMP packets are only expected in the context of routing, not in bridging mode. The Local stack are not involved in bridging forward decisions, thus should be not used for deciding the reverse path for those ICMP messages. This bug only affects IPV4, not in IPv6. This patch series adds a bit in IPCB to control whether ICMP packet should be generated on fragmentation or de-fragmentation error. Currently the only user is br_netfilter. Andy Zhou (2): ipv4_fragment: Add a bit in IPCB to control ICMP generation bridge: do not send icmp on fragmentation or defragmentation error include/net/inet_frag.h | 2 ++ include/net/ip.h | 1 + net/bridge/br_netfilter.c | 7 +++++++ net/ipv4/ip_fragment.c | 11 ++++++++++- net/ipv4/ip_output.c | 5 +++-- 5 files changed, 23 insertions(+), 3 deletions(-) -- 1.9.1 *** BLURB HERE *** Andy Zhou (2): ipv4_fragment: Add a bit in IPCB to control ICMP generation bridge: do not send icmp on fragmentation or defragmentation error include/net/inet_frag.h | 2 ++ include/net/ip.h | 1 + net/bridge/br_netfilter.c | 7 +++++++ net/ipv4/ip_fragment.c | 11 ++++++++++- net/ipv4/ip_output.c | 5 +++-- 5 files changed, 23 insertions(+), 3 deletions(-) -- 1.9.1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [net-next fragmenation icmp 1/2] ipv4_fragment: Add a bit in IPCB to control ICMP generation 2015-05-06 1:59 [fragmentation ICMP 0/2] fragmentation ICMP Andy Zhou @ 2015-05-06 1:59 ` Andy Zhou 2015-05-06 12:23 ` Eric Dumazet 2015-05-06 1:59 ` [net-next fragmenation icmp 2/2] bridge: do not send icmp on fragmentation or defragmentation error Andy Zhou 1 sibling, 1 reply; 5+ messages in thread From: Andy Zhou @ 2015-05-06 1:59 UTC (permalink / raw) To: davem; +Cc: netdev, Andy Zhou Currently, on fragmentation or defragmentation error, ICMP error message can be generated. This is fine when they are used in a routing context, but does not make sense when they are used in a bridging context, i.e. netfilter for bridging. This patch adds a bit in IPCB to control whether ICMP error message should be generated. IPV6 fragmentation and defragmentation functions are not reused by bridge netfilter. Thus, this change is only required for IPV4. Signed-off-by: Andy Zhou <azhou@nicira.com> --- include/net/inet_frag.h | 2 ++ include/net/ip.h | 1 + net/ipv4/ip_fragment.c | 11 ++++++++++- net/ipv4/ip_output.c | 5 +++-- 4 files changed, 16 insertions(+), 3 deletions(-) diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h index 8d17655..1d57045 100644 --- a/include/net/inet_frag.h +++ b/include/net/inet_frag.h @@ -44,6 +44,7 @@ enum { * @meat: length of received fragments so far * @flags: fragment queue flags * @max_size: (ipv4 only) maximum received fragment size with IP_DF set + * @no_icmp: Do not generate any fragmentation related icmp packet * @net: namespace that this frag belongs to */ struct inet_frag_queue { @@ -58,6 +59,7 @@ struct inet_frag_queue { int meat; __u8 flags; u16 max_size; + bool no_icmp; struct netns_frags *net; }; diff --git a/include/net/ip.h b/include/net/ip.h index d14af7e..8d81c865 100644 --- a/include/net/ip.h +++ b/include/net/ip.h @@ -45,6 +45,7 @@ struct inet_skb_parm { #define IPSKB_FRAG_COMPLETE BIT(3) #define IPSKB_REROUTED BIT(4) #define IPSKB_DOREDIRECT BIT(5) +#define IPSKB_NO_FRAG_ICMP BIT(6) u16 frag_max_size; }; diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c index cc1da6d..2f38d30 100644 --- a/net/ipv4/ip_fragment.c +++ b/net/ipv4/ip_fragment.c @@ -217,7 +217,8 @@ static void ip_expire(unsigned long arg) /* Only an end host needs to send an ICMP * "Fragment Reassembly Timeout" message, per RFC792. */ - if (qp->user == IP_DEFRAG_AF_PACKET || + if (qp->q.no_icmp || + qp->user == IP_DEFRAG_AF_PACKET || ((qp->user >= IP_DEFRAG_CONNTRACK_IN) && (qp->user <= __IP_DEFRAG_CONNTRACK_IN_END) && (skb_rtable(head)->rt_type != RTN_LOCAL))) @@ -323,6 +324,7 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb) int ihl, end; int err = -ENOENT; u8 ecn; + bool no_icmp; if (qp->q.flags & INET_FRAG_COMPLETE) goto err; @@ -340,6 +342,7 @@ static int ip_frag_queue(struct ipq *qp, struct sk_buff *skb) offset &= IP_OFFSET; offset <<= 3; /* offset is in 8-byte chunks */ ihl = ip_hdrlen(skb); + no_icmp = IPCB(skb)->flags & IPSKB_NO_FRAG_ICMP; /* Determine the position of this fragment. */ end = offset + skb->len - ihl; @@ -478,6 +481,8 @@ found: skb->len + ihl > qp->q.max_size) qp->q.max_size = skb->len + ihl; + qp->q.no_icmp = qp->q.no_icmp || no_icmp; + if (qp->q.flags == (INET_FRAG_FIRST_IN | INET_FRAG_LAST_IN) && qp->q.meat == qp->q.len) { unsigned long orefdst = skb->_skb_refdst; @@ -608,6 +613,10 @@ static int ip_frag_reasm(struct ipq *qp, struct sk_buff *prev, head->tstamp = qp->q.stamp; IPCB(head)->frag_max_size = qp->q.max_size; + IPCB(head)->flags &= ~IPSKB_NO_FRAG_ICMP; + if (qp->q.no_icmp) + IPCB(head)->flags |= IPSKB_NO_FRAG_ICMP; + iph = ip_hdr(head); /* max_size != 0 implies at least one fragment had IP_DF set */ iph->frag_off = qp->q.max_size ? htons(IP_DF) : 0; diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c index c65b93a..dfa425c 100644 --- a/net/ipv4/ip_output.c +++ b/net/ipv4/ip_output.c @@ -511,8 +511,9 @@ int ip_fragment(struct sock *sk, struct sk_buff *skb, (IPCB(skb)->frag_max_size && IPCB(skb)->frag_max_size > mtu))) { IP_INC_STATS(dev_net(dev), IPSTATS_MIB_FRAGFAILS); - icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED, - htonl(mtu)); + if (!(IPCB(skb)->flags & IPSKB_NO_FRAG_ICMP)) + icmp_send(skb, ICMP_DEST_UNREACH, ICMP_FRAG_NEEDED, + htonl(mtu)); kfree_skb(skb); return -EMSGSIZE; } -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [net-next fragmenation icmp 1/2] ipv4_fragment: Add a bit in IPCB to control ICMP generation 2015-05-06 1:59 ` [net-next fragmenation icmp 1/2] ipv4_fragment: Add a bit in IPCB to control ICMP generation Andy Zhou @ 2015-05-06 12:23 ` Eric Dumazet 2015-05-06 21:12 ` Andy Zhou 0 siblings, 1 reply; 5+ messages in thread From: Eric Dumazet @ 2015-05-06 12:23 UTC (permalink / raw) To: Andy Zhou; +Cc: davem, netdev On Tue, 2015-05-05 at 18:59 -0700, Andy Zhou wrote: > Currently, on fragmentation or defragmentation error, ICMP error message > can be generated. This is fine when they are used in a routing context, > but does not make sense when they are used in a bridging context, i.e. > netfilter for bridging. > > This patch adds a bit in IPCB to control whether ICMP error message > should be generated. > > IPV6 fragmentation and defragmentation functions are not reused by > bridge netfilter. Thus, this change is only required for IPV4. > > Signed-off-by: Andy Zhou <azhou@nicira.com> > --- > include/net/inet_frag.h | 2 ++ > include/net/ip.h | 1 + > net/ipv4/ip_fragment.c | 11 ++++++++++- > net/ipv4/ip_output.c | 5 +++-- > 4 files changed, 16 insertions(+), 3 deletions(-) > > diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h > index 8d17655..1d57045 100644 > --- a/include/net/inet_frag.h > +++ b/include/net/inet_frag.h > @@ -44,6 +44,7 @@ enum { > * @meat: length of received fragments so far > * @flags: fragment queue flags > * @max_size: (ipv4 only) maximum received fragment size with IP_DF set > + * @no_icmp: Do not generate any fragmentation related icmp packet > * @net: namespace that this frag belongs to > */ > struct inet_frag_queue { > @@ -58,6 +59,7 @@ struct inet_frag_queue { > int meat; > __u8 flags; > u16 max_size; > + bool no_icmp; > struct netns_frags *net; > }; > What about filling the 8bit hole ? int meat; __u8 flags; + bool no_icmp; u16 max_size; struct netns_frags *net; ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [net-next fragmenation icmp 1/2] ipv4_fragment: Add a bit in IPCB to control ICMP generation 2015-05-06 12:23 ` Eric Dumazet @ 2015-05-06 21:12 ` Andy Zhou 0 siblings, 0 replies; 5+ messages in thread From: Andy Zhou @ 2015-05-06 21:12 UTC (permalink / raw) To: Eric Dumazet; +Cc: David Miller, netdev@vger.kernel.org Since I only need 1 bit for this, I can probably define and use one more bit in the 'flags' field above. Thanks for the review. I will send out a 'v2'. On Wed, May 6, 2015 at 5:23 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote: > On Tue, 2015-05-05 at 18:59 -0700, Andy Zhou wrote: >> Currently, on fragmentation or defragmentation error, ICMP error message >> can be generated. This is fine when they are used in a routing context, >> but does not make sense when they are used in a bridging context, i.e. >> netfilter for bridging. >> >> This patch adds a bit in IPCB to control whether ICMP error message >> should be generated. >> >> IPV6 fragmentation and defragmentation functions are not reused by >> bridge netfilter. Thus, this change is only required for IPV4. >> >> Signed-off-by: Andy Zhou <azhou@nicira.com> >> --- >> include/net/inet_frag.h | 2 ++ >> include/net/ip.h | 1 + >> net/ipv4/ip_fragment.c | 11 ++++++++++- >> net/ipv4/ip_output.c | 5 +++-- >> 4 files changed, 16 insertions(+), 3 deletions(-) >> >> diff --git a/include/net/inet_frag.h b/include/net/inet_frag.h >> index 8d17655..1d57045 100644 >> --- a/include/net/inet_frag.h >> +++ b/include/net/inet_frag.h >> @@ -44,6 +44,7 @@ enum { >> * @meat: length of received fragments so far >> * @flags: fragment queue flags >> * @max_size: (ipv4 only) maximum received fragment size with IP_DF set >> + * @no_icmp: Do not generate any fragmentation related icmp packet >> * @net: namespace that this frag belongs to >> */ >> struct inet_frag_queue { >> @@ -58,6 +59,7 @@ struct inet_frag_queue { >> int meat; >> __u8 flags; >> u16 max_size; >> + bool no_icmp; >> struct netns_frags *net; >> }; >> > > > What about filling the 8bit hole ? > > int meat; > __u8 flags; > + bool no_icmp; > u16 max_size; > struct netns_frags *net; > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* [net-next fragmenation icmp 2/2] bridge: do not send icmp on fragmentation or defragmentation error 2015-05-06 1:59 [fragmentation ICMP 0/2] fragmentation ICMP Andy Zhou 2015-05-06 1:59 ` [net-next fragmenation icmp 1/2] ipv4_fragment: Add a bit in IPCB to control ICMP generation Andy Zhou @ 2015-05-06 1:59 ` Andy Zhou 1 sibling, 0 replies; 5+ messages in thread From: Andy Zhou @ 2015-05-06 1:59 UTC (permalink / raw) To: davem; +Cc: netdev, Andy Zhou Set IPSKB_NO_FRAG_ICMP bit when doing IP fragmentation and defragmentation in bridging mode. Signed-off-by: Andy Zhou <azhou@nicira.com> --- net/bridge/br_netfilter.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/net/bridge/br_netfilter.c b/net/bridge/br_netfilter.c index ab55e24..3f3366a 100644 --- a/net/bridge/br_netfilter.c +++ b/net/bridge/br_netfilter.c @@ -663,6 +663,10 @@ static unsigned int br_nf_pre_routing(const struct nf_hook_ops *ops, if (br_parse_ip_options(skb)) return NF_DROP; + /* In case this is a fragmented packet, do not send icmp packet on + * defragmentation error */ + IPCB(skb)->flags |= IPSKB_NO_FRAG_ICMP; + nf_bridge_put(skb->nf_bridge); if (!nf_bridge_alloc(skb)) return NF_DROP; @@ -875,6 +879,9 @@ static int br_nf_dev_queue_xmit(struct sock *sk, struct sk_buff *skb) skb_copy_from_linear_data_offset(skb, -data->size, data->mac, data->size); + /* Do not send ICMP packet on fragmentation error */ + IPCB(skb)->flags |= IPSKB_NO_FRAG_ICMP; + ret = ip_fragment(sk, skb, br_nf_push_frag_xmit); } else { ret = br_dev_queue_push_xmit(sk, skb); -- 1.9.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-05-06 21:12 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-05-06 1:59 [fragmentation ICMP 0/2] fragmentation ICMP Andy Zhou 2015-05-06 1:59 ` [net-next fragmenation icmp 1/2] ipv4_fragment: Add a bit in IPCB to control ICMP generation Andy Zhou 2015-05-06 12:23 ` Eric Dumazet 2015-05-06 21:12 ` Andy Zhou 2015-05-06 1:59 ` [net-next fragmenation icmp 2/2] bridge: do not send icmp on fragmentation or defragmentation error Andy Zhou
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).