| Message ID | 20230515134643.48314-2-lanhao@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: hns3: There are some cleanup for the HNS3 ethernet driver | expand |
On Mon, May 15, 2023 at 09:46:40PM +0800, Hao Lan wrote: > From: Jian Shen <shenjian15@huawei.com> > > The expression '(k ^ ~v)' is exaclty '(k & v)', and This part doesn't seem correct to me. Suppose both k and v are 0. For simplicity, consider only one bit. Then: (k ^ ~v) == (0 ^ ~0) == (0 ^ 1) = 1 But (k & v) == (0 & 0) == 0 > '(k & v) & k' is exaclty 'k & v'. So simplify the > expression for tcam key convert. This I follow. > (k ^ ~v) & k == (k & v) & k == k & k & v == k & v Looking at the truth table (in non table form), this seems correct. k == 0, v == 0: (k ^ ~v) & k == (0 ^ ~0) & 0 == (0 ^ 1) & 0 == 1 & 0 == 0 k & v == 0 & 0 == 0 Good! k == 0, v == 1: (k ^ ~v) & k == (0 ^ ~1) & 0 == (0 ^ 0) & 0 == 1 & 0 == 0 k & v == 0 & 1 == 0 Good! k == 1, v == 0: (k ^ ~v) & k == (1 ^ ~0) & 1 == (1 ^ 1) & 1 == 0 & 1 == 0 k & v == 1 & 0 == 0 Good! k == 1, v == 1: (k ^ ~v) & k == (1 ^ ~1) & 1 == (1 ^ 0) & 1 == 1 & 1 == 1 k & v == 1 & 1 == 1 Good! > > It also add necessary brackets for them. > > Signed-off-by: Jian Shen <shenjian15@huawei.com> > Signed-off-by: Hao Lan <lanhao@huawei.com> > --- > .../net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h | 11 +++-------- > 1 file changed, 3 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h > index 81aa6b0facf5..6a43d1515585 100644 > --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h > @@ -835,15 +835,10 @@ struct hclge_vf_vlan_cfg { > * Then for input key(k) and mask(v), we can calculate the value by > * the formulae: > * x = (~k) & v > - * y = (k ^ ~v) & k > + * y = k & v > */ > -#define calc_x(x, k, v) (x = ~(k) & (v)) > -#define calc_y(y, k, v) \ > - do { \ > - const typeof(k) _k_ = (k); \ > - const typeof(v) _v_ = (v); \ > - (y) = (_k_ ^ ~_v_) & (_k_); \ > - } while (0) > +#define calc_x(x, k, v) ((x) = ~(k) & (v)) > +#define calc_y(y, k, v) ((y) = (k) & (v)) This also looks good to me. > > #define HCLGE_MAC_STATS_FIELD_OFF(f) (offsetof(struct hclge_mac_stats, f)) > #define HCLGE_STATS_READ(p, offset) (*(u64 *)((u8 *)(p) + (offset)))
On Mon, May 15, 2023 at 05:51:03PM +0200, Simon Horman wrote: > On Mon, May 15, 2023 at 09:46:40PM +0800, Hao Lan wrote: > > From: Jian Shen <shenjian15@huawei.com> > > > > The expression '(k ^ ~v)' is exaclty '(k & v)', and Sorry, one more thing: s/exaclty/exactly/ Also one more time below. > > This part doesn't seem correct to me. > > Suppose both k and v are 0. > For simplicity, consider only one bit. > > Then: (k ^ ~v) == (0 ^ ~0) == (0 ^ 1) = 1 > But (k & v) == (0 & 0) == 0 > > > '(k & v) & k' is exaclty 'k & v'. So simplify the > > expression for tcam key convert. > > This I follow. > > > (k ^ ~v) & k == (k & v) & k == k & k & v == k & v > > Looking at the truth table (in non table form), this seems correct. > > k == 0, v == 0: > > (k ^ ~v) & k == (0 ^ ~0) & 0 == (0 ^ 1) & 0 == 1 & 0 == 0 > k & v == 0 & 0 == 0 > Good! > > k == 0, v == 1: > > (k ^ ~v) & k == (0 ^ ~1) & 0 == (0 ^ 0) & 0 == 1 & 0 == 0 > k & v == 0 & 1 == 0 > Good! > > k == 1, v == 0: > > (k ^ ~v) & k == (1 ^ ~0) & 1 == (1 ^ 1) & 1 == 0 & 1 == 0 > k & v == 1 & 0 == 0 > Good! > > k == 1, v == 1: > > (k ^ ~v) & k == (1 ^ ~1) & 1 == (1 ^ 0) & 1 == 1 & 1 == 1 > k & v == 1 & 1 == 1 > Good! > > > > > It also add necessary brackets for them. > > > > Signed-off-by: Jian Shen <shenjian15@huawei.com> > > Signed-off-by: Hao Lan <lanhao@huawei.com> > > --- > > .../net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h | 11 +++-------- > > 1 file changed, 3 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h > > index 81aa6b0facf5..6a43d1515585 100644 > > --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h > > +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h > > @@ -835,15 +835,10 @@ struct hclge_vf_vlan_cfg { > > * Then for input key(k) and mask(v), we can calculate the value by > > * the formulae: > > * x = (~k) & v > > - * y = (k ^ ~v) & k > > + * y = k & v > > */ > > -#define calc_x(x, k, v) (x = ~(k) & (v)) > > -#define calc_y(y, k, v) \ > > - do { \ > > - const typeof(k) _k_ = (k); \ > > - const typeof(v) _v_ = (v); \ > > - (y) = (_k_ ^ ~_v_) & (_k_); \ > > - } while (0) > > +#define calc_x(x, k, v) ((x) = ~(k) & (v)) > > +#define calc_y(y, k, v) ((y) = (k) & (v)) > > This also looks good to me. > > > > > #define HCLGE_MAC_STATS_FIELD_OFF(f) (offsetof(struct hclge_mac_stats, f)) > > #define HCLGE_STATS_READ(p, offset) (*(u64 *)((u8 *)(p) + (offset))) >
diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h index 81aa6b0facf5..6a43d1515585 100644 --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.h @@ -835,15 +835,10 @@ struct hclge_vf_vlan_cfg { * Then for input key(k) and mask(v), we can calculate the value by * the formulae: * x = (~k) & v - * y = (k ^ ~v) & k + * y = k & v */ -#define calc_x(x, k, v) (x = ~(k) & (v)) -#define calc_y(y, k, v) \ - do { \ - const typeof(k) _k_ = (k); \ - const typeof(v) _v_ = (v); \ - (y) = (_k_ ^ ~_v_) & (_k_); \ - } while (0) +#define calc_x(x, k, v) ((x) = ~(k) & (v)) +#define calc_y(y, k, v) ((y) = (k) & (v)) #define HCLGE_MAC_STATS_FIELD_OFF(f) (offsetof(struct hclge_mac_stats, f)) #define HCLGE_STATS_READ(p, offset) (*(u64 *)((u8 *)(p) + (offset)))