* [PATCH v4 06/11] net: dsa: microchip: ksz8795: change drivers prefix to be generic
From: Michael Grzeschik @ 2020-08-03 5:44 UTC (permalink / raw)
To: andrew; +Cc: netdev, f.fainelli, davem, kernel
In-Reply-To: <20200803054442.20089-1-m.grzeschik@pengutronix.de>
The driver can be used on other chips of this type. To reflect
this we rename the drivers prefix from ksz8795 to ksz8.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
v1 -> v4: - extracted this change from bigger previous patch
drivers/net/dsa/microchip/ksz8795.c | 222 ++++++++++++------------
drivers/net/dsa/microchip/ksz8795_spi.c | 2 +-
drivers/net/dsa/microchip/ksz_common.h | 2 +-
3 files changed, 111 insertions(+), 115 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index ba722f730bf0f7b..c21125a0b30e5c8 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -74,7 +74,7 @@ static void ksz_port_cfg(struct ksz_device *dev, int port, int offset, u8 bits,
bits, set ? bits : 0);
}
-static int ksz8795_reset_switch(struct ksz_device *dev)
+static int ksz8_reset_switch(struct ksz_device *dev)
{
/* reset switch */
ksz_write8(dev, REG_POWER_MANAGEMENT_1,
@@ -117,8 +117,7 @@ static void ksz8795_set_prio_queue(struct ksz_device *dev, int port, int queue)
true);
}
-static void ksz8795_r_mib_cnt(struct ksz_device *dev, int port, u16 addr,
- u64 *cnt)
+static void ksz8_r_mib_cnt(struct ksz_device *dev, int port, u16 addr, u64 *cnt)
{
u16 ctrl_addr;
u32 data;
@@ -148,8 +147,8 @@ static void ksz8795_r_mib_cnt(struct ksz_device *dev, int port, u16 addr,
mutex_unlock(&dev->alu_mutex);
}
-static void ksz8795_r_mib_pkt(struct ksz_device *dev, int port, u16 addr,
- u64 *dropped, u64 *cnt)
+static void ksz8_r_mib_pkt(struct ksz_device *dev, int port, u16 addr,
+ u64 *dropped, u64 *cnt)
{
u16 ctrl_addr;
u32 data;
@@ -195,7 +194,7 @@ static void ksz8795_r_mib_pkt(struct ksz_device *dev, int port, u16 addr,
mutex_unlock(&dev->alu_mutex);
}
-static void ksz8795_freeze_mib(struct ksz_device *dev, int port, bool freeze)
+static void ksz8_freeze_mib(struct ksz_device *dev, int port, bool freeze)
{
/* enable the port for flush/freeze function */
if (freeze)
@@ -207,7 +206,7 @@ static void ksz8795_freeze_mib(struct ksz_device *dev, int port, bool freeze)
ksz_cfg(dev, REG_SW_CTRL_6, BIT(port), false);
}
-static void ksz8795_port_init_cnt(struct ksz_device *dev, int port)
+static void ksz8_port_init_cnt(struct ksz_device *dev, int port)
{
struct ksz_port_mib *mib = &dev->ports[port].mib;
@@ -235,8 +234,7 @@ static void ksz8795_port_init_cnt(struct ksz_device *dev, int port)
memset(mib->counters, 0, dev->mib_cnt * sizeof(u64));
}
-static void ksz8795_r_table(struct ksz_device *dev, int table, u16 addr,
- u64 *data)
+static void ksz8_r_table(struct ksz_device *dev, int table, u16 addr, u64 *data)
{
u16 ctrl_addr;
@@ -248,8 +246,7 @@ static void ksz8795_r_table(struct ksz_device *dev, int table, u16 addr,
mutex_unlock(&dev->alu_mutex);
}
-static void ksz8795_w_table(struct ksz_device *dev, int table, u16 addr,
- u64 data)
+static void ksz8_w_table(struct ksz_device *dev, int table, u16 addr, u64 data)
{
u16 ctrl_addr;
@@ -261,7 +258,7 @@ static void ksz8795_w_table(struct ksz_device *dev, int table, u16 addr,
mutex_unlock(&dev->alu_mutex);
}
-static int ksz8795_valid_dyn_entry(struct ksz_device *dev, u8 *data)
+static int ksz8_valid_dyn_entry(struct ksz_device *dev, u8 *data)
{
int timeout = 100;
@@ -284,9 +281,9 @@ static int ksz8795_valid_dyn_entry(struct ksz_device *dev, u8 *data)
return 0;
}
-static int ksz8795_r_dyn_mac_table(struct ksz_device *dev, u16 addr,
- u8 *mac_addr, u8 *fid, u8 *src_port,
- u8 *timestamp, u16 *entries)
+static int ksz8_r_dyn_mac_table(struct ksz_device *dev, u16 addr,
+ u8 *mac_addr, u8 *fid, u8 *src_port,
+ u8 *timestamp, u16 *entries)
{
u32 data_hi, data_lo;
u16 ctrl_addr;
@@ -298,7 +295,7 @@ static int ksz8795_r_dyn_mac_table(struct ksz_device *dev, u16 addr,
mutex_lock(&dev->alu_mutex);
ksz_write16(dev, REG_IND_CTRL_0, ctrl_addr);
- rc = ksz8795_valid_dyn_entry(dev, &data);
+ rc = ksz8_valid_dyn_entry(dev, &data);
if (rc == -EAGAIN) {
if (addr == 0)
*entries = 0;
@@ -341,13 +338,13 @@ static int ksz8795_r_dyn_mac_table(struct ksz_device *dev, u16 addr,
return rc;
}
-static int ksz8795_r_sta_mac_table(struct ksz_device *dev, u16 addr,
- struct alu_struct *alu)
+static int ksz8_r_sta_mac_table(struct ksz_device *dev, u16 addr,
+ struct alu_struct *alu)
{
u32 data_hi, data_lo;
u64 data;
- ksz8795_r_table(dev, TABLE_STATIC_MAC, addr, &data);
+ ksz8_r_table(dev, TABLE_STATIC_MAC, addr, &data);
data_hi = data >> 32;
data_lo = (u32)data;
if (data_hi & (STATIC_MAC_TABLE_VALID | STATIC_MAC_TABLE_OVERRIDE)) {
@@ -370,8 +367,8 @@ static int ksz8795_r_sta_mac_table(struct ksz_device *dev, u16 addr,
return -ENXIO;
}
-static void ksz8795_w_sta_mac_table(struct ksz_device *dev, u16 addr,
- struct alu_struct *alu)
+static void ksz8_w_sta_mac_table(struct ksz_device *dev, u16 addr,
+ struct alu_struct *alu)
{
u32 data_hi, data_lo;
u64 data;
@@ -394,17 +391,17 @@ static void ksz8795_w_sta_mac_table(struct ksz_device *dev, u16 addr,
data_hi &= ~STATIC_MAC_TABLE_OVERRIDE;
data = (u64)data_hi << 32 | data_lo;
- ksz8795_w_table(dev, TABLE_STATIC_MAC, addr, data);
+ ksz8_w_table(dev, TABLE_STATIC_MAC, addr, data);
}
-static void ksz8795_from_vlan(u16 vlan, u8 *fid, u8 *member, u8 *valid)
+static void ksz8_from_vlan(u16 vlan, u8 *fid, u8 *member, u8 *valid)
{
*fid = vlan & VLAN_TABLE_FID;
*member = (vlan & VLAN_TABLE_MEMBERSHIP) >> VLAN_TABLE_MEMBERSHIP_S;
*valid = !!(vlan & VLAN_TABLE_VALID);
}
-static void ksz8795_to_vlan(u8 fid, u8 member, u8 valid, u16 *vlan)
+static void ksz8_to_vlan(u8 fid, u8 member, u8 valid, u16 *vlan)
{
*vlan = fid;
*vlan |= (u16)member << VLAN_TABLE_MEMBERSHIP_S;
@@ -412,12 +409,14 @@ static void ksz8795_to_vlan(u8 fid, u8 member, u8 valid, u16 *vlan)
*vlan |= VLAN_TABLE_VALID;
}
-static void ksz8795_r_vlan_entries(struct ksz_device *dev, u16 addr)
+static void ksz8_r_vlan_entries(struct ksz_device *dev, u16 addr)
{
+ struct ksz8 *ksz8 = dev->priv;
+ struct ksz_shifts *shifts = ksz8->shifts;
u64 data;
int i;
- ksz8795_r_table(dev, TABLE_VLAN, addr, &data);
+ ksz8_r_table(dev, TABLE_VLAN, addr, &data);
addr *= dev->port_cnt;
for (i = 0; i < dev->port_cnt; i++) {
dev->vlan_cache[addr + i].table[0] = (u16)data;
@@ -425,7 +424,7 @@ static void ksz8795_r_vlan_entries(struct ksz_device *dev, u16 addr)
}
}
-static void ksz8795_r_vlan_table(struct ksz_device *dev, u16 vid, u16 *vlan)
+static void ksz8_r_vlan_table(struct ksz_device *dev, u16 vid, u32 *vlan)
{
int index;
u16 *data;
@@ -435,11 +434,11 @@ static void ksz8795_r_vlan_table(struct ksz_device *dev, u16 vid, u16 *vlan)
data = (u16 *)&buf;
addr = vid / dev->port_cnt;
index = vid & 3;
- ksz8795_r_table(dev, TABLE_VLAN, addr, &buf);
+ ksz8_r_table(dev, TABLE_VLAN, addr, &buf);
*vlan = data[index];
}
-static void ksz8795_w_vlan_table(struct ksz_device *dev, u16 vid, u16 vlan)
+static void ksz8_w_vlan_table(struct ksz_device *dev, u16 vid, u32 vlan)
{
int index;
u16 *data;
@@ -449,13 +448,13 @@ static void ksz8795_w_vlan_table(struct ksz_device *dev, u16 vid, u16 vlan)
data = (u16 *)&buf;
addr = vid / dev->port_cnt;
index = vid & 3;
- ksz8795_r_table(dev, TABLE_VLAN, addr, &buf);
+ ksz8_r_table(dev, TABLE_VLAN, addr, &buf);
data[index] = vlan;
dev->vlan_cache[vid].table[0] = vlan;
- ksz8795_w_table(dev, TABLE_VLAN, addr, buf);
+ ksz8_w_table(dev, TABLE_VLAN, addr, buf);
}
-static void ksz8795_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
+static void ksz8_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
{
u8 restart, speed, ctrl, link;
int processed = true;
@@ -546,7 +545,7 @@ static void ksz8795_r_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 *val)
*val = data;
}
-static void ksz8795_w_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 val)
+static void ksz8_w_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 val)
{
u8 p = phy;
u8 restart, speed, ctrl, data;
@@ -644,15 +643,15 @@ static void ksz8795_w_phy(struct ksz_device *dev, u16 phy, u16 reg, u16 val)
}
}
-static enum dsa_tag_protocol ksz8795_get_tag_protocol(struct dsa_switch *ds,
- int port,
- enum dsa_tag_protocol mp)
+static enum dsa_tag_protocol ksz8_get_tag_protocol(struct dsa_switch *ds,
+ int port,
+ enum dsa_tag_protocol mp)
{
return DSA_TAG_PROTO_KSZ8795;
}
-static void ksz8795_get_strings(struct dsa_switch *ds, int port,
- u32 stringset, uint8_t *buf)
+static void ksz8_get_strings(struct dsa_switch *ds, int port,
+ u32 stringset, uint8_t *buf)
{
int i;
@@ -662,8 +661,7 @@ static void ksz8795_get_strings(struct dsa_switch *ds, int port,
}
}
-static void ksz8795_cfg_port_member(struct ksz_device *dev, int port,
- u8 member)
+static void ksz8_cfg_port_member(struct ksz_device *dev, int port, u8 member)
{
u8 data;
@@ -674,8 +672,7 @@ static void ksz8795_cfg_port_member(struct ksz_device *dev, int port,
dev->ports[port].member = member;
}
-static void ksz8795_port_stp_state_set(struct dsa_switch *ds, int port,
- u8 state)
+static void ksz8_port_stp_state_set(struct dsa_switch *ds, int port, u8 state)
{
struct ksz_device *dev = ds->priv;
int forward = dev->member;
@@ -733,7 +730,7 @@ static void ksz8795_port_stp_state_set(struct dsa_switch *ds, int port,
p->stp_state = state;
/* Port membership may share register with STP state. */
if (member >= 0 && member != p->member)
- ksz8795_cfg_port_member(dev, port, (u8)member);
+ ksz8_cfg_port_member(dev, port, (u8)member);
/* Check if forwarding needs to be updated. */
if (state != BR_STATE_FORWARDING) {
@@ -748,7 +745,7 @@ static void ksz8795_port_stp_state_set(struct dsa_switch *ds, int port,
ksz_update_port_member(dev, port);
}
-static void ksz8795_flush_dyn_mac_table(struct ksz_device *dev, int port)
+static void ksz8_flush_dyn_mac_table(struct ksz_device *dev, int port)
{
u8 *learn = kzalloc(dev->mib_port_cnt, GFP_KERNEL);
int first, index, cnt;
@@ -782,8 +779,7 @@ static void ksz8795_flush_dyn_mac_table(struct ksz_device *dev, int port)
kfree(learn);
}
-static int ksz8795_port_vlan_filtering(struct dsa_switch *ds, int port,
- bool flag)
+static int ksz8_port_vlan_filtering(struct dsa_switch *ds, int port, bool flag)
{
struct ksz_device *dev = ds->priv;
@@ -792,8 +788,8 @@ static int ksz8795_port_vlan_filtering(struct dsa_switch *ds, int port,
return 0;
}
-static void ksz8795_port_vlan_add(struct dsa_switch *ds, int port,
- const struct switchdev_obj_port_vlan *vlan)
+static void ksz8_port_vlan_add(struct dsa_switch *ds, int port,
+ const struct switchdev_obj_port_vlan *vlan)
{
bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
struct ksz_device *dev = ds->priv;
@@ -803,8 +799,8 @@ static void ksz8795_port_vlan_add(struct dsa_switch *ds, int port,
ksz_port_cfg(dev, port, P_TAG_CTRL, PORT_REMOVE_TAG, untagged);
for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
- ksz8795_r_vlan_table(dev, vid, &data);
- ksz8795_from_vlan(data, &fid, &member, &valid);
+ ksz8_r_vlan_table(dev, vid, &data);
+ ksz8_from_vlan(data, &fid, &member, &valid);
/* First time to setup the VLAN entry. */
if (!valid) {
@@ -814,8 +810,8 @@ static void ksz8795_port_vlan_add(struct dsa_switch *ds, int port,
}
member |= BIT(port);
- ksz8795_to_vlan(fid, member, valid, &data);
- ksz8795_w_vlan_table(dev, vid, data);
+ ksz8_to_vlan(fid, member, valid, &data);
+ ksz8_w_vlan_table(dev, vid, data);
/* change PVID */
if (vlan->flags & BRIDGE_VLAN_INFO_PVID)
@@ -830,8 +826,8 @@ static void ksz8795_port_vlan_add(struct dsa_switch *ds, int port,
}
}
-static int ksz8795_port_vlan_del(struct dsa_switch *ds, int port,
- const struct switchdev_obj_port_vlan *vlan)
+static int ksz8_port_vlan_del(struct dsa_switch *ds, int port,
+ const struct switchdev_obj_port_vlan *vlan)
{
bool untagged = vlan->flags & BRIDGE_VLAN_INFO_UNTAGGED;
struct ksz_device *dev = ds->priv;
@@ -844,8 +840,8 @@ static int ksz8795_port_vlan_del(struct dsa_switch *ds, int port,
ksz_port_cfg(dev, port, P_TAG_CTRL, PORT_REMOVE_TAG, untagged);
for (vid = vlan->vid_begin; vid <= vlan->vid_end; vid++) {
- ksz8795_r_vlan_table(dev, vid, &data);
- ksz8795_from_vlan(data, &fid, &member, &valid);
+ ksz8_r_vlan_table(dev, vid, &data);
+ ksz8_from_vlan(data, &fid, &member, &valid);
member &= ~BIT(port);
@@ -858,8 +854,8 @@ static int ksz8795_port_vlan_del(struct dsa_switch *ds, int port,
if (pvid == vid)
new_pvid = 1;
- ksz8795_to_vlan(fid, member, valid, &data);
- ksz8795_w_vlan_table(dev, vid, data);
+ ksz8_to_vlan(fid, member, valid, &data);
+ ksz8_w_vlan_table(dev, vid, data);
}
if (new_pvid != pvid)
@@ -868,9 +864,9 @@ static int ksz8795_port_vlan_del(struct dsa_switch *ds, int port,
return 0;
}
-static int ksz8795_port_mirror_add(struct dsa_switch *ds, int port,
- struct dsa_mall_mirror_tc_entry *mirror,
- bool ingress)
+static int ksz8_port_mirror_add(struct dsa_switch *ds, int port,
+ struct dsa_mall_mirror_tc_entry *mirror,
+ bool ingress)
{
struct ksz_device *dev = ds->priv;
@@ -892,8 +888,8 @@ static int ksz8795_port_mirror_add(struct dsa_switch *ds, int port,
return 0;
}
-static void ksz8795_port_mirror_del(struct dsa_switch *ds, int port,
- struct dsa_mall_mirror_tc_entry *mirror)
+static void ksz8_port_mirror_del(struct dsa_switch *ds, int port,
+ struct dsa_mall_mirror_tc_entry *mirror)
{
struct ksz_device *dev = ds->priv;
u8 data;
@@ -913,7 +909,7 @@ static void ksz8795_port_mirror_del(struct dsa_switch *ds, int port,
PORT_MIRROR_SNIFFER, false);
}
-static void ksz8795_port_setup(struct ksz_device *dev, int port, bool cpu_port)
+static void ksz8_port_setup(struct ksz_device *dev, int port, bool cpu_port)
{
struct ksz_port *p = &dev->ports[port];
u8 data8, member;
@@ -971,10 +967,10 @@ static void ksz8795_port_setup(struct ksz_device *dev, int port, bool cpu_port)
} else {
member = dev->host_mask | p->vid_member;
}
- ksz8795_cfg_port_member(dev, port, member);
+ ksz8_cfg_port_member(dev, port, member);
}
-static void ksz8795_config_cpu_port(struct dsa_switch *ds)
+static void ksz8_config_cpu_port(struct dsa_switch *ds)
{
struct ksz_device *dev = ds->priv;
struct ksz_port *p;
@@ -991,7 +987,7 @@ static void ksz8795_config_cpu_port(struct dsa_switch *ds)
p->vid_member = dev->port_mask;
p->on = 1;
- ksz8795_port_setup(dev, dev->cpu_port, true);
+ ksz8_port_setup(dev, dev->cpu_port, true);
dev->member = dev->host_mask;
for (i = 0; i < dev->port_cnt; i++) {
@@ -1002,7 +998,7 @@ static void ksz8795_config_cpu_port(struct dsa_switch *ds)
*/
p->vid_member = BIT(i);
p->member = dev->port_mask;
- ksz8795_port_stp_state_set(ds, i, BR_STATE_DISABLED);
+ ksz8_port_stp_state_set(ds, i, BR_STATE_DISABLED);
/* Last port may be disabled. */
if (i == dev->port_cnt)
@@ -1026,7 +1022,7 @@ static void ksz8795_config_cpu_port(struct dsa_switch *ds)
}
}
-static int ksz8795_setup(struct dsa_switch *ds)
+static int ksz8_setup(struct dsa_switch *ds)
{
struct ksz_device *dev = ds->priv;
struct alu_struct alu;
@@ -1037,7 +1033,7 @@ static int ksz8795_setup(struct dsa_switch *ds)
if (!dev->vlan_cache)
return -ENOMEM;
- ret = ksz8795_reset_switch(dev);
+ ret = ksz8_reset_switch(dev);
if (ret) {
dev_err(ds->dev, "failed to reset switch\n");
return ret;
@@ -1060,7 +1056,7 @@ static int ksz8795_setup(struct dsa_switch *ds)
UNICAST_VLAN_BOUNDARY | NO_EXC_COLLISION_DROP,
UNICAST_VLAN_BOUNDARY | NO_EXC_COLLISION_DROP);
- ksz8795_config_cpu_port(ds);
+ ksz8_config_cpu_port(ds);
ksz_cfg(dev, REG_SW_CTRL_2, MULTICAST_STORM_DISABLE, true);
@@ -1075,7 +1071,7 @@ static int ksz8795_setup(struct dsa_switch *ds)
BROADCAST_STORM_PROT_RATE) / 100);
for (i = 0; i < VLAN_TABLE_ENTRIES; i++)
- ksz8795_r_vlan_entries(dev, i);
+ ksz8_r_vlan_entries(dev, i);
/* Setup STP address for STP operation. */
memset(&alu, 0, sizeof(alu));
@@ -1084,45 +1080,45 @@ static int ksz8795_setup(struct dsa_switch *ds)
alu.is_override = true;
alu.port_forward = dev->host_mask;
- ksz8795_w_sta_mac_table(dev, 0, &alu);
+ ksz8_w_sta_mac_table(dev, 0, &alu);
ksz_init_mib_timer(dev);
return 0;
}
-static const struct dsa_switch_ops ksz8795_switch_ops = {
- .get_tag_protocol = ksz8795_get_tag_protocol,
- .setup = ksz8795_setup,
+static const struct dsa_switch_ops ksz8_switch_ops = {
+ .get_tag_protocol = ksz8_get_tag_protocol,
+ .setup = ksz8_setup,
.phy_read = ksz_phy_read16,
.phy_write = ksz_phy_write16,
.phylink_mac_link_down = ksz_mac_link_down,
.port_enable = ksz_enable_port,
- .get_strings = ksz8795_get_strings,
+ .get_strings = ksz8_get_strings,
.get_ethtool_stats = ksz_get_ethtool_stats,
.get_sset_count = ksz_sset_count,
.port_bridge_join = ksz_port_bridge_join,
.port_bridge_leave = ksz_port_bridge_leave,
- .port_stp_state_set = ksz8795_port_stp_state_set,
+ .port_stp_state_set = ksz8_port_stp_state_set,
.port_fast_age = ksz_port_fast_age,
- .port_vlan_filtering = ksz8795_port_vlan_filtering,
+ .port_vlan_filtering = ksz8_port_vlan_filtering,
.port_vlan_prepare = ksz_port_vlan_prepare,
- .port_vlan_add = ksz8795_port_vlan_add,
- .port_vlan_del = ksz8795_port_vlan_del,
+ .port_vlan_add = ksz8_port_vlan_add,
+ .port_vlan_del = ksz8_port_vlan_del,
.port_fdb_dump = ksz_port_fdb_dump,
.port_mdb_prepare = ksz_port_mdb_prepare,
.port_mdb_add = ksz_port_mdb_add,
.port_mdb_del = ksz_port_mdb_del,
- .port_mirror_add = ksz8795_port_mirror_add,
- .port_mirror_del = ksz8795_port_mirror_del,
+ .port_mirror_add = ksz8_port_mirror_add,
+ .port_mirror_del = ksz8_port_mirror_del,
};
-static u32 ksz8795_get_port_addr(int port, int offset)
+static u32 ksz8_get_port_addr(int port, int offset)
{
return PORT_CTRL_ADDR(port, offset);
}
-static int ksz8795_switch_detect(struct ksz_device *dev)
+static int ksz8_switch_detect(struct ksz_device *dev)
{
u8 id1, id2;
u16 id16;
@@ -1175,7 +1171,7 @@ struct ksz_chip_data {
int port_cnt;
};
-static const struct ksz_chip_data ksz8795_switch_chips[] = {
+static const struct ksz_chip_data ksz8_switch_chips[] = {
{
.chip_id = 0x8795,
.dev_name = "KSZ8795",
@@ -1205,14 +1201,14 @@ static const struct ksz_chip_data ksz8795_switch_chips[] = {
},
};
-static int ksz8795_switch_init(struct ksz_device *dev)
+static int ksz8_switch_init(struct ksz_device *dev)
{
int i;
- dev->ds->ops = &ksz8795_switch_ops;
+ dev->ds->ops = &ksz8_switch_ops;
- for (i = 0; i < ARRAY_SIZE(ksz8795_switch_chips); i++) {
- const struct ksz_chip_data *chip = &ksz8795_switch_chips[i];
+ for (i = 0; i < ARRAY_SIZE(ksz8_switch_chips); i++) {
+ const struct ksz_chip_data *chip = &ksz8_switch_chips[i];
if (dev->chip_id == chip->chip_id) {
dev->name = chip->dev_name;
@@ -1258,36 +1254,36 @@ static int ksz8795_switch_init(struct ksz_device *dev)
return 0;
}
-static void ksz8795_switch_exit(struct ksz_device *dev)
+static void ksz8_switch_exit(struct ksz_device *dev)
{
- ksz8795_reset_switch(dev);
+ ksz8_reset_switch(dev);
}
-static const struct ksz_dev_ops ksz8795_dev_ops = {
- .get_port_addr = ksz8795_get_port_addr,
- .cfg_port_member = ksz8795_cfg_port_member,
- .flush_dyn_mac_table = ksz8795_flush_dyn_mac_table,
- .port_setup = ksz8795_port_setup,
- .r_phy = ksz8795_r_phy,
- .w_phy = ksz8795_w_phy,
- .r_dyn_mac_table = ksz8795_r_dyn_mac_table,
- .r_sta_mac_table = ksz8795_r_sta_mac_table,
- .w_sta_mac_table = ksz8795_w_sta_mac_table,
- .r_mib_cnt = ksz8795_r_mib_cnt,
- .r_mib_pkt = ksz8795_r_mib_pkt,
- .freeze_mib = ksz8795_freeze_mib,
- .port_init_cnt = ksz8795_port_init_cnt,
- .shutdown = ksz8795_reset_switch,
- .detect = ksz8795_switch_detect,
- .init = ksz8795_switch_init,
- .exit = ksz8795_switch_exit,
+static const struct ksz_dev_ops ksz8_dev_ops = {
+ .get_port_addr = ksz8_get_port_addr,
+ .cfg_port_member = ksz8_cfg_port_member,
+ .flush_dyn_mac_table = ksz8_flush_dyn_mac_table,
+ .port_setup = ksz8_port_setup,
+ .r_phy = ksz8_r_phy,
+ .w_phy = ksz8_w_phy,
+ .r_dyn_mac_table = ksz8_r_dyn_mac_table,
+ .r_sta_mac_table = ksz8_r_sta_mac_table,
+ .w_sta_mac_table = ksz8_w_sta_mac_table,
+ .r_mib_cnt = ksz8_r_mib_cnt,
+ .r_mib_pkt = ksz8_r_mib_pkt,
+ .freeze_mib = ksz8_freeze_mib,
+ .port_init_cnt = ksz8_port_init_cnt,
+ .shutdown = ksz8_reset_switch,
+ .detect = ksz8_switch_detect,
+ .init = ksz8_switch_init,
+ .exit = ksz8_switch_exit,
};
-int ksz8795_switch_register(struct ksz_device *dev)
+int ksz8_switch_register(struct ksz_device *dev)
{
- return ksz_switch_register(dev, &ksz8795_dev_ops);
+ return ksz_switch_register(dev, &ksz8_dev_ops);
}
-EXPORT_SYMBOL(ksz8795_switch_register);
+EXPORT_SYMBOL(ksz8_switch_register);
MODULE_AUTHOR("Tristram Ha <Tristram.Ha@microchip.com>");
MODULE_DESCRIPTION("Microchip KSZ8795 Series Switch DSA Driver");
diff --git a/drivers/net/dsa/microchip/ksz8795_spi.c b/drivers/net/dsa/microchip/ksz8795_spi.c
index 8b00f8e6c02f4f2..3bab09c46f6a7bd 100644
--- a/drivers/net/dsa/microchip/ksz8795_spi.c
+++ b/drivers/net/dsa/microchip/ksz8795_spi.c
@@ -49,7 +49,7 @@ static int ksz8795_spi_probe(struct spi_device *spi)
if (spi->dev.platform_data)
dev->pdata = spi->dev.platform_data;
- ret = ksz8795_switch_register(dev);
+ ret = ksz8_switch_register(dev);
/* Main DSA driver may not be started yet. */
if (ret)
diff --git a/drivers/net/dsa/microchip/ksz_common.h b/drivers/net/dsa/microchip/ksz_common.h
index 206838160f4940f..cd5aec59d3978c5 100644
--- a/drivers/net/dsa/microchip/ksz_common.h
+++ b/drivers/net/dsa/microchip/ksz_common.h
@@ -143,7 +143,7 @@ int ksz_switch_register(struct ksz_device *dev,
const struct ksz_dev_ops *ops);
void ksz_switch_remove(struct ksz_device *dev);
-int ksz8795_switch_register(struct ksz_device *dev);
+int ksz8_switch_register(struct ksz_device *dev);
int ksz9477_switch_register(struct ksz_device *dev);
void ksz_update_port_member(struct ksz_device *dev, int port);
--
2.28.0
^ permalink raw reply related
* [PATCH v4 04/11] net: dsa: microchip: ksz8795: use port_cnt where possible
From: Michael Grzeschik @ 2020-08-03 5:44 UTC (permalink / raw)
To: andrew; +Cc: netdev, f.fainelli, davem, kernel
In-Reply-To: <20200803054442.20089-1-m.grzeschik@pengutronix.de>
Since the driver can be used on more switches it needs
to use flexible port count whereever possible.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
v1 -> v4: - extracted this change from bigger previous patch
drivers/net/dsa/microchip/ksz8795.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 8f1d15ea15d9a75..947ea1e45f5b2c6 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -418,8 +418,8 @@ static void ksz8795_r_vlan_entries(struct ksz_device *dev, u16 addr)
int i;
ksz8795_r_table(dev, TABLE_VLAN, addr, &data);
- addr *= 4;
- for (i = 0; i < 4; i++) {
+ addr *= dev->port_cnt;
+ for (i = 0; i < dev->port_cnt; i++) {
dev->vlan_cache[addr + i].table[0] = (u16)data;
data >>= VLAN_TABLE_S;
}
@@ -433,7 +433,7 @@ static void ksz8795_r_vlan_table(struct ksz_device *dev, u16 vid, u16 *vlan)
u64 buf;
data = (u16 *)&buf;
- addr = vid / 4;
+ addr = vid / dev->port_cnt;
index = vid & 3;
ksz8795_r_table(dev, TABLE_VLAN, addr, &buf);
*vlan = data[index];
@@ -447,7 +447,7 @@ static void ksz8795_w_vlan_table(struct ksz_device *dev, u16 vid, u16 vlan)
u64 buf;
data = (u16 *)&buf;
- addr = vid / 4;
+ addr = vid / dev->port_cnt;
index = vid & 3;
ksz8795_r_table(dev, TABLE_VLAN, addr, &buf);
data[index] = vlan;
@@ -691,12 +691,12 @@ static void ksz8795_port_stp_state_set(struct dsa_switch *ds, int port,
switch (state) {
case BR_STATE_DISABLED:
data |= PORT_LEARN_DISABLE;
- if (port < SWITCH_PORT_NUM)
+ if (port < dev->port_cnt)
member = 0;
break;
case BR_STATE_LISTENING:
data |= (PORT_RX_ENABLE | PORT_LEARN_DISABLE);
- if (port < SWITCH_PORT_NUM &&
+ if (port < dev->port_cnt &&
p->stp_state == BR_STATE_DISABLED)
member = dev->host_mask | p->vid_member;
break;
@@ -720,7 +720,7 @@ static void ksz8795_port_stp_state_set(struct dsa_switch *ds, int port,
break;
case BR_STATE_BLOCKING:
data |= PORT_LEARN_DISABLE;
- if (port < SWITCH_PORT_NUM &&
+ if (port < dev->port_cnt &&
p->stp_state == BR_STATE_DISABLED)
member = dev->host_mask | p->vid_member;
break;
@@ -993,7 +993,7 @@ static void ksz8795_config_cpu_port(struct dsa_switch *ds)
ksz8795_port_setup(dev, dev->cpu_port, true);
dev->member = dev->host_mask;
- for (i = 0; i < SWITCH_PORT_NUM; i++) {
+ for (i = 0; i < dev->port_cnt; i++) {
p = &dev->ports[i];
/* Initialize to non-zero so that ksz_cfg_port_member() will
--
2.28.0
^ permalink raw reply related
* [PATCH v4 11/11] dt-bindings: net: dsa: document additional Microchip KSZ8863/8873 switch
From: Michael Grzeschik @ 2020-08-03 5:44 UTC (permalink / raw)
To: andrew; +Cc: netdev, f.fainelli, davem, kernel, devicetree, Rob Herring
In-Reply-To: <20200803054442.20089-1-m.grzeschik@pengutronix.de>
It is a 3-Port 10/100 Ethernet Switch. One CPU-Port and two
Switch-Ports.
Cc: devicetree@vger.kernel.org
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
v1 -> v3: - nothing changes
- already Acked-by Rob Herring
v1 -> v4: - nothing changes
Documentation/devicetree/bindings/net/dsa/ksz.txt | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/net/dsa/ksz.txt b/Documentation/devicetree/bindings/net/dsa/ksz.txt
index 95e91e84151c339..a5d71862f53cbcd 100644
--- a/Documentation/devicetree/bindings/net/dsa/ksz.txt
+++ b/Documentation/devicetree/bindings/net/dsa/ksz.txt
@@ -8,6 +8,8 @@ Required properties:
- "microchip,ksz8765"
- "microchip,ksz8794"
- "microchip,ksz8795"
+ - "microchip,ksz8863"
+ - "microchip,ksz8873"
- "microchip,ksz9477"
- "microchip,ksz9897"
- "microchip,ksz9896"
--
2.28.0
^ permalink raw reply related
* [PATCH v4 01/11] net: phy: Add support for microchip SMI0 MDIO bus
From: Michael Grzeschik @ 2020-08-03 5:44 UTC (permalink / raw)
To: andrew; +Cc: netdev, f.fainelli, davem, kernel
In-Reply-To: <20200803054442.20089-1-m.grzeschik@pengutronix.de>
From: Andrew Lunn <andrew@lunn.ch>
SMI0 is a mangled version of MDIO. The main low level difference is
the MDIO C22 OP code is always 0, not 0x2 or 0x1 for Read/Write. The
read/write information is instead encoded in the PHY address.
Extend the bit-bang code to allow the op code to be overridden, but
default to normal C22 values. Add an extra compatible to the mdio-gpio
driver, and when this compatible is present, set the op codes to 0.
A higher level driver, sitting on top of the basic MDIO bus driver can
then implement the rest of the microchip SMI0 odderties.
Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
v1 -> v2: - patch not present
v2 -> v3: - first patch
v3 -> v4: - added override_c22_op
drivers/net/phy/mdio-bitbang.c | 8 ++++++--
drivers/net/phy/mdio-gpio.c | 9 +++++++++
include/linux/mdio-bitbang.h | 3 +++
3 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/drivers/net/phy/mdio-bitbang.c b/drivers/net/phy/mdio-bitbang.c
index 5136275c8e7399f..528e255d1ffe77f 100644
--- a/drivers/net/phy/mdio-bitbang.c
+++ b/drivers/net/phy/mdio-bitbang.c
@@ -158,7 +158,7 @@ static int mdiobb_read(struct mii_bus *bus, int phy, int reg)
reg = mdiobb_cmd_addr(ctrl, phy, reg);
mdiobb_cmd(ctrl, MDIO_C45_READ, phy, reg);
} else
- mdiobb_cmd(ctrl, MDIO_READ, phy, reg);
+ mdiobb_cmd(ctrl, ctrl->op_c22_read, phy, reg);
ctrl->ops->set_mdio_dir(ctrl, 0);
@@ -189,7 +189,7 @@ static int mdiobb_write(struct mii_bus *bus, int phy, int reg, u16 val)
reg = mdiobb_cmd_addr(ctrl, phy, reg);
mdiobb_cmd(ctrl, MDIO_C45_WRITE, phy, reg);
} else
- mdiobb_cmd(ctrl, MDIO_WRITE, phy, reg);
+ mdiobb_cmd(ctrl, ctrl->op_c22_write, phy, reg);
/* send the turnaround (10) */
mdiobb_send_bit(ctrl, 1);
@@ -215,6 +215,10 @@ struct mii_bus *alloc_mdio_bitbang(struct mdiobb_ctrl *ctrl)
bus->read = mdiobb_read;
bus->write = mdiobb_write;
bus->priv = ctrl;
+ if (!ctrl->override_op_c22) {
+ ctrl->op_c22_read = MDIO_READ;
+ ctrl->op_c22_write = MDIO_WRITE;
+ }
return bus;
}
diff --git a/drivers/net/phy/mdio-gpio.c b/drivers/net/phy/mdio-gpio.c
index 1b00235d7dc5b56..e8d83cee1bc17e1 100644
--- a/drivers/net/phy/mdio-gpio.c
+++ b/drivers/net/phy/mdio-gpio.c
@@ -132,6 +132,14 @@ static struct mii_bus *mdio_gpio_bus_init(struct device *dev,
new_bus->phy_ignore_ta_mask = pdata->phy_ignore_ta_mask;
}
+ if (dev->of_node &&
+ of_device_is_compatible(dev->of_node, "microchip,mdio-smi0")) {
+ bitbang->ctrl.op_c22_read = 0;
+ bitbang->ctrl.op_c22_write = 0;
+ } else {
+ bitbang->ctrl.override_op_c22 = 1;
+ }
+
dev_set_drvdata(dev, new_bus);
return new_bus;
@@ -196,6 +204,7 @@ static int mdio_gpio_remove(struct platform_device *pdev)
static const struct of_device_id mdio_gpio_of_match[] = {
{ .compatible = "virtual,mdio-gpio", },
+ { .compatible = "microchip,mdio-smi0" },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, mdio_gpio_of_match);
diff --git a/include/linux/mdio-bitbang.h b/include/linux/mdio-bitbang.h
index 5d71e8a8500f5ed..5016e6f60de38af 100644
--- a/include/linux/mdio-bitbang.h
+++ b/include/linux/mdio-bitbang.h
@@ -33,6 +33,9 @@ struct mdiobb_ops {
struct mdiobb_ctrl {
const struct mdiobb_ops *ops;
+ unsigned int override_op_c22;
+ u8 op_c22_read;
+ u8 op_c22_write;
};
/* The returned bus is not yet registered with the phy layer. */
--
2.28.0
^ permalink raw reply related
* [PATCH v4 02/11] dt-bindings: net: mdio-gpio: add compatible for microchip,mdio-smi0
From: Michael Grzeschik @ 2020-08-03 5:44 UTC (permalink / raw)
To: andrew; +Cc: netdev, f.fainelli, davem, kernel, devicetree, Rob Herring
In-Reply-To: <20200803054442.20089-1-m.grzeschik@pengutronix.de>
Microchip SMI0 Mode is a special mode, where the MDIO Read/Write
commands are part of the PHY Address and the OP Code is always 0. We add
the compatible for this special mode of the bitbanged mdio driver.
Cc: devicetree@vger.kernel.org
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
v1 -> v2: - patch not present
v2 -> v3: - first patch
v3 -> v4: - no changes
Documentation/devicetree/bindings/net/mdio-gpio.txt | 1 +
1 file changed, 1 insertion(+)
diff --git a/Documentation/devicetree/bindings/net/mdio-gpio.txt b/Documentation/devicetree/bindings/net/mdio-gpio.txt
index 8dbcf8295c6c9ce..4d91a36c5cf5031 100644
--- a/Documentation/devicetree/bindings/net/mdio-gpio.txt
+++ b/Documentation/devicetree/bindings/net/mdio-gpio.txt
@@ -2,6 +2,7 @@ MDIO on GPIOs
Currently defined compatibles:
- virtual,gpio-mdio
+- microchip,mdio-smi0
MDC and MDIO lines connected to GPIO controllers are listed in the
gpios property as described in section VIII.1 in the following order:
--
2.28.0
^ permalink raw reply related
* [PATCH v4 03/11] net: tag: ksz: Add KSZ8863 tag code
From: Michael Grzeschik @ 2020-08-03 5:44 UTC (permalink / raw)
To: andrew; +Cc: netdev, f.fainelli, davem, kernel
In-Reply-To: <20200803054442.20089-1-m.grzeschik@pengutronix.de>
Add DSA tag code for Microchip KSZ8863 switch.
Reviewed-by: Florian Fainelli <f.fainelli@gmail.com>
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
v1 -> v2: - fixed __be16 handling
v2 -> v3: - no changes
v3 -> v4: - changed handling to only one padding byte
include/net/dsa.h | 2 ++
net/dsa/tag_ksz.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 59 insertions(+)
diff --git a/include/net/dsa.h b/include/net/dsa.h
index 75c8fac82017444..321ecbee0834f5e 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -45,6 +45,7 @@ struct phylink_link_state;
#define DSA_TAG_PROTO_OCELOT_VALUE 15
#define DSA_TAG_PROTO_AR9331_VALUE 16
#define DSA_TAG_PROTO_RTL4_A_VALUE 17
+#define DSA_TAG_PROTO_KSZ8863_VALUE 18
enum dsa_tag_protocol {
DSA_TAG_PROTO_NONE = DSA_TAG_PROTO_NONE_VALUE,
@@ -65,6 +66,7 @@ enum dsa_tag_protocol {
DSA_TAG_PROTO_OCELOT = DSA_TAG_PROTO_OCELOT_VALUE,
DSA_TAG_PROTO_AR9331 = DSA_TAG_PROTO_AR9331_VALUE,
DSA_TAG_PROTO_RTL4_A = DSA_TAG_PROTO_RTL4_A_VALUE,
+ DSA_TAG_PROTO_KSZ8863 = DSA_TAG_PROTO_KSZ8863_VALUE,
};
struct packet_type;
diff --git a/net/dsa/tag_ksz.c b/net/dsa/tag_ksz.c
index bd1a3158d79a85f..fb9cc0e84288782 100644
--- a/net/dsa/tag_ksz.c
+++ b/net/dsa/tag_ksz.c
@@ -242,8 +242,65 @@ static const struct dsa_device_ops ksz9893_netdev_ops = {
DSA_TAG_DRIVER(ksz9893_netdev_ops);
MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ9893);
+/* For ingress (Host -> KSZ8863), 1 byte is added before FCS.
+ * ---------------------------------------------------------------------------
+ * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes)
+ * ---------------------------------------------------------------------------
+ * tag0[1,0] : represents port
+ * (e.g. 0b00=addr-lookup 0b01=port1, 0b10=port2, 0b11=port1+port2)
+ * tag0[3,2] : bits two and three represent prioritization
+ * (e.g. 0b00xx=prio0, 0b01xx=prio1, 0b10xx=prio2, 0b11xx=prio3)
+ *
+ * For egress (KSZ8873 -> Host), 1 byte is added before FCS.
+ * ---------------------------------------------------------------------------
+ * DA(6bytes)|SA(6bytes)|....|Data(nbytes)|tag0(1byte)|FCS(4bytes)
+ * ---------------------------------------------------------------------------
+ * tag0[0] : zero-based value represents port
+ * (eg, 0b0=port1, 0b1=port2)
+ */
+
+static struct sk_buff *ksz8863_xmit(struct sk_buff *skb,
+ struct net_device *dev)
+{
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+ struct sk_buff *nskb;
+ u8 *tag;
+
+ nskb = ksz_common_xmit(skb, dev, KSZ_INGRESS_TAG_LEN);
+ if (!nskb)
+ return NULL;
+
+ /* Tag encoding */
+ tag = skb_put(nskb, KSZ_INGRESS_TAG_LEN);
+
+ *tag = BIT(dp->index); /* destination port */
+
+ return nskb;
+}
+
+static struct sk_buff *ksz8863_rcv(struct sk_buff *skb, struct net_device *dev,
+ struct packet_type *pt)
+{
+ /* Tag decoding */
+ u8 *tag = skb_tail_pointer(skb) - KSZ_EGRESS_TAG_LEN;
+
+ return ksz_common_rcv(skb, dev, tag[0] & 1, KSZ_EGRESS_TAG_LEN);
+}
+
+static const struct dsa_device_ops ksz8863_netdev_ops = {
+ .name = "ksz8863",
+ .proto = DSA_TAG_PROTO_KSZ8863,
+ .xmit = ksz8863_xmit,
+ .rcv = ksz8863_rcv,
+ .overhead = KSZ_INGRESS_TAG_LEN,
+};
+
+DSA_TAG_DRIVER(ksz8863_netdev_ops);
+MODULE_ALIAS_DSA_TAG_DRIVER(DSA_TAG_PROTO_KSZ8863);
+
static struct dsa_tag_driver *dsa_tag_driver_array[] = {
&DSA_TAG_DRIVER_NAME(ksz8795_netdev_ops),
+ &DSA_TAG_DRIVER_NAME(ksz8863_netdev_ops),
&DSA_TAG_DRIVER_NAME(ksz9477_netdev_ops),
&DSA_TAG_DRIVER_NAME(ksz9893_netdev_ops),
};
--
2.28.0
^ permalink raw reply related
* [PATCH v4 05/11] net: dsa: microchip: ksz8795: dynamic allocate memory for flush_dyn_mac_table
From: Michael Grzeschik @ 2020-08-03 5:44 UTC (permalink / raw)
To: andrew; +Cc: netdev, f.fainelli, davem, kernel
In-Reply-To: <20200803054442.20089-1-m.grzeschik@pengutronix.de>
To get the driver working with other chips using different port counts
the dyn_mac_table should be flushed depending on the amount of physical
ports.
Signed-off-by: Michael Grzeschik <m.grzeschik@pengutronix.de>
---
v1 -> v4: - extracted this change from bigger previous patch
drivers/net/dsa/microchip/ksz8795.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/dsa/microchip/ksz8795.c b/drivers/net/dsa/microchip/ksz8795.c
index 947ea1e45f5b2c6..ba722f730bf0f7b 100644
--- a/drivers/net/dsa/microchip/ksz8795.c
+++ b/drivers/net/dsa/microchip/ksz8795.c
@@ -750,11 +750,11 @@ static void ksz8795_port_stp_state_set(struct dsa_switch *ds, int port,
static void ksz8795_flush_dyn_mac_table(struct ksz_device *dev, int port)
{
- u8 learn[TOTAL_PORT_NUM];
+ u8 *learn = kzalloc(dev->mib_port_cnt, GFP_KERNEL);
int first, index, cnt;
struct ksz_port *p;
- if ((uint)port < TOTAL_PORT_NUM) {
+ if ((uint)port < dev->mib_port_cnt) {
first = port;
cnt = port + 1;
} else {
@@ -779,6 +779,7 @@ static void ksz8795_flush_dyn_mac_table(struct ksz_device *dev, int port)
if (!(learn[index] & PORT_LEARN_DISABLE))
ksz_pwrite8(dev, index, P_STP_CTRL, learn[index]);
}
+ kfree(learn);
}
static int ksz8795_port_vlan_filtering(struct dsa_switch *ds, int port,
--
2.28.0
^ permalink raw reply related
* Re: [PATCH bpf-next 1/2] bpf: change uapi for bpf iterator map elements
From: Andrii Nakryiko @ 2020-08-03 5:11 UTC (permalink / raw)
To: Yonghong Song
Cc: bpf, Networking, Alexei Starovoitov, Daniel Borkmann, Kernel Team
In-Reply-To: <bb01225b-d4a4-c76b-5e1f-3dc37135f637@fb.com>
On Sun, Aug 2, 2020 at 7:23 PM Yonghong Song <yhs@fb.com> wrote:
>
>
>
> On 8/2/20 6:25 PM, Andrii Nakryiko wrote:
> > On Sat, Aug 1, 2020 at 9:22 PM Yonghong Song <yhs@fb.com> wrote:
> >>
> >> Commit a5cbe05a6673 ("bpf: Implement bpf iterator for
> >> map elements") added bpf iterator support for
> >> map elements. The map element bpf iterator requires
> >> info to identify a particular map. In the above
> >> commit, the attr->link_create.target_fd is used
> >> to carry map_fd and an enum bpf_iter_link_info
> >> is added to uapi to specify the target_fd actually
> >> representing a map_fd:
> >> enum bpf_iter_link_info {
> >> BPF_ITER_LINK_UNSPEC = 0,
> >> BPF_ITER_LINK_MAP_FD = 1,
> >>
> >> MAX_BPF_ITER_LINK_INFO,
> >> };
> >>
> >> This is an extensible approach as we can grow
> >> enumerator for pid, cgroup_id, etc. and we can
> >> unionize target_fd for pid, cgroup_id, etc.
> >> But in the future, there are chances that
> >> more complex customization may happen, e.g.,
> >> for tasks, it could be filtered based on
> >> both cgroup_id and user_id.
> >>
> >> This patch changed the uapi to have fields
> >> __aligned_u64 iter_info;
> >> __u32 iter_info_len;
> >> for additional iter_info for link_create.
> >> The iter_info is defined as
> >> union bpf_iter_link_info {
> >> struct {
> >> __u32 map_fd;
> >> } map;
> >> };
> >>
> >> So future extension for additional customization
> >> will be easier. The bpf_iter_link_info will be
> >> passed to target callback to validate and generic
> >> bpf_iter framework does not need to deal it any
> >> more.
> >>
> >> Signed-off-by: Yonghong Song <yhs@fb.com>
> >> ---
> >> include/linux/bpf.h | 10 ++++---
> >> include/uapi/linux/bpf.h | 15 +++++-----
> >> kernel/bpf/bpf_iter.c | 52 +++++++++++++++-------------------
> >> kernel/bpf/map_iter.c | 37 ++++++++++++++++++------
> >> kernel/bpf/syscall.c | 2 +-
> >> net/core/bpf_sk_storage.c | 37 ++++++++++++++++++------
> >> tools/include/uapi/linux/bpf.h | 15 +++++-----
> >> 7 files changed, 104 insertions(+), 64 deletions(-)
> >>
> >
> > [...]
> >
> >> int bpf_iter_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
> >> {
> >> + union bpf_iter_link_info __user *ulinfo;
> >> struct bpf_link_primer link_primer;
> >> struct bpf_iter_target_info *tinfo;
> >> - struct bpf_iter_aux_info aux = {};
> >> + union bpf_iter_link_info linfo;
> >> struct bpf_iter_link *link;
> >> - u32 prog_btf_id, target_fd;
> >> + u32 prog_btf_id, linfo_len;
> >> bool existed = false;
> >> - struct bpf_map *map;
> >> int err;
> >>
> >> + memset(&linfo, 0, sizeof(union bpf_iter_link_info));
> >> +
> >> + ulinfo = u64_to_user_ptr(attr->link_create.iter_info);
> >> + linfo_len = attr->link_create.iter_info_len;
> >> + if (ulinfo && linfo_len) {
> >
> > We probably want to be more strict here: if either pointer or len is
> > non-zero, both should be present and valid. Otherwise we can have
> > garbage in iter_info, as long as iter_info_len is zero.
>
> yes, it is possible iter_info_len = 0 and iter_info is not null and
> if this happens, iter_info will not be examined.
>
> in kernel, we have places this is handled similarly. For example,
> for cgroup bpf_prog query.
>
> kernel/bpf/cgroup.c, function __cgroup_bpf_query
>
> __u32 __user *prog_ids = u64_to_user_ptr(attr->query.prog_ids);
> ...
> if (attr->query.prog_cnt == 0 || !prog_ids || !cnt)
> return 0;
>
> In the above case, it is possible prog_cnt = 0 and prog_ids != NULL,
> or prog_ids == NULL and prog_cnt != 0, and we won't return error
> to user space.
>
> Not 100% sure whether we have convention here or not.
I don't know either, but I'd assume that we didn't think about 100%
strictness when originally implementing this. So I'd go with a very
strict check for this new functionality.
>
> >
> >> + err = bpf_check_uarg_tail_zero(ulinfo, sizeof(linfo),
> >> + linfo_len);
> >> + if (err)
> >> + return err;
> >> + linfo_len = min_t(u32, linfo_len, sizeof(linfo));
> >> + if (copy_from_user(&linfo, ulinfo, linfo_len))
> >> + return -EFAULT;
> >> + }
> >> +
> >> prog_btf_id = prog->aux->attach_btf_id;
> >> mutex_lock(&targets_mutex);
> >> list_for_each_entry(tinfo, &targets, list) {
> >> @@ -411,13 +425,6 @@ int bpf_iter_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
> >> if (!existed)
> >> return -ENOENT;
> >>
> >> - /* Make sure user supplied flags are target expected. */
> >> - target_fd = attr->link_create.target_fd;
> >> - if (attr->link_create.flags != tinfo->reg_info->req_linfo)
> >> - return -EINVAL;
> >> - if (!attr->link_create.flags && target_fd)
> >> - return -EINVAL;
> >> -
> >
> > Please still ensure that no flags are specified.
>
> Make sense. I also need to ensure target_fd is 0 since it is not used
> any more.
>
yep, good catch
> >
> >
> >> link = kzalloc(sizeof(*link), GFP_USER | __GFP_NOWARN);
> >> if (!link)
> >> return -ENOMEM;
> >> @@ -431,28 +438,15 @@ int bpf_iter_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
> >> return err;
> >> }
> >>
> >
> > [...]
> >
> >> -static int bpf_iter_check_map(struct bpf_prog *prog,
> >> - struct bpf_iter_aux_info *aux)
> >> +static int bpf_iter_attach_map(struct bpf_prog *prog,
> >> + union bpf_iter_link_info *linfo,
> >> + struct bpf_iter_aux_info *aux)
> >> {
> >> - struct bpf_map *map = aux->map;
> >> + struct bpf_map *map;
> >> + int err = -EINVAL;
> >>
> >> - if (map->map_type != BPF_MAP_TYPE_SK_STORAGE)
> >> + if (!linfo->map.map_fd)
> >> return -EINVAL;
> >
> > This could be -EBADF?
>
> Good suggestion. Will do.
>
> >
> >>
> >> - if (prog->aux->max_rdonly_access > map->value_size)
> >> - return -EACCES;
> >> + map = bpf_map_get_with_uref(linfo->map.map_fd);
> >> + if (IS_ERR(map))
> >> + return PTR_ERR(map);
> >> +
> >> + if (map->map_type != BPF_MAP_TYPE_SK_STORAGE)
> >> + goto put_map;
> >> +
> >> + if (prog->aux->max_rdonly_access > map->value_size) {
> >> + err = -EACCES;
> >> + goto put_map;
> >> + }
> >
> > [...]
> >
^ permalink raw reply
* Re: [PATCH bpf-next 5/5] selftests/bpf: add benchmark for uprobe vs. user_prog
From: Andrii Nakryiko @ 2020-08-03 5:10 UTC (permalink / raw)
To: Song Liu
Cc: open list, bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Kernel Team, john fastabend, KP Singh, Jesper Dangaard Brouer,
Daniel Xu
In-Reply-To: <DDCD362E-21D3-46BF-90A6-8F3221CBB54E@fb.com>
On Sun, Aug 2, 2020 at 9:47 PM Song Liu <songliubraving@fb.com> wrote:
>
>
> > On Aug 2, 2020, at 6:51 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > On Sat, Aug 1, 2020 at 1:50 AM Song Liu <songliubraving@fb.com> wrote:
> >>
> >> Add a benchmark to compare performance of
> >> 1) uprobe;
> >> 2) user program w/o args;
> >> 3) user program w/ args;
> >> 4) user program w/ args on random cpu.
> >>
> >
> > Can you please add it to the existing benchmark runner instead, e.g.,
> > along the other bench_trigger benchmarks? No need to re-implement
> > benchmark setup. And also that would also allow to compare existing
> > ways of cheaply triggering a program vs this new _USER program?
>
> Will try.
>
> >
> > If the performance is not significantly better than other ways, do you
> > think it still makes sense to add a new BPF program type? I think
> > triggering KPROBE/TRACEPOINT from bpf_prog_test_run() would be very
> > nice, maybe it's possible to add that instead of a new program type?
> > Either way, let's see comparison with other program triggering
> > mechanisms first.
>
> Triggering KPROBE and TRACEPOINT from bpf_prog_test_run() will be useful.
> But I don't think they can be used instead of user program, for a couple
> reasons. First, KPROBE/TRACEPOINT may be triggered by other programs
> running in the system, so user will have to filter those noise out in
> each program. Second, it is not easy to specify CPU for KPROBE/TRACEPOINT,
> while this feature could be useful in many cases, e.g. get stack trace
> on a given CPU.
>
Right, it's not as convenient with KPROBE/TRACEPOINT as with the USER
program you've added specifically with that feature in mind. But if
you pin user-space thread on the needed CPU and trigger kprobe/tp,
then you'll get what you want. As for the "noise", see how
bench_trigger() deals with that: it records thread ID and filters
everything not matching. You can do the same with CPU ID. It's not as
automatic as with a special BPF program type, but still pretty simple,
which is why I'm still deciding (for myself) whether USER program type
is necessary :)
> Thanks,
> Song
^ permalink raw reply
* Re: [PATCH bpf-next 3/5] selftests/bpf: add selftest for BPF_PROG_TYPE_USER
From: Andrii Nakryiko @ 2020-08-03 5:07 UTC (permalink / raw)
To: Song Liu
Cc: open list, bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Kernel Team, john fastabend, KP Singh, Jesper Dangaard Brouer,
Daniel Xu
In-Reply-To: <C5606E9B-D3EC-4425-82F5-DA5865836D3E@fb.com>
On Sun, Aug 2, 2020 at 9:33 PM Song Liu <songliubraving@fb.com> wrote:
>
>
>
> > On Aug 2, 2020, at 6:43 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > On Sat, Aug 1, 2020 at 1:50 AM Song Liu <songliubraving@fb.com> wrote:
> >>
> >> This test checks the correctness of BPF_PROG_TYPE_USER program, including:
> >> running on the right cpu, passing in correct args, returning retval, and
> >> being able to call bpf_get_stack|stackid.
> >>
> >> Signed-off-by: Song Liu <songliubraving@fb.com>
> >> ---
> >> .../selftests/bpf/prog_tests/user_prog.c | 52 +++++++++++++++++
> >> tools/testing/selftests/bpf/progs/user_prog.c | 56 +++++++++++++++++++
> >> 2 files changed, 108 insertions(+)
> >> create mode 100644 tools/testing/selftests/bpf/prog_tests/user_prog.c
> >> create mode 100644 tools/testing/selftests/bpf/progs/user_prog.c
> >>
> >> diff --git a/tools/testing/selftests/bpf/prog_tests/user_prog.c b/tools/testing/selftests/bpf/prog_tests/user_prog.c
> >> new file mode 100644
> >> index 0000000000000..416707b3bff01
> >> --- /dev/null
> >> +++ b/tools/testing/selftests/bpf/prog_tests/user_prog.c
> >> @@ -0,0 +1,52 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +/* Copyright (c) 2020 Facebook */
> >> +#include <test_progs.h>
> >> +#include "user_prog.skel.h"
> >> +
> >> +static int duration;
> >> +
> >> +void test_user_prog(void)
> >> +{
> >> + struct bpf_user_prog_args args = {{0, 1, 2, 3, 4}};
> >> + struct bpf_prog_test_run_attr attr = {};
> >> + struct user_prog *skel;
> >> + int i, numcpu, ret;
> >> +
> >> + skel = user_prog__open_and_load();
> >> +
> >> + if (CHECK(!skel, "user_prog__open_and_load",
> >> + "skeleton open_and_laod failed\n"))
> >> + return;
> >> +
> >> + numcpu = libbpf_num_possible_cpus();
> >
> > nit: possible doesn't mean online right now, so it will fail on
> > offline or non-present CPUs
>
> Just found parse_cpu_mask_file(), will use it to fix this.
>
> [...]
>
> >> +
> >> +volatile int cpu_match = 1;
> >> +volatile __u64 sum = 1;
> >> +volatile int get_stack_success = 0;
> >> +volatile int get_stackid_success = 0;
> >> +volatile __u64 stacktrace[PERF_MAX_STACK_DEPTH];
> >
> > nit: no need for volatile for non-static variables
> >
> >> +
> >> +SEC("user")
> >> +int user_func(struct bpf_user_prog_ctx *ctx)
> >
> > If you put args in bpf_user_prog_ctx as a first field, you should be
> > able to re-use the BPF_PROG macro to access those arguments in a more
> > user-friendly way.
>
> I am not sure I am following here. Do you mean something like:
>
> struct bpf_user_prog_ctx {
> __u64 args[BPF_USER_PROG_MAX_ARGS];
> struct pt_regs *regs;
> };
>
> (swap args and regs)?
>
Yes, BPF_PROG assumes that context is a plain u64[5] array, so if you
put args at the beginning, it will work nicely with BPF_PROG.
> Thanks,
> Song
>
>
^ permalink raw reply
* Re: BUG: unable to handle kernel NULL pointer dereference in bpf_prog_ADDR
From: John Fastabend @ 2020-08-03 5:06 UTC (permalink / raw)
To: Eric Dumazet, syzbot, andriin, ast, bpf, daniel, davem, hawk,
john.fastabend, kafai, kpsingh, kuba, linux-kernel, netdev,
songliubraving, syzkaller-bugs, yhs
In-Reply-To: <618c205e-c92f-1c3a-94a3-00793f0ad279@gmail.com>
Eric Dumazet wrote:
>
>
> On 8/2/20 3:45 PM, syzbot wrote:
> > Hello,
> >
> > syzbot found the following issue on:
> >
> > HEAD commit: ac3a0c84 Merge git://git.kernel.org/pub/scm/linux/kernel/g..
> > git tree: upstream
> > console output: https://syzkaller.appspot.com/x/log.txt?x=13234970900000
> > kernel config: https://syzkaller.appspot.com/x/.config?x=c0cfcf935bcc94d2
> > dashboard link: https://syzkaller.appspot.com/bug?extid=192a7fbbece55f740074
> > compiler: gcc (GCC) 10.1.0-syz 20200507
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=141541ea900000
> >
> > IMPORTANT: if you fix the issue, please add the following tag to the commit:
> > Reported-by: syzbot+192a7fbbece55f740074@syzkaller.appspotmail.com
> >
> > BUG: kernel NULL pointer dereference, address: 0000000000000000
> > #PF: supervisor read access in kernel mode
> > #PF: error_code(0x0000) - not-present page
> > PGD 9176a067 P4D 9176a067 PUD 9176b067 PMD 0
> > Oops: 0000 [#1] PREEMPT SMP KASAN
> > CPU: 1 PID: 8142 Comm: syz-executor.2 Not tainted 5.8.0-rc7-syzkaller #0
> > Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
> > RIP: 0010:bpf_prog_e48ebe87b99394c4+0x1f/0x590
> > Code: cc cc cc cc cc cc cc cc cc cc cc 0f 1f 44 00 00 55 48 89 e5 48 81 ec 00 00 00 00 53 41 55 41 56 41 57 6a 00 31 c0 48 8b 47 28 <48> 8b 40 00 8b 80 00 01 00 00 5b 41 5f 41 5e 41 5d 5b c9 c3 cc cc
> > RSP: 0018:ffffc900038a7b00 EFLAGS: 00010246
> > RAX: 0000000000000000 RBX: dffffc0000000000 RCX: dffffc0000000000
> > RDX: ffff88808cfb0200 RSI: ffffc90000e7e038 RDI: ffffc900038a7ca8
> > RBP: ffffc900038a7b28 R08: 0000000000000000 R09: 0000000000000000
> > R10: 0000000000000000 R11: 0000000000000000 R12: ffffc90000e7e000
> > R13: ffffc90000e7e000 R14: 0000000000000001 R15: 0000000000000000
> > FS: 00007fda07fef700(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000000000000000 CR3: 0000000091769000 CR4: 00000000001406e0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> > Call Trace:
> > bpf_prog_run_xdp include/linux/filter.h:734 [inline]
> > bpf_test_run+0x221/0xc70 net/bpf/test_run.c:47
> > bpf_prog_test_run_xdp+0x2ca/0x510 net/bpf/test_run.c:524
> > bpf_prog_test_run kernel/bpf/syscall.c:2983 [inline]
> > __do_sys_bpf+0x2117/0x4b10 kernel/bpf/syscall.c:4135
> > do_syscall_64+0x60/0xe0 arch/x86/entry/common.c:384
> > entry_SYSCALL_64_after_hwframe+0x44/0xa9
> > RIP: 0033:0x45cc79
> > Code: 2d b6 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 0f 83 fb b5 fb ff c3 66 2e 0f 1f 84 00 00 00 00
> > RSP: 002b:00007fda07feec78 EFLAGS: 00000246 ORIG_RAX: 0000000000000141
> > RAX: ffffffffffffffda RBX: 0000000000001740 RCX: 000000000045cc79
> > RDX: 0000000000000028 RSI: 0000000020000080 RDI: 000000000000000a
> > RBP: 000000000078bfe0 R08: 0000000000000000 R09: 0000000000000000
> > R10: 0000000000000000 R11: 0000000000000246 R12: 000000000078bfac
> > R13: 00007ffc3ef769bf R14: 00007fda07fef9c0 R15: 000000000078bfac
> > Modules linked in:
> > CR2: 0000000000000000
> > ---[ end trace b2d24107e7fdae7d ]---
> > RIP: 0010:bpf_prog_e48ebe87b99394c4+0x1f/0x590
> > Code: cc cc cc cc cc cc cc cc cc cc cc 0f 1f 44 00 00 55 48 89 e5 48 81 ec 00 00 00 00 53 41 55 41 56 41 57 6a 00 31 c0 48 8b 47 28 <48> 8b 40 00 8b 80 00 01 00 00 5b 41 5f 41 5e 41 5d 5b c9 c3 cc cc
> > RSP: 0018:ffffc900038a7b00 EFLAGS: 00010246
> > RAX: 0000000000000000 RBX: dffffc0000000000 RCX: dffffc0000000000
> > RDX: ffff88808cfb0200 RSI: ffffc90000e7e038 RDI: ffffc900038a7ca8
> > RBP: ffffc900038a7b28 R08: 0000000000000000 R09: 0000000000000000
> > R10: 0000000000000000 R11: 0000000000000000 R12: ffffc90000e7e000
> > R13: ffffc90000e7e000 R14: 0000000000000001 R15: 0000000000000000
> > FS: 00007fda07fef700(0000) GS:ffff8880ae700000(0000) knlGS:0000000000000000
> > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> > CR2: 0000000000000000 CR3: 0000000091769000 CR4: 00000000001406e0
> > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> >
> >
> > ---
> > This report is generated by a bot. It may contain errors.
> > See https://goo.gl/tpsmEJ for more information about syzbot.
> > syzbot engineers can be reached at syzkaller@googlegroups.com.
> >
> > syzbot will keep track of this issue. See:
> > https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
> > syzbot can test patches for this issue, for details see:
> > https://goo.gl/tpsmEJ#testing-patches
> >
>
>
> # https://syzkaller.appspot.com/bug?id=d60883a0b19a778d2bcab55f3f6459467f4a3ea7
> # See https://goo.gl/kgGztJ for information about syzkaller reproducers.
> #{"threaded":true,"collide":true,"repeat":true,"procs":6,"sandbox":"none","fault_call":-1,"tun":true,"netdev":true,"resetnet":true,"cgroups":true,"binfmt_misc":true,"close_fds":true,"vhci":true,"tmpdir":true,"segv":true}
> bpf$PROG_LOAD(0x5, &(0x7f00000ba000)={0x6, 0x4, &(0x7f0000346fc8)=@framed={{}, [@alu={0x8000000201a7f19, 0x0, 0x201a7fa6, 0x0, 0x1, 0x14}]}, &(0x7f0000000040)='syzkaller\x00', 0x1, 0xfb, &(0x7f0000002880)=""/251, 0x0, 0x0, [], 0x0, 0x21}, 0x48)
> r0 = bpf$PROG_LOAD(0x5, &(0x7f00002a0fb8)={0x13, 0x4, &(0x7f0000000480)=ANY=[@ANYBLOB="8500000011000000350000000000000085000000230000009500000d000000003c8ea5932cf669ebecab19b3fd50fec5eade4bb02a231016bc5733a4f152b8bdfdfebcfdaf3d5363dd79d50034e58579eda0cfe296"], &(0x7f0000000140)='GPL\x00', 0x4, 0x99, &(0x7f0000000180)=""/153, 0x0, 0x0, [], 0x0, 0x0, 0xffffffffffffffff, 0x8, 0x0, 0x0, 0x10, 0x0, 0xfffffffffffffd0c}, 0x64)
> bpf$BPF_PROG_TEST_RUN(0xa, &(0x7f0000000080)={r0, 0x1000000, 0xe, 0x0, &(0x7f00000000c0)="61df712bc884fed5722780b6c2a7", 0x0, 0x8000, 0x0, 0xff9f}, 0x28)
>
>
> Not clear how a BPF_PROG_TYPE_LWT_SEG6LOCAL ends up using bpf_prog_test_run_xdp() ?
>
>
Strange... I'll take a look in the morning if no one beats me to it.
^ permalink raw reply
* Re: [PATCH bpf-next 2/5] libbpf: support BPF_PROG_TYPE_USER programs
From: Andrii Nakryiko @ 2020-08-03 5:05 UTC (permalink / raw)
To: Song Liu
Cc: open list, bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Kernel Team, john fastabend, KP Singh, Jesper Dangaard Brouer,
Daniel Xu
In-Reply-To: <3B31DE6E-B128-48D7-91A9-84D51BDF205B@fb.com>
On Sun, Aug 2, 2020 at 9:21 PM Song Liu <songliubraving@fb.com> wrote:
>
>
>
> > On Aug 2, 2020, at 6:40 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
> >
> > On Sat, Aug 1, 2020 at 1:50 AM Song Liu <songliubraving@fb.com> wrote:
> >>
> >> Add cpu_plus to bpf_prog_test_run_attr. Add BPF_PROG_SEC "user" for
> >> BPF_PROG_TYPE_USER programs.
> >>
> >> Signed-off-by: Song Liu <songliubraving@fb.com>
> >> ---
> >> tools/lib/bpf/bpf.c | 1 +
> >> tools/lib/bpf/bpf.h | 3 +++
> >> tools/lib/bpf/libbpf.c | 1 +
> >> tools/lib/bpf/libbpf_probes.c | 1 +
> >> 4 files changed, 6 insertions(+)
> >>
> >> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
> >> index e1bdf214f75fe..b28c3daa9c270 100644
> >> --- a/tools/lib/bpf/bpf.c
> >> +++ b/tools/lib/bpf/bpf.c
> >> @@ -693,6 +693,7 @@ int bpf_prog_test_run_xattr(struct bpf_prog_test_run_attr *test_attr)
> >> attr.test.ctx_size_in = test_attr->ctx_size_in;
> >> attr.test.ctx_size_out = test_attr->ctx_size_out;
> >> attr.test.repeat = test_attr->repeat;
> >> + attr.test.cpu_plus = test_attr->cpu_plus;
> >>
> >> ret = sys_bpf(BPF_PROG_TEST_RUN, &attr, sizeof(attr));
> >> test_attr->data_size_out = attr.test.data_size_out;
> >> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
> >> index 6d367e01d05e9..0c799740df566 100644
> >> --- a/tools/lib/bpf/bpf.h
> >> +++ b/tools/lib/bpf/bpf.h
> >> @@ -205,6 +205,9 @@ struct bpf_prog_test_run_attr {
> >> void *ctx_out; /* optional */
> >> __u32 ctx_size_out; /* in: max length of ctx_out
> >> * out: length of cxt_out */
> >> + __u32 cpu_plus; /* specify which cpu to run the test with
> >> + * cpu_plus = cpu_id + 1.
> >> + * If cpu_plus = 0, run on current cpu */
> >
> > We can't do this due to ABI guarantees. We'll have to add a new API
> > using OPTS arguments.
>
> To make sure I understand this correctly, the concern is when we compile
> the binary with one version of libbpf and run it with libbpf.so of a
> different version, right?
>
yep, exactly
> Thanks,
> Song
>
> >
> >> };
> >>
> >> LIBBPF_API int bpf_prog_test_run_xattr(struct bpf_prog_test_run_attr *test_attr);
> >> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> >> index b9f11f854985b..9ce175a486214 100644
> >> --- a/tools/lib/bpf/libbpf.c
> >> +++ b/tools/lib/bpf/libbpf.c
> >> @@ -6922,6 +6922,7 @@ static const struct bpf_sec_def section_defs[] = {
> >> BPF_PROG_SEC("lwt_out", BPF_PROG_TYPE_LWT_OUT),
> >> BPF_PROG_SEC("lwt_xmit", BPF_PROG_TYPE_LWT_XMIT),
> >> BPF_PROG_SEC("lwt_seg6local", BPF_PROG_TYPE_LWT_SEG6LOCAL),
> >> + BPF_PROG_SEC("user", BPF_PROG_TYPE_USER),
> >
> > let's do "user/" for consistency with most other prog types (and nice
> > separation between prog type and custom user name)
>
> Will update.
>
thanks!
^ permalink raw reply
* KASAN: use-after-free Write in sco_chan_del
From: syzbot @ 2020-08-03 5:05 UTC (permalink / raw)
To: davem, johan.hedberg, kuba, linux-bluetooth, linux-kernel, marcel,
netdev, syzkaller-bugs
Hello,
syzbot found the following issue on:
HEAD commit: ac3a0c84 Merge git://git.kernel.org/pub/scm/linux/kernel/g..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=11638a42900000
kernel config: https://syzkaller.appspot.com/x/.config?x=c0cfcf935bcc94d2
dashboard link: https://syzkaller.appspot.com/bug?extid=8f6017ee5c7fb9515782
compiler: gcc (GCC) 10.1.0-syz 20200507
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=17fd776c900000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=15ac7014900000
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+8f6017ee5c7fb9515782@syzkaller.appspotmail.com
==================================================================
BUG: KASAN: use-after-free in instrument_atomic_write include/linux/instrumented.h:71 [inline]
BUG: KASAN: use-after-free in atomic_dec_and_test include/asm-generic/atomic-instrumented.h:748 [inline]
BUG: KASAN: use-after-free in hci_conn_drop include/net/bluetooth/hci_core.h:1049 [inline]
BUG: KASAN: use-after-free in sco_chan_del+0xe6/0x430 net/bluetooth/sco.c:148
Write of size 4 at addr ffff8880a03d8010 by task syz-executor104/6978
CPU: 1 PID: 6978 Comm: syz-executor104 Not tainted 5.8.0-rc7-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x18f/0x20d lib/dump_stack.c:118
print_address_description.constprop.0.cold+0xae/0x436 mm/kasan/report.c:383
__kasan_report mm/kasan/report.c:513 [inline]
kasan_report.cold+0x1f/0x37 mm/kasan/report.c:530
check_memory_region_inline mm/kasan/generic.c:186 [inline]
check_memory_region+0x13d/0x180 mm/kasan/generic.c:192
instrument_atomic_write include/linux/instrumented.h:71 [inline]
atomic_dec_and_test include/asm-generic/atomic-instrumented.h:748 [inline]
hci_conn_drop include/net/bluetooth/hci_core.h:1049 [inline]
sco_chan_del+0xe6/0x430 net/bluetooth/sco.c:148
__sco_sock_close+0x16e/0x5b0 net/bluetooth/sco.c:433
sco_sock_close net/bluetooth/sco.c:447 [inline]
sco_sock_release+0x69/0x290 net/bluetooth/sco.c:1021
__sock_release+0xcd/0x280 net/socket.c:605
sock_close+0x18/0x20 net/socket.c:1278
__fput+0x33c/0x880 fs/file_table.c:281
task_work_run+0xdd/0x190 kernel/task_work.c:135
exit_task_work include/linux/task_work.h:25 [inline]
do_exit+0xb72/0x2a40 kernel/exit.c:805
do_group_exit+0x125/0x310 kernel/exit.c:903
get_signal+0x40b/0x1ee0 kernel/signal.c:2743
do_signal+0x82/0x2520 arch/x86/kernel/signal.c:810
exit_to_usermode_loop arch/x86/entry/common.c:235 [inline]
__prepare_exit_to_usermode+0x156/0x1f0 arch/x86/entry/common.c:269
do_syscall_64+0x6c/0xe0 arch/x86/entry/common.c:393
entry_SYSCALL_64_after_hwframe+0x44/0xa9
RIP: 0033:0x446e69
Code: Bad RIP value.
RSP: 002b:00007fff15a45008 EFLAGS: 00000246 ORIG_RAX: 000000000000002a
RAX: fffffffffffffffc RBX: 0000000000000000 RCX: 0000000000446e69
RDX: 0000000000000008 RSI: 0000000020000000 RDI: 0000000000000004
RBP: 0000000000000004 R08: 0000000000000002 R09: 00000003000100ff
R10: 0000000000000004 R11: 0000000000000246 R12: 0000000000000000
R13: 0000000000407ac0 R14: 0000000000000000 R15: 0000000000000000
Allocated by task 6978:
save_stack+0x1b/0x40 mm/kasan/common.c:48
set_track mm/kasan/common.c:56 [inline]
__kasan_kmalloc.constprop.0+0xc2/0xd0 mm/kasan/common.c:494
kmem_cache_alloc_trace+0x14f/0x2d0 mm/slab.c:3551
kmalloc include/linux/slab.h:555 [inline]
kzalloc include/linux/slab.h:669 [inline]
hci_conn_add+0x53/0x1340 net/bluetooth/hci_conn.c:525
hci_connect_sco+0x350/0x860 net/bluetooth/hci_conn.c:1279
sco_connect net/bluetooth/sco.c:240 [inline]
sco_sock_connect+0x308/0x980 net/bluetooth/sco.c:576
__sys_connect_file+0x155/0x1a0 net/socket.c:1854
__sys_connect+0x160/0x190 net/socket.c:1871
__do_sys_connect net/socket.c:1882 [inline]
__se_sys_connect net/socket.c:1879 [inline]
__x64_sys_connect+0x6f/0xb0 net/socket.c:1879
do_syscall_64+0x60/0xe0 arch/x86/entry/common.c:384
entry_SYSCALL_64_after_hwframe+0x44/0xa9
Freed by task 6972:
save_stack+0x1b/0x40 mm/kasan/common.c:48
set_track mm/kasan/common.c:56 [inline]
kasan_set_free_info mm/kasan/common.c:316 [inline]
__kasan_slab_free+0xf5/0x140 mm/kasan/common.c:455
__cache_free mm/slab.c:3426 [inline]
kfree+0x103/0x2c0 mm/slab.c:3757
device_release+0x71/0x200 drivers/base/core.c:1579
kobject_cleanup lib/kobject.c:693 [inline]
kobject_release lib/kobject.c:722 [inline]
kref_put include/linux/kref.h:65 [inline]
kobject_put+0x1c0/0x270 lib/kobject.c:739
put_device+0x1b/0x30 drivers/base/core.c:2799
hci_conn_del+0x27e/0x6a0 net/bluetooth/hci_conn.c:645
hci_phy_link_complete_evt.isra.0+0x508/0x790 net/bluetooth/hci_event.c:4921
hci_event_packet+0x481a/0x86f5 net/bluetooth/hci_event.c:6180
hci_rx_work+0x22e/0xb10 net/bluetooth/hci_core.c:4705
process_one_work+0x94c/0x1670 kernel/workqueue.c:2269
worker_thread+0x64c/0x1120 kernel/workqueue.c:2415
kthread+0x3b5/0x4a0 kernel/kthread.c:291
ret_from_fork+0x1f/0x30 arch/x86/entry/entry_64.S:293
The buggy address belongs to the object at ffff8880a03d8000
which belongs to the cache kmalloc-4k of size 4096
The buggy address is located 16 bytes inside of
4096-byte region [ffff8880a03d8000, ffff8880a03d9000)
The buggy address belongs to the page:
page:ffffea000280f600 refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 head:ffffea000280f600 order:1 compound_mapcount:0
flags: 0xfffe0000010200(slab|head)
raw: 00fffe0000010200 ffffea0002882888 ffffea00027ede08 ffff8880aa002000
raw: 0000000000000000 ffff8880a03d8000 0000000100000001 0000000000000000
page dumped because: kasan: bad access detected
Memory state around the buggy address:
ffff8880a03d7f00: 00 00 00 00 00 00 00 00 00 00 00 00 00 fc fc fc
ffff8880a03d7f80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
>ffff8880a03d8000: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff8880a03d8080: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff8880a03d8100: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
syzbot can test patches for this issue, for details see:
https://goo.gl/tpsmEJ#testing-patches
^ permalink raw reply
* Re: [PATCH] tools/bpf/bpftool: Fix wrong return value in do_dump()
From: John Fastabend @ 2020-08-03 5:04 UTC (permalink / raw)
To: Andrii Nakryiko, Tianjia Zhang
Cc: Alexei Starovoitov, Daniel Borkmann, Martin Lau, Song Liu,
Yonghong Song, Andrii Nakryiko, john fastabend, KP Singh,
Quentin Monnet, Jakub Kicinski, Toke Høiland-Jørgensen,
Tobias Klauser, Jiri Olsa, Networking, bpf, open list,
tianjia.zhang
In-Reply-To: <CAEf4Bza0C3iB3S8wXkkQxPoE+ndNuUtkmU3L8g7NzMgjHzkx8Q@mail.gmail.com>
Andrii Nakryiko wrote:
> On Sun, Aug 2, 2020 at 4:16 AM Tianjia Zhang
> <tianjia.zhang@linux.alibaba.com> wrote:
> >
> > In case of btf_id does not exist, a negative error code -ENOENT
> > should be returned.
> >
> > Fixes: c93cc69004df3 ("bpftool: add ability to dump BTF types")
> > Cc: Andrii Nakryiko <andriin@fb.com>
> > Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> > ---
>
>
> Acked-by: Andrii Nakryiko <andriin@fb.com>
>
Acked-by: John Fastabend <john.fastabend@gmail.com>
^ permalink raw reply
* Re: [Linux-kernel-mentees] [PATCH net] rds: Prevent kernel-infoleak in rds_notify_queue_get()
From: Leon Romanovsky @ 2020-08-03 4:58 UTC (permalink / raw)
To: Joe Perches
Cc: Jason Gunthorpe, Greg Kroah-Hartman, Peilin Ye, Santosh Shilimkar,
David S. Miller, Jakub Kicinski, Dan Carpenter, Arnd Bergmann,
linux-kernel-mentees, netdev, linux-rdma, rds-devel, linux-kernel
In-Reply-To: <60584f4c0303106b42463ddcfb108ec4a1f0b705.camel@perches.com>
On Sun, Aug 02, 2020 at 03:45:40PM -0700, Joe Perches wrote:
> On Sun, 2020-08-02 at 19:28 -0300, Jason Gunthorpe wrote:
> > On Sun, Aug 02, 2020 at 03:23:58PM -0700, Joe Perches wrote:
> > > On Sun, 2020-08-02 at 19:10 -0300, Jason Gunthorpe wrote:
> > > > On Sat, Aug 01, 2020 at 08:38:33AM +0300, Leon Romanovsky wrote:
> > > >
> > > > > I'm using {} instead of {0} because of this GCC bug.
> > > > > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53119
> > > >
> > > > This is why the {} extension exists..
> > >
> > > There is no guarantee that the gcc struct initialization {}
> > > extension also zeros padding.
> >
> > We just went over this. Yes there is, C11 requires it.
>
> c11 is not c90. The kernel uses c90.
It is not accurate, kernel uses gnu89 dialect, which is C90 with some
C99 features [1]. In our case, we rely on GCC extension {} that doesn't
contradict standart [2] and fills holes with zeros too.
[1] Makefile:500
496 KBUILD_CFLAGS := -Wall -Wundef -Werror=strict-prototypes -Wno-trigraphs \
497 -fno-strict-aliasing -fno-common -fshort-wchar -fno-PIE \
498 -Werror=implicit-function-declaration -Werror=implicit-int \
499 -Wno-format-security \
500 -std=gnu89
[2] From GCC:
https://gcc.gnu.org/onlinedocs/gcc/C-Dialect-Options.html
"When a base standard is specified, the compiler accepts all programs
following that standard plus those using GNU extensions that do not
contradict it."
Thanks
>
>
>
^ permalink raw reply
* Re: [PATCH bpf-next 5/5] selftests/bpf: add benchmark for uprobe vs. user_prog
From: Song Liu @ 2020-08-03 4:47 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: open list, bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Kernel Team, john fastabend, KP Singh, Jesper Dangaard Brouer,
Daniel Xu
In-Reply-To: <CAEf4BzaP4TGF7kcmZRAKsy=oWPpFA6sUGFkctpGz-fPp+YuSOQ@mail.gmail.com>
> On Aug 2, 2020, at 6:51 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Sat, Aug 1, 2020 at 1:50 AM Song Liu <songliubraving@fb.com> wrote:
>>
>> Add a benchmark to compare performance of
>> 1) uprobe;
>> 2) user program w/o args;
>> 3) user program w/ args;
>> 4) user program w/ args on random cpu.
>>
>
> Can you please add it to the existing benchmark runner instead, e.g.,
> along the other bench_trigger benchmarks? No need to re-implement
> benchmark setup. And also that would also allow to compare existing
> ways of cheaply triggering a program vs this new _USER program?
Will try.
>
> If the performance is not significantly better than other ways, do you
> think it still makes sense to add a new BPF program type? I think
> triggering KPROBE/TRACEPOINT from bpf_prog_test_run() would be very
> nice, maybe it's possible to add that instead of a new program type?
> Either way, let's see comparison with other program triggering
> mechanisms first.
Triggering KPROBE and TRACEPOINT from bpf_prog_test_run() will be useful.
But I don't think they can be used instead of user program, for a couple
reasons. First, KPROBE/TRACEPOINT may be triggered by other programs
running in the system, so user will have to filter those noise out in
each program. Second, it is not easy to specify CPU for KPROBE/TRACEPOINT,
while this feature could be useful in many cases, e.g. get stack trace
on a given CPU.
Thanks,
Song
^ permalink raw reply
* Re: [PATCH bpf-next 4/5] selftests/bpf: move two functions to test_progs.c
From: Song Liu @ 2020-08-03 4:34 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: open list, bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Kernel Team, john fastabend, KP Singh, Jesper Dangaard Brouer,
Daniel Xu
In-Reply-To: <CAEf4BzY44oYFXRPeG1y3W96xrCR2muGpeKJ7XHQ-3EpaZ__Veg@mail.gmail.com>
> On Aug 2, 2020, at 6:46 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Sat, Aug 1, 2020 at 1:50 AM Song Liu <songliubraving@fb.com> wrote:
>>
>> Move time_get_ns() and get_base_addr() to test_progs.c, so they can be
>> used in other tests.
>>
>> Signed-off-by: Song Liu <songliubraving@fb.com>
>> ---
>> .../selftests/bpf/prog_tests/attach_probe.c | 21 -------------
>> .../selftests/bpf/prog_tests/test_overhead.c | 8 -----
>> tools/testing/selftests/bpf/test_progs.c | 30 +++++++++++++++++++
>> tools/testing/selftests/bpf/test_progs.h | 2 ++
>> 4 files changed, 32 insertions(+), 29 deletions(-)
>>
>
> [...]
>
>> static int test_task_rename(const char *prog)
>> {
>> int i, fd, duration = 0, err;
>> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
>> index b1e4dadacd9b4..c9e6a5ad5b9a4 100644
>> --- a/tools/testing/selftests/bpf/test_progs.c
>> +++ b/tools/testing/selftests/bpf/test_progs.c
>> @@ -622,6 +622,36 @@ int cd_flavor_subdir(const char *exec_name)
>> return chdir(flavor);
>> }
>>
>> +__u64 time_get_ns(void)
>> +{
>
> I'd try to avoid adding stuff to test_progs.c. There is generic
> testing_helpers.c, maybe let's put this there?
>
>> + struct timespec ts;
>> +
>> + clock_gettime(CLOCK_MONOTONIC, &ts);
>> + return ts.tv_sec * 1000000000ull + ts.tv_nsec;
>> +}
>> +
>> +ssize_t get_base_addr(void)
>> +{
>
> This would definitely be better in trace_helpers.c, though.
Will update.
Thanks,
Song
^ permalink raw reply
* Re: [PATCH bpf-next 3/5] selftests/bpf: add selftest for BPF_PROG_TYPE_USER
From: Song Liu @ 2020-08-03 4:33 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: open list, bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Kernel Team, john fastabend, KP Singh, Jesper Dangaard Brouer,
Daniel Xu
In-Reply-To: <CAEf4BzazkFMw3LAs3M2hxSLWWZJ7ywykwte=0WDhC1zgMYw-3A@mail.gmail.com>
> On Aug 2, 2020, at 6:43 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Sat, Aug 1, 2020 at 1:50 AM Song Liu <songliubraving@fb.com> wrote:
>>
>> This test checks the correctness of BPF_PROG_TYPE_USER program, including:
>> running on the right cpu, passing in correct args, returning retval, and
>> being able to call bpf_get_stack|stackid.
>>
>> Signed-off-by: Song Liu <songliubraving@fb.com>
>> ---
>> .../selftests/bpf/prog_tests/user_prog.c | 52 +++++++++++++++++
>> tools/testing/selftests/bpf/progs/user_prog.c | 56 +++++++++++++++++++
>> 2 files changed, 108 insertions(+)
>> create mode 100644 tools/testing/selftests/bpf/prog_tests/user_prog.c
>> create mode 100644 tools/testing/selftests/bpf/progs/user_prog.c
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/user_prog.c b/tools/testing/selftests/bpf/prog_tests/user_prog.c
>> new file mode 100644
>> index 0000000000000..416707b3bff01
>> --- /dev/null
>> +++ b/tools/testing/selftests/bpf/prog_tests/user_prog.c
>> @@ -0,0 +1,52 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +/* Copyright (c) 2020 Facebook */
>> +#include <test_progs.h>
>> +#include "user_prog.skel.h"
>> +
>> +static int duration;
>> +
>> +void test_user_prog(void)
>> +{
>> + struct bpf_user_prog_args args = {{0, 1, 2, 3, 4}};
>> + struct bpf_prog_test_run_attr attr = {};
>> + struct user_prog *skel;
>> + int i, numcpu, ret;
>> +
>> + skel = user_prog__open_and_load();
>> +
>> + if (CHECK(!skel, "user_prog__open_and_load",
>> + "skeleton open_and_laod failed\n"))
>> + return;
>> +
>> + numcpu = libbpf_num_possible_cpus();
>
> nit: possible doesn't mean online right now, so it will fail on
> offline or non-present CPUs
Just found parse_cpu_mask_file(), will use it to fix this.
[...]
>> +
>> +volatile int cpu_match = 1;
>> +volatile __u64 sum = 1;
>> +volatile int get_stack_success = 0;
>> +volatile int get_stackid_success = 0;
>> +volatile __u64 stacktrace[PERF_MAX_STACK_DEPTH];
>
> nit: no need for volatile for non-static variables
>
>> +
>> +SEC("user")
>> +int user_func(struct bpf_user_prog_ctx *ctx)
>
> If you put args in bpf_user_prog_ctx as a first field, you should be
> able to re-use the BPF_PROG macro to access those arguments in a more
> user-friendly way.
I am not sure I am following here. Do you mean something like:
struct bpf_user_prog_ctx {
__u64 args[BPF_USER_PROG_MAX_ARGS];
struct pt_regs *regs;
};
(swap args and regs)?
Thanks,
Song
^ permalink raw reply
* Re: [PATCH bpf-next 2/5] libbpf: support BPF_PROG_TYPE_USER programs
From: Song Liu @ 2020-08-03 4:21 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: open list, bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Kernel Team, john fastabend, KP Singh, Jesper Dangaard Brouer,
Daniel Xu
In-Reply-To: <CAEf4BzYp4gO1P+OrY7hGyQjdia3BuSu4DX2_z=UF6RfGNa+gkQ@mail.gmail.com>
> On Aug 2, 2020, at 6:40 PM, Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote:
>
> On Sat, Aug 1, 2020 at 1:50 AM Song Liu <songliubraving@fb.com> wrote:
>>
>> Add cpu_plus to bpf_prog_test_run_attr. Add BPF_PROG_SEC "user" for
>> BPF_PROG_TYPE_USER programs.
>>
>> Signed-off-by: Song Liu <songliubraving@fb.com>
>> ---
>> tools/lib/bpf/bpf.c | 1 +
>> tools/lib/bpf/bpf.h | 3 +++
>> tools/lib/bpf/libbpf.c | 1 +
>> tools/lib/bpf/libbpf_probes.c | 1 +
>> 4 files changed, 6 insertions(+)
>>
>> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
>> index e1bdf214f75fe..b28c3daa9c270 100644
>> --- a/tools/lib/bpf/bpf.c
>> +++ b/tools/lib/bpf/bpf.c
>> @@ -693,6 +693,7 @@ int bpf_prog_test_run_xattr(struct bpf_prog_test_run_attr *test_attr)
>> attr.test.ctx_size_in = test_attr->ctx_size_in;
>> attr.test.ctx_size_out = test_attr->ctx_size_out;
>> attr.test.repeat = test_attr->repeat;
>> + attr.test.cpu_plus = test_attr->cpu_plus;
>>
>> ret = sys_bpf(BPF_PROG_TEST_RUN, &attr, sizeof(attr));
>> test_attr->data_size_out = attr.test.data_size_out;
>> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
>> index 6d367e01d05e9..0c799740df566 100644
>> --- a/tools/lib/bpf/bpf.h
>> +++ b/tools/lib/bpf/bpf.h
>> @@ -205,6 +205,9 @@ struct bpf_prog_test_run_attr {
>> void *ctx_out; /* optional */
>> __u32 ctx_size_out; /* in: max length of ctx_out
>> * out: length of cxt_out */
>> + __u32 cpu_plus; /* specify which cpu to run the test with
>> + * cpu_plus = cpu_id + 1.
>> + * If cpu_plus = 0, run on current cpu */
>
> We can't do this due to ABI guarantees. We'll have to add a new API
> using OPTS arguments.
To make sure I understand this correctly, the concern is when we compile
the binary with one version of libbpf and run it with libbpf.so of a
different version, right?
Thanks,
Song
>
>> };
>>
>> LIBBPF_API int bpf_prog_test_run_xattr(struct bpf_prog_test_run_attr *test_attr);
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index b9f11f854985b..9ce175a486214 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -6922,6 +6922,7 @@ static const struct bpf_sec_def section_defs[] = {
>> BPF_PROG_SEC("lwt_out", BPF_PROG_TYPE_LWT_OUT),
>> BPF_PROG_SEC("lwt_xmit", BPF_PROG_TYPE_LWT_XMIT),
>> BPF_PROG_SEC("lwt_seg6local", BPF_PROG_TYPE_LWT_SEG6LOCAL),
>> + BPF_PROG_SEC("user", BPF_PROG_TYPE_USER),
>
> let's do "user/" for consistency with most other prog types (and nice
> separation between prog type and custom user name)
Will update.
^ permalink raw reply
* Re: Bug#966459: linux: traffic class socket options (both IPv4/IPv6) inconsistent with docs/standards
From: Ben Hutchings @ 2020-08-03 3:32 UTC (permalink / raw)
To: Thorsten Glaser, 966459; +Cc: netdev
In-Reply-To: <alpine.DEB.2.23.453.2008022243310.15898@tglase-nb.lan.tarent.de>
[-- Attachment #1: Type: text/plain, Size: 663 bytes --]
On Sun, 2020-08-02 at 22:44 +0200, Thorsten Glaser wrote:
> On Sun, 2 Aug 2020, Ben Hutchings wrote:
>
> > The RFC says that the IPV6_TCLASS option's value is an int, and that
>
> for setsockopt (“option's”), not cmsg
>
> > No, the wording is *not* clear.
>
> Agreed.
>
> So perhaps let’s try to find out what’s actually right…
For what it's worth, FreeBSD/Darwin and Windows also put 4 bytes of
data in a IPV6_TCLASS cmsg. So whether or not it's "right", it's
consistent between three independent implementations.
Ben.
--
Ben Hutchings
For every complex problem
there is a solution that is simple, neat, and wrong.
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* linux-next: manual merge of the bpf-next tree with the net-next tree
From: Stephen Rothwell @ 2020-08-03 3:05 UTC (permalink / raw)
To: Daniel Borkmann, Alexei Starovoitov, David Miller, Networking
Cc: Linux Next Mailing List, Linux Kernel Mailing List, Roopa Prabhu,
Andrii Nakryiko
[-- Attachment #1: Type: text/plain, Size: 4107 bytes --]
Hi all,
Today's linux-next merge of the bpf-next tree got a conflict in:
net/core/dev.c
between commit:
829eb208e80d ("rtnetlink: add support for protodown reason")
from the net-next tree and commits:
7f0a838254bd ("bpf, xdp: Maintain info on attached XDP BPF programs in net_device")
aa8d3a716b59 ("bpf, xdp: Add bpf_link-based XDP attachment API")
from the bpf-next tree.
I fixed it up (see below) and can carry the fix as necessary. This
is now fixed as far as linux-next is concerned, but any non trivial
conflicts should be mentioned to your upstream maintainer when your tree
is submitted for merging. You may also want to consider cooperating
with the maintainer of the conflicting tree to minimise any particularly
complex conflicts.
--
Cheers,
Stephen Rothwell
diff --cc net/core/dev.c
index f7ef0f5c5569,c8b911b10187..000000000000
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@@ -8715,54 -8712,75 +8711,100 @@@ int dev_change_proto_down_generic(struc
}
EXPORT_SYMBOL(dev_change_proto_down_generic);
+/**
+ * dev_change_proto_down_reason - proto down reason
+ *
+ * @dev: device
+ * @mask: proto down mask
+ * @value: proto down value
+ */
+void dev_change_proto_down_reason(struct net_device *dev, unsigned long mask,
+ u32 value)
+{
+ int b;
+
+ if (!mask) {
+ dev->proto_down_reason = value;
+ } else {
+ for_each_set_bit(b, &mask, 32) {
+ if (value & (1 << b))
+ dev->proto_down_reason |= BIT(b);
+ else
+ dev->proto_down_reason &= ~BIT(b);
+ }
+ }
+}
+EXPORT_SYMBOL(dev_change_proto_down_reason);
+
- u32 __dev_xdp_query(struct net_device *dev, bpf_op_t bpf_op,
- enum bpf_netdev_command cmd)
+ struct bpf_xdp_link {
+ struct bpf_link link;
+ struct net_device *dev; /* protected by rtnl_lock, no refcnt held */
+ int flags;
+ };
+
+ static enum bpf_xdp_mode dev_xdp_mode(u32 flags)
{
- struct netdev_bpf xdp;
+ if (flags & XDP_FLAGS_HW_MODE)
+ return XDP_MODE_HW;
+ if (flags & XDP_FLAGS_DRV_MODE)
+ return XDP_MODE_DRV;
+ return XDP_MODE_SKB;
+ }
- if (!bpf_op)
- return 0;
+ static bpf_op_t dev_xdp_bpf_op(struct net_device *dev, enum bpf_xdp_mode mode)
+ {
+ switch (mode) {
+ case XDP_MODE_SKB:
+ return generic_xdp_install;
+ case XDP_MODE_DRV:
+ case XDP_MODE_HW:
+ return dev->netdev_ops->ndo_bpf;
+ default:
+ return NULL;
+ };
+ }
- memset(&xdp, 0, sizeof(xdp));
- xdp.command = cmd;
+ static struct bpf_xdp_link *dev_xdp_link(struct net_device *dev,
+ enum bpf_xdp_mode mode)
+ {
+ return dev->xdp_state[mode].link;
+ }
+
+ static struct bpf_prog *dev_xdp_prog(struct net_device *dev,
+ enum bpf_xdp_mode mode)
+ {
+ struct bpf_xdp_link *link = dev_xdp_link(dev, mode);
+
+ if (link)
+ return link->link.prog;
+ return dev->xdp_state[mode].prog;
+ }
+
+ u32 dev_xdp_prog_id(struct net_device *dev, enum bpf_xdp_mode mode)
+ {
+ struct bpf_prog *prog = dev_xdp_prog(dev, mode);
- /* Query must always succeed. */
- WARN_ON(bpf_op(dev, &xdp) < 0 && cmd == XDP_QUERY_PROG);
+ return prog ? prog->aux->id : 0;
+ }
- return xdp.prog_id;
+ static void dev_xdp_set_link(struct net_device *dev, enum bpf_xdp_mode mode,
+ struct bpf_xdp_link *link)
+ {
+ dev->xdp_state[mode].link = link;
+ dev->xdp_state[mode].prog = NULL;
}
- static int dev_xdp_install(struct net_device *dev, bpf_op_t bpf_op,
- struct netlink_ext_ack *extack, u32 flags,
- struct bpf_prog *prog)
+ static void dev_xdp_set_prog(struct net_device *dev, enum bpf_xdp_mode mode,
+ struct bpf_prog *prog)
+ {
+ dev->xdp_state[mode].link = NULL;
+ dev->xdp_state[mode].prog = prog;
+ }
+
+ static int dev_xdp_install(struct net_device *dev, enum bpf_xdp_mode mode,
+ bpf_op_t bpf_op, struct netlink_ext_ack *extack,
+ u32 flags, struct bpf_prog *prog)
{
- bool non_hw = !(flags & XDP_FLAGS_HW_MODE);
- struct bpf_prog *prev_prog = NULL;
struct netdev_bpf xdp;
int err;
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH bpf-next 2/2] libbpf: support new uapi for map element bpf iterator
From: Yonghong Song @ 2020-08-03 2:30 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Networking, Alexei Starovoitov, Daniel Borkmann, Kernel Team
In-Reply-To: <CAEf4BzaeT1HULBE0dQULSF62Wm6=t49Dc8jjHVJ9Nt1noxeCtQ@mail.gmail.com>
On 8/2/20 6:35 PM, Andrii Nakryiko wrote:
> On Sat, Aug 1, 2020 at 9:22 PM Yonghong Song <yhs@fb.com> wrote:
>>
>> Previous commit adjusted kernel uapi for map
>> element bpf iterator. This patch adjusted libbpf API
>> due to uapi change.
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>> tools/lib/bpf/bpf.c | 4 +++-
>> tools/lib/bpf/bpf.h | 5 +++--
>> tools/lib/bpf/libbpf.c | 7 +++++--
>> 3 files changed, 11 insertions(+), 5 deletions(-)
>>
>> diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
>> index eab14c97c15d..c75a84398d51 100644
>> --- a/tools/lib/bpf/bpf.c
>> +++ b/tools/lib/bpf/bpf.c
>> @@ -598,7 +598,9 @@ int bpf_link_create(int prog_fd, int target_fd,
>> attr.link_create.prog_fd = prog_fd;
>> attr.link_create.target_fd = target_fd;
>> attr.link_create.attach_type = attach_type;
>> - attr.link_create.flags = OPTS_GET(opts, flags, 0);
>> + attr.link_create.iter_info =
>> + ptr_to_u64(OPTS_GET(opts, iter_info, (void *)0));
>> + attr.link_create.iter_info_len = OPTS_GET(opts, iter_info_len, 0);
>>
>> return sys_bpf(BPF_LINK_CREATE, &attr, sizeof(attr));
>> }
>> diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
>> index 28855fd5b5f4..c9895f191305 100644
>> --- a/tools/lib/bpf/bpf.h
>> +++ b/tools/lib/bpf/bpf.h
>> @@ -170,9 +170,10 @@ LIBBPF_API int bpf_prog_detach2(int prog_fd, int attachable_fd,
>>
>> struct bpf_link_create_opts {
>> size_t sz; /* size of this struct for forward/backward compatibility */
>> - __u32 flags;
>
> I'd actually keep flags in link_create_ops, as it's part of the kernel
> UAPI anyways, we won't have to add it later. Just pass it through into
> bpf_attr.
Okay. Will keep it.
>
>> + union bpf_iter_link_info *iter_info;
>> + __u32 iter_info_len;
>> };
>> -#define bpf_link_create_opts__last_field flags
>> +#define bpf_link_create_opts__last_field iter_info_len
>>
>> LIBBPF_API int bpf_link_create(int prog_fd, int target_fd,
>> enum bpf_attach_type attach_type,
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index 7be04e45d29c..dc8fabf9d30d 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -8298,6 +8298,7 @@ bpf_program__attach_iter(struct bpf_program *prog,
>> const struct bpf_iter_attach_opts *opts)
>> {
>> DECLARE_LIBBPF_OPTS(bpf_link_create_opts, link_create_opts);
>> + union bpf_iter_link_info linfo;
>> char errmsg[STRERR_BUFSIZE];
>> struct bpf_link *link;
>> int prog_fd, link_fd;
>> @@ -8307,8 +8308,10 @@ bpf_program__attach_iter(struct bpf_program *prog,
>> return ERR_PTR(-EINVAL);
>>
>> if (OPTS_HAS(opts, map_fd)) {
>> - target_fd = opts->map_fd;
>> - link_create_opts.flags = BPF_ITER_LINK_MAP_FD;
>> + memset(&linfo, 0, sizeof(linfo));
>> + linfo.map.map_fd = opts->map_fd;
>> + link_create_opts.iter_info = &linfo;
>> + link_create_opts.iter_info_len = sizeof(linfo);
>
> Maybe instead of having map_fd directly in bpf_iter_attach_opts, let's
> just accept bpf_iter_link_info and its len directly from the user?
> Right now kernel UAPI and libbpf API for customizing iterator
> attachment differ. It would be simpler to keep them in sync and we
> won't have to discuss how to evolve bpf_iter_attach_opts as we add
> more customization for different types of iterators. Thoughts?
Good suggestion. Previously we don't have a structure to encapsulate
map_fd so map_fd is added to the bpf_iter_attach_opts. Indeed, we can
directly add bpf_iter_link_info to link_iter_attach_opts, and this
will free libbpf from handling any future additions in bpf_iter_link_info.
>
>> }
>>
>> prog_fd = bpf_program__fd(prog);
>> --
>> 2.24.1
>>
^ permalink raw reply
* Re: [PATCH bpf-next 1/2] bpf: change uapi for bpf iterator map elements
From: Yonghong Song @ 2020-08-03 2:23 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: bpf, Networking, Alexei Starovoitov, Daniel Borkmann, Kernel Team
In-Reply-To: <CAEf4BzbaRXHpZ5b_6rojnk2dQxLFCOEwtGjNExdg5FEWadF+9g@mail.gmail.com>
On 8/2/20 6:25 PM, Andrii Nakryiko wrote:
> On Sat, Aug 1, 2020 at 9:22 PM Yonghong Song <yhs@fb.com> wrote:
>>
>> Commit a5cbe05a6673 ("bpf: Implement bpf iterator for
>> map elements") added bpf iterator support for
>> map elements. The map element bpf iterator requires
>> info to identify a particular map. In the above
>> commit, the attr->link_create.target_fd is used
>> to carry map_fd and an enum bpf_iter_link_info
>> is added to uapi to specify the target_fd actually
>> representing a map_fd:
>> enum bpf_iter_link_info {
>> BPF_ITER_LINK_UNSPEC = 0,
>> BPF_ITER_LINK_MAP_FD = 1,
>>
>> MAX_BPF_ITER_LINK_INFO,
>> };
>>
>> This is an extensible approach as we can grow
>> enumerator for pid, cgroup_id, etc. and we can
>> unionize target_fd for pid, cgroup_id, etc.
>> But in the future, there are chances that
>> more complex customization may happen, e.g.,
>> for tasks, it could be filtered based on
>> both cgroup_id and user_id.
>>
>> This patch changed the uapi to have fields
>> __aligned_u64 iter_info;
>> __u32 iter_info_len;
>> for additional iter_info for link_create.
>> The iter_info is defined as
>> union bpf_iter_link_info {
>> struct {
>> __u32 map_fd;
>> } map;
>> };
>>
>> So future extension for additional customization
>> will be easier. The bpf_iter_link_info will be
>> passed to target callback to validate and generic
>> bpf_iter framework does not need to deal it any
>> more.
>>
>> Signed-off-by: Yonghong Song <yhs@fb.com>
>> ---
>> include/linux/bpf.h | 10 ++++---
>> include/uapi/linux/bpf.h | 15 +++++-----
>> kernel/bpf/bpf_iter.c | 52 +++++++++++++++-------------------
>> kernel/bpf/map_iter.c | 37 ++++++++++++++++++------
>> kernel/bpf/syscall.c | 2 +-
>> net/core/bpf_sk_storage.c | 37 ++++++++++++++++++------
>> tools/include/uapi/linux/bpf.h | 15 +++++-----
>> 7 files changed, 104 insertions(+), 64 deletions(-)
>>
>
> [...]
>
>> int bpf_iter_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
>> {
>> + union bpf_iter_link_info __user *ulinfo;
>> struct bpf_link_primer link_primer;
>> struct bpf_iter_target_info *tinfo;
>> - struct bpf_iter_aux_info aux = {};
>> + union bpf_iter_link_info linfo;
>> struct bpf_iter_link *link;
>> - u32 prog_btf_id, target_fd;
>> + u32 prog_btf_id, linfo_len;
>> bool existed = false;
>> - struct bpf_map *map;
>> int err;
>>
>> + memset(&linfo, 0, sizeof(union bpf_iter_link_info));
>> +
>> + ulinfo = u64_to_user_ptr(attr->link_create.iter_info);
>> + linfo_len = attr->link_create.iter_info_len;
>> + if (ulinfo && linfo_len) {
>
> We probably want to be more strict here: if either pointer or len is
> non-zero, both should be present and valid. Otherwise we can have
> garbage in iter_info, as long as iter_info_len is zero.
yes, it is possible iter_info_len = 0 and iter_info is not null and
if this happens, iter_info will not be examined.
in kernel, we have places this is handled similarly. For example,
for cgroup bpf_prog query.
kernel/bpf/cgroup.c, function __cgroup_bpf_query
__u32 __user *prog_ids = u64_to_user_ptr(attr->query.prog_ids);
...
if (attr->query.prog_cnt == 0 || !prog_ids || !cnt)
return 0;
In the above case, it is possible prog_cnt = 0 and prog_ids != NULL,
or prog_ids == NULL and prog_cnt != 0, and we won't return error
to user space.
Not 100% sure whether we have convention here or not.
>
>> + err = bpf_check_uarg_tail_zero(ulinfo, sizeof(linfo),
>> + linfo_len);
>> + if (err)
>> + return err;
>> + linfo_len = min_t(u32, linfo_len, sizeof(linfo));
>> + if (copy_from_user(&linfo, ulinfo, linfo_len))
>> + return -EFAULT;
>> + }
>> +
>> prog_btf_id = prog->aux->attach_btf_id;
>> mutex_lock(&targets_mutex);
>> list_for_each_entry(tinfo, &targets, list) {
>> @@ -411,13 +425,6 @@ int bpf_iter_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
>> if (!existed)
>> return -ENOENT;
>>
>> - /* Make sure user supplied flags are target expected. */
>> - target_fd = attr->link_create.target_fd;
>> - if (attr->link_create.flags != tinfo->reg_info->req_linfo)
>> - return -EINVAL;
>> - if (!attr->link_create.flags && target_fd)
>> - return -EINVAL;
>> -
>
> Please still ensure that no flags are specified.
Make sense. I also need to ensure target_fd is 0 since it is not used
any more.
>
>
>> link = kzalloc(sizeof(*link), GFP_USER | __GFP_NOWARN);
>> if (!link)
>> return -ENOMEM;
>> @@ -431,28 +438,15 @@ int bpf_iter_link_attach(const union bpf_attr *attr, struct bpf_prog *prog)
>> return err;
>> }
>>
>
> [...]
>
>> -static int bpf_iter_check_map(struct bpf_prog *prog,
>> - struct bpf_iter_aux_info *aux)
>> +static int bpf_iter_attach_map(struct bpf_prog *prog,
>> + union bpf_iter_link_info *linfo,
>> + struct bpf_iter_aux_info *aux)
>> {
>> - struct bpf_map *map = aux->map;
>> + struct bpf_map *map;
>> + int err = -EINVAL;
>>
>> - if (map->map_type != BPF_MAP_TYPE_SK_STORAGE)
>> + if (!linfo->map.map_fd)
>> return -EINVAL;
>
> This could be -EBADF?
Good suggestion. Will do.
>
>>
>> - if (prog->aux->max_rdonly_access > map->value_size)
>> - return -EACCES;
>> + map = bpf_map_get_with_uref(linfo->map.map_fd);
>> + if (IS_ERR(map))
>> + return PTR_ERR(map);
>> +
>> + if (map->map_type != BPF_MAP_TYPE_SK_STORAGE)
>> + goto put_map;
>> +
>> + if (prog->aux->max_rdonly_access > map->value_size) {
>> + err = -EACCES;
>> + goto put_map;
>> + }
>
> [...]
>
^ permalink raw reply
* [PATCH net-next] tipc: Use is_broadcast_ether_addr() instead of memcmp()
From: Huang Guobin @ 2020-08-03 2:00 UTC (permalink / raw)
To: jmaloy, ying.xue, davem, kuba; +Cc: netdev, tipc-discussion, linux-kernel
Using is_broadcast_ether_addr() instead of directly use
memcmp() to determine if the ethernet address is broadcast
address.
spatch with a semantic match is used to found this problem.
(http://coccinelle.lip6.fr/)
Signed-off-by: Huang Guobin <huangguobin4@huawei.com>
---
net/tipc/eth_media.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/net/tipc/eth_media.c b/net/tipc/eth_media.c
index 8b0bb600602d..c68019697cfe 100644
--- a/net/tipc/eth_media.c
+++ b/net/tipc/eth_media.c
@@ -62,12 +62,10 @@ static int tipc_eth_raw2addr(struct tipc_bearer *b,
struct tipc_media_addr *addr,
char *msg)
{
- char bcast_mac[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff};
-
memset(addr, 0, sizeof(*addr));
ether_addr_copy(addr->value, msg);
addr->media_id = TIPC_MEDIA_TYPE_ETH;
- addr->broadcast = !memcmp(addr->value, bcast_mac, ETH_ALEN);
+ addr->broadcast = is_broadcast_ether_addr(addr->value);
return 0;
}
--
2.17.1
^ permalink raw reply related
* Re: [PATCH bpf-next 5/5] selftests/bpf: add benchmark for uprobe vs. user_prog
From: Andrii Nakryiko @ 2020-08-03 1:51 UTC (permalink / raw)
To: Song Liu
Cc: open list, bpf, Networking, Alexei Starovoitov, Daniel Borkmann,
Kernel Team, john fastabend, KP Singh, Jesper Dangaard Brouer,
Daniel Xu
In-Reply-To: <20200801084721.1812607-6-songliubraving@fb.com>
On Sat, Aug 1, 2020 at 1:50 AM Song Liu <songliubraving@fb.com> wrote:
>
> Add a benchmark to compare performance of
> 1) uprobe;
> 2) user program w/o args;
> 3) user program w/ args;
> 4) user program w/ args on random cpu.
>
Can you please add it to the existing benchmark runner instead, e.g.,
along the other bench_trigger benchmarks? No need to re-implement
benchmark setup. And also that would also allow to compare existing
ways of cheaply triggering a program vs this new _USER program?
If the performance is not significantly better than other ways, do you
think it still makes sense to add a new BPF program type? I think
triggering KPROBE/TRACEPOINT from bpf_prog_test_run() would be very
nice, maybe it's possible to add that instead of a new program type?
Either way, let's see comparison with other program triggering
mechanisms first.
> Sample output:
>
> ./test_progs -t uprobe_vs_user_prog -v
> test_uprobe_vs_user_prog:PASS:uprobe_vs_user_prog__open_and_load 0 nsec
> test_uprobe_vs_user_prog:PASS:get_base_addr 0 nsec
> test_uprobe_vs_user_prog:PASS:attach_uprobe 0 nsec
> run_perf_test:PASS:uprobe 0 nsec
> Each uprobe uses 1419 nanoseconds
> run_perf_test:PASS:user_prog_no_args 0 nsec
> Each user_prog_no_args uses 313 nanoseconds
> run_perf_test:PASS:user_prog_with_args 0 nsec
> Each user_prog_with_args uses 335 nanoseconds
> run_perf_test:PASS:user_prog_with_args_on_cpu 0 nsec
> Each user_prog_with_args_on_cpu uses 2821 nanoseconds
> Summary: 1/0 PASSED, 0 SKIPPED, 0 FAILED
>
> Signed-off-by: Song Liu <songliubraving@fb.com>
> ---
> .../bpf/prog_tests/uprobe_vs_user_prog.c | 101 ++++++++++++++++++
> .../selftests/bpf/progs/uprobe_vs_user_prog.c | 21 ++++
> 2 files changed, 122 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/prog_tests/uprobe_vs_user_prog.c
> create mode 100644 tools/testing/selftests/bpf/progs/uprobe_vs_user_prog.c
>
[...]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox