Florian Westphal schreef op 19/05/2014 19:09:

David Newall [off-list ref] wrote:

[ remove lkml and cc nf-devel ]

quoted

I tried to persevere with the commit: I recalculated checksum, which
left routes and times improperly updated in options.  Then I tried
calling ip_forward_options, which looks like it would correctly
update RR and TS (not to mention checksum)m but that bombed because
skb_rtable returned NULL.

Yes.  bridge<->netfilter wiring is pure duct tape.
The glue code will set up a fake rtable for the skb after the
prerouting hook. [ see br_nf_pre_routing_finish() ].

quoted

I see three ways to progress:

1. Possibly call ip_forward_option, but that requires somebody who
understands this code to help;
2. Just recalculate the checksum, leaving crap in the options; or
3. Revert the commit.

I think none of these are an option.

I fail to understand why a bridge should honor/modifiy IP options.

For the 'local delivery' case the ip stack will take care of
option parsing, for forwarding it should be sufficient to do
sanity tests (for netfilters sake).

quoted

From a quick glance, it should be sufficient to edit

br_parse_ip_options() and remove everything after

memset(IPCB(skb), 0, sizeof(struct inet_skb_parm));

A 2nd step would be to move a copy of ip_options_compile()
into br_netfilter.c and trim it down to only validate the
ipv4 header without modifying it.

Perhaps it's possible to call ip_options_compile with a skb == NULL, 
like ip_options.c::ip_options_get_finish does. That way we don't need to 
duplicate code.
An alternative would be to make sure that the data pointed to by IPCB 
and BR_INPUT_SKB_CB don't overlap. If this were the case, we could 
indeed just revert the commit that was referred to.


cheers,
Bart