12 Apr, 2014

1 commit

  • br_allowed_ingress() has two problems.

    1. If br_allowed_ingress() is called by br_handle_frame_finish() and
    vlan_untag() in br_allowed_ingress() fails, skb will be freed by both
    vlan_untag() and br_handle_frame_finish().

    2. If br_allowed_ingress() is called by br_dev_xmit() and
    br_allowed_ingress() fails, the skb will not be freed.

    Fix these two problems by freeing the skb in br_allowed_ingress()
    if it fails.

    Signed-off-by: Toshiaki Makita
    Signed-off-by: David S. Miller

    Toshiaki Makita
     

05 Apr, 2014

1 commit

  • All xtables variants suffer from the defect that the copy_to_user()
    to copy the counters to user memory may fail after the table has
    already been exchanged and thus exposed. Return an error at this
    point will result in freeing the already exposed table. Any
    subsequent packet processing will result in a kernel panic.

    We can't copy the counters before exposing the new tables as we
    want provide the counter state after the old table has been
    unhooked. Therefore convert this into a silent error.

    Cc: Florian Westphal
    Signed-off-by: Thomas Graf
    Signed-off-by: Pablo Neira Ayuso

    Thomas Graf
     

01 Apr, 2014

1 commit


30 Mar, 2014

2 commits

  • Conflicts:
    drivers/net/ethernet/marvell/mvneta.c

    The mvneta.c conflict is a case of overlapping changes,
    a conversion to devm_ioremap_resource() vs. a conversion
    to netdev_alloc_pcpu_stats.

    Signed-off-by: David S. Miller

    David S. Miller
     
  • The gfp parameter was added in:
    commit 47be03a28cc6c80e3aa2b3e8ed6d960ff0c5c0af
    Author: Amerigo Wang
    Date: Fri Aug 10 01:24:37 2012 +0000

    netpoll: use GFP_ATOMIC in slave_enable_netpoll() and __netpoll_setup()

    slave_enable_netpoll() and __netpoll_setup() may be called
    with read_lock() held, so should use GFP_ATOMIC to allocate
    memory. Eric suggested to pass gfp flags to __netpoll_setup().

    Cc: Eric Dumazet
    Cc: "David S. Miller"
    Reported-by: Dan Carpenter
    Signed-off-by: Eric Dumazet
    Signed-off-by: Cong Wang
    Signed-off-by: David S. Miller

    The reason for the gfp parameter was removed in:
    commit c4cdef9b7183159c23c7302aaf270d64c549f557
    Author: dingtianhong
    Date: Tue Jul 23 15:25:27 2013 +0800

    bonding: don't call slave_xxx_netpoll under spinlocks

    The slave_xxx_netpoll will call synchronize_rcu_bh(),
    so the function may schedule and sleep, it should't be
    called under spinlocks.

    bond_netpoll_setup() and bond_netpoll_cleanup() are always
    protected by rtnl lock, it is no need to take the read lock,
    as the slave list couldn't be changed outside rtnl lock.

    Signed-off-by: Ding Tianhong
    Cc: Jay Vosburgh
    Cc: Andy Gospodarek
    Signed-off-by: David S. Miller

    Nothing else that calls __netpoll_setup or ndo_netpoll_setup
    requires a gfp paramter, so remove the gfp parameter from both
    of these functions making the code clearer.

    Signed-off-by: "Eric W. Biederman"
    Signed-off-by: David S. Miller

    Eric W. Biederman
     

29 Mar, 2014

3 commits

  • When the vlan filtering is enabled on the bridge, but
    the filter is not configured on the bridge device itself,
    running tcpdump on the bridge device will result in a
    an Oops with NULL pointer dereference. The reason
    is that br_pass_frame_up() will bypass the vlan
    check because promisc flag is set. It will then try
    to get the table pointer and process the packet based
    on the table. Since the table pointer is NULL, we oops.
    Catch this special condition in br_handle_vlan().

    Reported-by: Toshiaki Makita
    CC: Toshiaki Makita
    Signed-off-by: Vlad Yasevich
    Acked-by: Toshiaki Makita
    Signed-off-by: David S. Miller

    Vlad Yasevich
     
  • If a bridge with vlan_filtering enabled receives frames with stacked
    vlan tags, i.e., they have two vlan tags, br_vlan_untag() strips not
    only the outer tag but also the inner tag.

    br_vlan_untag() is called only from br_handle_vlan(), and in this case,
    it is enough to set skb->vlan_tci to 0 here, because vlan_tci has already
    been set before calling br_handle_vlan().

    Signed-off-by: Toshiaki Makita
    Acked-by: Vlad Yasevich
    Signed-off-by: David S. Miller

    Toshiaki Makita
     
  • Bridge vlan code (br_vlan_get_tag()) assumes that all frames have vlan_tci
    if they are tagged, but if vlan tx offload is manually disabled on bridge
    device and frames are sent from vlan device on the bridge device, the tags
    are embedded in skb->data and they break this assumption.
    Extract embedded vlan tags and move them to vlan_tci at ingress.

    Signed-off-by: Toshiaki Makita
    Acked-by: Vlad Yasevich
    Signed-off-by: David S. Miller

    Toshiaki Makita
     

27 Mar, 2014

1 commit

  • This patch replaces rcu_assign_pointer(x, NULL) with RCU_INIT_POINTER(x, NULL)

    The rcu_assign_pointer() ensures that the initialization of a structure
    is carried out before storing a pointer to that structure.
    And in the case of the NULL pointer, there is no structure to initialize.
    So, rcu_assign_pointer(p, NULL) can be safely converted to RCU_INIT_POINTER(p, NULL)

    Signed-off-by: Monam Agarwal
    Signed-off-by: David S. Miller

    Monam Agarwal
     

15 Mar, 2014

2 commits


12 Mar, 2014

2 commits

  • Without this check someone could easily create a denial of service
    by injecting multicast-specific queries to enable the bridge
    snooping part if no real querier issuing periodic general queries
    is present on the link which would result in the bridge wrongly
    shutting down ports for multicast traffic as the bridge did not learn
    about these listeners.

    With this patch the snooping code is enabled upon receiving valid,
    general queries only.

    Signed-off-by: Linus Lüssing
    Signed-off-by: David S. Miller

    Linus Lüssing
     
  • General IGMP and MLD queries are supposed to have the multicast
    link-local all-nodes address as their destination according to RFC2236
    section 9, RFC3376 section 4.1.12/9.1, RFC2710 section 8 and RFC3810
    section 5.1.15.

    Without this check, such malformed IGMP/MLD queries can result in a
    denial of service: The queries are ignored by most IGMP/MLD listeners
    therefore they will not respond with an IGMP/MLD report. However,
    without this patch these malformed MLD queries would enable the
    snooping part in the bridge code, potentially shutting down the
    according ports towards these hosts for multicast traffic as the
    bridge did not learn about these listeners.

    Reported-by: Jan Stancek
    Signed-off-by: Linus Lüssing
    Signed-off-by: David S. Miller

    Linus Lüssing
     

07 Mar, 2014

1 commit

  • Commit e688a604807647 ("net: introduce DST_NOPEER dst flag") introduced
    DST_NOPEER because because of crashes in ipv6_select_ident called from
    udp6_ufo_fragment.

    Since commit 916e4cf46d0204 ("ipv6: reuse ip6_frag_id from
    ip6_ufo_append_data") we don't call ipv6_select_ident any more from
    ip6_ufo_append_data, thus this flag lost its purpose and can be removed.

    Cc: Eric Dumazet
    Signed-off-by: Hannes Frederic Sowa
    Acked-by: Eric Dumazet
    Signed-off-by: David S. Miller

    Hannes Frederic Sowa
     

06 Mar, 2014

1 commit

  • MLD queries are supposed to have an IPv6 link-local source address
    according to RFC2710, section 4 and RFC3810, section 5.1.14. This patch
    adds a sanity check to ignore such broken MLD queries.

    Without this check, such malformed MLD queries can result in a
    denial of service: The queries are ignored by any MLD listener
    therefore they will not respond with an MLD report. However,
    without this patch these malformed MLD queries would enable the
    snooping part in the bridge code, potentially shutting down the
    according ports towards these hosts for multicast traffic as the
    bridge did not learn about these listeners.

    Reported-by: Jan Stancek
    Signed-off-by: Linus Lüssing
    Reviewed-by: Hannes Frederic Sowa
    Signed-off-by: David S. Miller

    Linus Lüssing
     

25 Feb, 2014

2 commits


15 Feb, 2014

1 commit


11 Feb, 2014

9 commits

  • br_fdb_change_mac_address() calls fdb_insert()/fdb_delete() without
    br->hash_lock.

    These hash list updates are racy with br_fdb_update()/br_fdb_cleanup().

    Signed-off-by: Toshiaki Makita
    Acked-by: Vlad Yasevich
    Signed-off-by: David S. Miller

    Toshiaki Makita
     
  • Vlan codes unconditionally delete local fdb entries.
    We should consider the possibility that other ports have the same
    address and vlan.

    Example of problematic case:
    ip link set eth0 address 12:34:56:78:90:ab
    ip link set eth1 address aa:bb:cc:dd:ee:ff
    brctl addif br0 eth0
    brctl addif br0 eth1 # br0 will have mac address 12:34:56:78:90:ab
    bridge vlan add dev eth0 vid 10
    bridge vlan add dev eth1 vid 10
    bridge vlan add dev br0 vid 10 self
    We will have fdb entry such that f->dst == eth0, f->vlan_id == 10 and
    f->addr == 12:34:56:78:90:ab at this time.
    Next, delete eth0 vlan 10.
    bridge vlan del dev eth0 vid 10
    In this case, we still need the entry for br0, but it will be deleted.

    Note that br0 needs the entry even though its mac address is not set
    manually. To delete the entry with proper condition checking,
    fdb_delete_local() is suitable to use.

    Signed-off-by: Toshiaki Makita
    Acked-by: Vlad Yasevich
    Signed-off-by: David S. Miller

    Toshiaki Makita
     
  • br_fdb_delete_by_port() doesn't care about vlan and mac address of the
    bridge device.

    As the check is almost the same as mac address changing, slightly modify
    fdb_delete_local() and use it.

    Note that we can always set added_by_user to 0 in fdb_delete_local() because
    - br_fdb_delete_by_port() calls fdb_delete_local() for local entries
    regardless of its added_by_user. In this case, we have to check if another
    port has the same address and vlan, and if found, we have to create the
    entry (by changing dst). This is kernel-added entry, not user-added.
    - br_fdb_changeaddr() doesn't call fdb_delete_local() for user-added entry.

    Signed-off-by: Toshiaki Makita
    Acked-by: Vlad Yasevich
    Signed-off-by: David S. Miller

    Toshiaki Makita
     
  • br_fdb_change_mac_address() doesn't check if the local entry has the
    same address as any of bridge ports.
    Although I'm not sure when it is beneficial, current implementation allow
    the bridge device to receive any mac address of its ports.
    To preserve this behavior, we have to check if the mac address of the
    entry being deleted is identical to that of any port.

    As this check is almost the same as that in br_fdb_changeaddr(), create
    a common function fdb_delete_local() and call it from
    br_fdb_changeadddr() and br_fdb_change_mac_address().

    Signed-off-by: Toshiaki Makita
    Acked-by: Vlad Yasevich
    Signed-off-by: David S. Miller

    Toshiaki Makita
     
  • We should take into account the followings when deleting a local fdb
    entry.

    - nbp_vlan_find() can be used only when vid != 0 to check if an entry is
    deletable, because a fdb entry with vid 0 can exist at any time while
    nbp_vlan_find() always return false with vid 0.

    Example of problematic case:
    ip link set eth0 address 12:34:56:78:90:ab
    ip link set eth1 address 12:34:56:78:90:ab
    brctl addif br0 eth0
    brctl addif br0 eth1
    ip link set eth0 address aa:bb:cc:dd:ee:ff
    Then, the fdb entry 12:34:56:78:90:ab will be deleted even though the
    bridge port eth1 still has that address.

    - The port to which the bridge device is attached might needs a local entry
    if its mac address is set manually.

    Example of problematic case:
    ip link set eth0 address 12:34:56:78:90:ab
    brctl addif br0 eth0
    ip link set br0 address 12:34:56:78:90:ab
    ip link set eth0 address aa:bb:cc:dd:ee:ff
    Then, the fdb still must have the entry 12:34:56:78:90:ab, but it will be
    deleted.

    We can use br->dev->addr_assign_type to check if the address is manually
    set or not, but I propose another approach.

    Since we delete and insert local entries whenever changing mac address
    of the bridge device, we can change dst of the entry to NULL regardless of
    addr_assign_type when deleting an entry associated with a certain port,
    and if it is found to be unnecessary later, then delete it.
    That is, if changing mac address of a port, the entry might be changed
    to its dst being NULL first, but is eventually deleted when recalculating
    and changing bridge id.

    This approach is especially useful when we want to share the code with
    deleting vlan in which the bridge device might want such an entry regardless
    of addr_assign_type, and makes things easy because we don't have to consider
    if mac address of the bridge device will be changed or not at the time we
    delete a local entry of a port, which means fdb code will not be bothered
    even if the bridge id calculating logic is changed in the future.

    Also, this change reduces inconsistent state, where frames whose dst is the
    mac address of the bridge, can't reach the bridge because of premature fdb
    entry deletion. This change reduces the possibility that the bridge device
    replies unreachable mac address to arp requests, which could occur during
    the short window between calling del_nbp() and br_stp_recalculate_bridge_id()
    in br_del_if(). This will effective after br_fdb_delete_by_port() starts to
    use the same code by following patch.

    Signed-off-by: Toshiaki Makita
    Acked-by: Vlad Yasevich
    Signed-off-by: David S. Miller

    Toshiaki Makita
     
  • Vlan code may need fdb change when changing mac address of bridge device
    even if it is caused by the mac address changing of a bridge port.

    Example configuration:
    ip link set eth0 address 12:34:56:78:90:ab
    ip link set eth1 address aa:bb:cc:dd:ee:ff
    brctl addif br0 eth0
    brctl addif br0 eth1 # br0 will have mac address 12:34:56:78:90:ab
    bridge vlan add dev br0 vid 10 self
    bridge vlan add dev eth0 vid 10
    We will have fdb entry such that f->dst == NULL, f->vlan_id == 10 and
    f->addr == 12:34:56:78:90:ab at this time.
    Next, change the mac address of eth0 to greater value.
    ip link set eth0 address ee:ff:12:34:56:78
    Then, mac address of br0 will be recalculated and set to aa:bb:cc:dd:ee:ff.
    However, an entry aa:bb:cc:dd:ee:ff will not be created and we will be not
    able to communicate using br0 on vlan 10.

    Address this issue by deleting and adding local entries whenever
    changing the mac address of the bridge device.

    If there already exists an entry that has the same address, for example,
    in case that br_fdb_changeaddr() has already inserted it,
    br_fdb_change_mac_address() will simply fail to insert it and no
    duplicated entry will be made, as it was.

    This approach also needs br_add_if() to call br_fdb_insert() before
    br_stp_recalculate_bridge_id() so that we don't create an entry whose
    dst == NULL in this function to preserve previous behavior.

    Note that this is a slight change in behavior where the bridge device can
    receive the traffic to the new address before calling
    br_stp_recalculate_bridge_id() in br_add_if().
    However, it is not a problem because we have already the address on the
    new port and such a way to insert new one before recalculating bridge id
    is taken in br_device_event() as well.

    Signed-off-by: Toshiaki Makita
    Acked-by: Vlad Yasevich
    Signed-off-by: David S. Miller

    Toshiaki Makita
     
  • We have been always failed to delete the old entry at
    br_fdb_change_mac_address() because br_set_mac_address() updates
    dev->dev_addr before calling br_fdb_change_mac_address() and
    br_fdb_change_mac_address() uses dev->dev_addr to find the old entry.

    That update of dev_addr is completely unnecessary because the same work
    is done in br_stp_change_bridge_id() which is called right away after
    calling br_fdb_change_mac_address().

    Signed-off-by: Toshiaki Makita
    Acked-by: Vlad Yasevich
    Signed-off-by: David S. Miller

    Toshiaki Makita
     
  • Since commit bc9a25d21ef8 ("bridge: Add vlan support for local fdb entries"),
    br_fdb_changeaddr() has inserted a new local fdb entry only if it can
    find old one. But if we have two ports where they have the same address
    or user has deleted a local entry, there will be no entry for one of the
    ports.

    Example of problematic case:
    ip link set eth0 address aa:bb:cc:dd:ee:ff
    ip link set eth1 address aa:bb:cc:dd:ee:ff
    brctl addif br0 eth0
    brctl addif br0 eth1 # eth1 will not have a local entry due to dup.
    ip link set eth1 address 12:34:56:78:90:ab
    Then, the new entry for the address 12:34:56:78:90:ab will not be
    created, and the bridge device will not be able to communicate.

    Insert new entries regardless of whether we can find old entries or not.

    Signed-off-by: Toshiaki Makita
    Acked-by: Vlad Yasevich
    Signed-off-by: David S. Miller

    Toshiaki Makita
     
  • br_fdb_changeaddr() assumes that there is at most one local entry per port
    per vlan. It used to be true, but since commit 36fd2b63e3b4 ("bridge: allow
    creating/deleting fdb entries via netlink"), it has not been so.
    Therefore, the function might fail to search a correct previous address
    to be deleted and delete an arbitrary local entry if user has added local
    entries manually.

    Example of problematic case:
    ip link set eth0 address ee:ff:12:34:56:78
    brctl addif br0 eth0
    bridge fdb add 12:34:56:78:90:ab dev eth0 master
    ip link set eth0 address aa:bb:cc:dd:ee:ff
    Then, the address 12:34:56:78:90:ab might be deleted instead of
    ee:ff:12:34:56:78, the original mac address of eth0.

    Address this issue by introducing a new flag, added_by_user, to struct
    net_bridge_fdb_entry.

    Note that br_fdb_delete_by_port() has to set added_by_user to 0 in cases
    like:
    ip link set eth0 address 12:34:56:78:90:ab
    ip link set eth1 address aa:bb:cc:dd:ee:ff
    brctl addif br0 eth0
    bridge fdb add aa:bb:cc:dd:ee:ff dev eth0 master
    brctl addif br0 eth1
    brctl delif br0 eth0
    In this case, kernel should delete the user-added entry aa:bb:cc:dd:ee:ff,
    but it also should have been added by "brctl addif br0 eth1" originally,
    so we don't delete it and treat it a new kernel-created entry.

    Signed-off-by: Toshiaki Makita
    Signed-off-by: David S. Miller

    Toshiaki Makita
     

07 Feb, 2014

1 commit

  • Commit 93d8bf9fb8f3 ("bridge: cleanup netpoll code") introduced
    a check in br_netpoll_enable(), but this check is incorrect for
    br_netpoll_setup(). This patch moves the code after the check
    into __br_netpoll_enable() and calls it in br_netpoll_setup().
    For br_add_if(), the check is still needed.

    Fixes: 93d8bf9fb8f3 ("bridge: cleanup netpoll code")
    Cc: Toshiaki Makita
    Cc: Stephen Hemminger
    Cc: David S. Miller
    Signed-off-by: Cong Wang
    Signed-off-by: Cong Wang
    Acked-by: Toshiaki Makita
    Tested-by: Toshiaki Makita
    Signed-off-by: David S. Miller

    Cong Wang
     

23 Jan, 2014

1 commit

  • br_handle_vlan() pushes HW accelerated vlan tag into skbuff when outgoing
    port is the bridge device.
    This is unnecessary because __netif_receive_skb_core() can handle skbs
    with HW accelerated vlan tag. In current implementation,
    __netif_receive_skb_core() needs to extract the vlan tag embedded in skb
    data. This could cause low network performance especially when receiving
    frames at a high frame rate on the bridge device.

    Signed-off-by: Toshiaki Makita
    Acked-by: Vlad Yasevich
    Signed-off-by: David S. Miller

    Toshiaki Makita
     

14 Jan, 2014

1 commit


10 Jan, 2014

4 commits


08 Jan, 2014

2 commits

  • Add support to register chains to multiple hooks for different address
    families for mixed IPv4/IPv6 tables.

    Signed-off-by: Patrick McHardy

    Patrick McHardy
     
  • Currently the AF-specific hook functions override the chain-type specific
    hook functions. That doesn't make too much sense since the chain types
    are a special case of the AF-specific hooks.

    Make the AF-specific hook functions the default and make the optional
    chain type hooks override them.

    As a side effect, the necessary code restructuring reduces the code size,
    f.i. in case of nf_tables_ipv4.o:

    nf_tables_ipv4_init_net | -24
    nft_do_chain_ipv4 | -113
    2 functions changed, 137 bytes removed, diff: -137

    Signed-off-by: Patrick McHardy
    Signed-off-by: Pablo Neira Ayuso

    Patrick McHardy
     

07 Jan, 2014

3 commits

  • Conflicts:
    drivers/net/ethernet/qlogic/qlcnic/qlcnic_sriov_pf.c
    net/ipv6/ip6_tunnel.c
    net/ipv6/ip6_vti.c

    ipv6 tunnel statistic bug fixes conflicting with consolidation into
    generic sw per-cpu net stats.

    qlogic conflict between queue counting bug fix and the addition
    of multiple MAC address support.

    Signed-off-by: David S. Miller

    David S. Miller
     
  • Use DEVICE_ATTR_RO/RW macros to simplify bridge sysfs attribute definitions.

    Signed-off-by: Scott Feldman
    Signed-off-by: David S. Miller

    sfeldma@cumulusnetworks.com
     
  • br_multicast_set_hash_max() is called from process context in
    net/bridge/br_sysfs_br.c by the sysfs store_hash_max() function.

    br_multicast_set_hash_max() calls spin_lock(&br->multicast_lock),
    which can deadlock the CPU if a softirq that also tries to take the
    same lock interrupts br_multicast_set_hash_max() while the lock is
    held . This can happen quite easily when any of the bridge multicast
    timers expire, which try to take the same lock.

    The fix here is to use spin_lock_bh(), preventing other softirqs from
    executing on this CPU.

    Steps to reproduce:

    1. Create a bridge with several interfaces (I used 4).
    2. Set the "multicast query interval" to a low number, like 2.
    3. Enable the bridge as a multicast querier.
    4. Repeatedly set the bridge hash_max parameter via sysfs.

    # brctl addbr br0
    # brctl addif br0 eth1 eth2 eth3 eth4
    # brctl setmcqi br0 2
    # brctl setmcquerier br0 1

    # while true ; do echo 4096 > /sys/class/net/br0/bridge/hash_max; done

    Signed-off-by: Curt Brune
    Signed-off-by: Scott Feldman
    Signed-off-by: David S. Miller

    Curt Brune
     

05 Jan, 2014

1 commit